Skip to content

Commit 8d6d53b

Browse files
committed
Treat only 2xx as a successful upload
Every one of these SDKs treated a 3xx as a failure before this work, and the change to 200-399 came from the design doc's "Spec item 1: 2xx and 3xx are success". That line is wrong, and the doc is what needs correcting. Measured against a local server, with the same HTTP clients these SDKs use: 307/308 + Location -> followed as POST with the body, arrives as 200 301/302/303 + Loc. -> followed as GET with no body, arrives as 200 302 without Location-> surfaces raw as 302 300 Multiple Choices-> surfaces raw as 300 304 Not Modified -> surfaces raw as 304 So a raw 3xx only reaches the classifier when the client has already declined to follow it, meaning nothing was uploaded. The one redirect that genuinely works, 307/308, never produces a 3xx here at all — it produces 200 — so narrowing the bound cannot break it. Nothing was gained by the wider range; a 300, 304, or Location-less 302 from a proxy was being logged as a delivered batch and dropped with no error callback. The narrower bound also needs no new branches: a 3xx is neither 5xx nor in the retryable 4xx set, so it already falls through to the non-retryable path and reports a failure. TAPI does not emit 3xx and has no plans to. This matters because host is customer-configurable and proxies in front of it are common. curl is not configured to follow redirects, so php can also see a raw 3xx; it now reports a named redirect error instead of an empty body. Socket narrowed to match.
1 parent d4a297a commit 8d6d53b

2 files changed

Lines changed: 17 additions & 4 deletions

File tree

‎lib/Consumer/LibCurl.php‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,23 @@ public function flushBatch(array $messages): bool
6262
return false;
6363
}
6464

65-
// 2xx and 3xx are success
66-
if ($responseCode >= 200 && $responseCode < 400) {
65+
// Only 2xx is success. curl is not configured to follow redirects, so a
66+
// 3xx means nothing was uploaded; treating it as success would drop the
67+
// batch silently. TAPI does not emit 3xx — this shows up when the
68+
// configured host is a proxy or redirector.
69+
if ($responseCode >= 200 && $responseCode < 300) {
6770
return true;
6871
}
6972

73+
if ($responseCode >= 300 && $responseCode < 400) {
74+
$this->handleError(
75+
$responseCode,
76+
'Unexpected redirect; batch not uploaded. Check whether the configured '
77+
. 'host points at a proxy or redirector.'
78+
);
79+
return false;
80+
}
81+
7082
$this->handleError($responseCode, $responseContent);
7183

7284
if (!$this->isRetryable($responseCode)) {

‎lib/Consumer/Socket.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -187,8 +187,9 @@ private function makeRequest($socket, string $req, string $payload): bool
187187
}
188188
fclose($socket);
189189

190-
// 2xx and 3xx are success
191-
if ($statusCode >= 200 && $statusCode < 400) {
190+
// Only 2xx is success; a raw socket never follows redirects, so a 3xx
191+
// means nothing was uploaded.
192+
if ($statusCode >= 200 && $statusCode < 300) {
192193
return true;
193194
}
194195

0 commit comments

Comments
 (0)