Skip to content

[CHA-5724] fix: retry requests on dropped connections - #178

Closed
nijeesh-stream wants to merge 2 commits into
mainfrom
fix/cha-5724-retry-dropped-connections
Closed

nijeesh-stream wants to merge 2 commits into
mainfrom
fix/cha-5724-retry-dropped-connections

Conversation

@nijeesh-stream

@nijeesh-stream nijeesh-stream commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

TLDR

  • Problem: Google's front-end load balancer in front of the Chat API sometimes drops a connection. When that happens, PHP SDK calls fail with cURL error 56: OpenSSL SSL_read: unexpected eof while reading. Google says clients have to cope with dropped connections.
  • What breaks today: the SDK never retries, so one dropped connection is one failed call. A customer on 3.17.0 (Guzzle 7, curl with OpenSSL 3, Alpine) sees these intermittently. 93% of their traffic is GET /unread.
  • What this changes: the default HTTP client now gets a Guzzle retry middleware. It makes at most 2 retries (3 attempts in total) with a short jittered backoff. Any request is retried if the connection was never established, because then the request was never sent. If the connection drops after the request may have been sent, only GET, HEAD and OPTIONS are retried. The rule looks only at the curl error and the HTTP method, never at the endpoint.
  • Risks: a failing call can take up to about 300ms of backoff plus the extra attempts. POST, PUT, PATCH and DELETE are never retried once they may have been sent, so no message or write is duplicated. Clients set with setHttpClient() are left as they are. Client::retryMiddleware() lets those users opt in.

Ticket

Summary

Client::__construct pushes Client::retryMiddleware() (built on GuzzleHttp\Middleware::retry) onto the HandlerStack it creates. The decision uses the curl errno from getHandlerContext()['errno'], not the exception class. Guzzle maps errors to exception classes inconsistently: 28, 6, 7, 35 and 52 become ConnectException, everything else becomes RequestException.

Condition Methods Retried? Why
curl 6 COULDNT_RESOLVE_HOST, 7 COULDNT_CONNECT, 35 SSL_CONNECT_ERROR any yes, up to 2 times The connection was never established, so the request was never sent and is safe to repeat.
curl 52 GOT_NOTHING, 55 SEND_ERROR, 56 RECV_ERROR GET, HEAD, OPTIONS yes, up to 2 times The request may have been processed, but repeating a safe method does no harm.
curl 52, 55, 56 POST, PUT, PATCH, DELETE no The server may already have processed it, and our writes are not generally idempotent (for example SendMessage without an id duplicates the message).
curl 28 timeout any no Existing timeout behaviour stays the same.
any HTTP response (4xx/5xx) any no The server answered.
no errno (for example the non-curl stream handler) any no We can't tell whether the request was sent.

Backoff: 50–100ms before the 1st retry and 100–200ms before the 2nd, so at most 300ms in total.

Public API added: Client::retryMiddleware(): callable, documented in the README FAQ for users who inject their own Guzzle client. Everything else is private.

Evidence

Local reproductions (TLS server written in PHP, curl with OpenSSL 3, Guzzle 7.16):

  • A stale pooled keep-alive connection, which the server closed while idle, is already handled transparently by curl for both GET and POST. This change doesn't touch that case.
  • If the server reads the full request and then sends a TCP FIN without a TLS close_notify, the client gets RequestException with errno 56. The message is identical to the customer's: cURL error 56: OpenSSL SSL_read: OpenSSL/3.5.4: error:0A000126:SSL routines::unexpected eof while reading. The server did receive the request.
  • curl reports this mid-flight drop as 56 or 52 depending on the build. The Docker image (Debian, OpenSSL 3.5.4) reports 56 (RequestException), like the customer. The Ubuntu GitHub runner reports 52 Empty reply from server (ConnectException). The policy treats 52 and 56 the same way (retry only GET/HEAD/OPTIONS), so both are covered. The real-socket tests accept either errno and assert the attempt counts exactly.
  • If the server accepts TCP and closes it before the TLS handshake, the client gets ConnectException with errno 35, and the request was never sent.

Tests added in tests/unit/RetryTest.php. The first 5 use a real TLS server (tests/unit/fixtures/tls_server.php) started as a subprocess, and go through the SDK's default client. The errno of each attempt is recorded with on_stats:

  1. GET (unreadCounts) gets a mid-flight drop, then a 200 → succeeds. The server saw 2 GETs; the first attempt's errno was 52 or 56, the second 0.
  2. POST (upsertUser) gets a mid-flight drop → throws errno 52 or 56. The server saw exactly 1 POST, so there was no retry.
  3. POST where the first TLS handshake fails → retried, with attempt errnos [35, 0]. The JSON body arrived intact on the successful attempt, which shows the body is rewound.
  4. HTTP 500 → ServerException. The server saw 1 request.
  5. Every attempt drops → gives up after 3 attempts (each errno 52 or 56). The server saw 3 connections.
  6. MockHandler matrix: errnos 6/7/35 are retried for all methods; 52/55/56 are retried only for GET/HEAD/OPTIONS; 28 and a missing errno are not retried; 429/500/502/503 responses are not retried.

First run on the committed branch (f9e84c9) with PHP 8.1, Guzzle 7.16.0 and the repo's docker/Dockerfile:

$ vendor/bin/phpunit --testsuite "Unit Test Suite" --filter RetryTest --testdox
Retry (GetStream\Unit\Retry)
 ✔ Get is retried when connection drops after request was sent
 ✔ Post is not retried when connection drops after request was sent
 ✔ Post is retried when tls handshake fails
 ✔ Server error is not retried
 ✔ Get gives up after three attempts
 ✔ Connect phase errors are retried for any method
 ✔ Errors after send are retried only for safe methods
 ✔ Timeouts and unknown errors are not retried
 ✔ Http error responses are not retried
OK (9 tests, 72 assertions)

$ vendor/bin/phpunit --testsuite "Unit Test Suite"
OK (57 tests, 138 assertions)
$ vendor/bin/php-cs-fixer fix -v --dry-run --stop-on-violation   # exit 0
$ vendor/bin/phan --no-progress-bar                              # exit 0

After the follow-up test: commit (961eb0b, which accepts errno 52 or 56), the same Docker run gave: unit suite OK (57 tests, 144 assertions), RetryTest OK (9 tests, 78 assertions), and php-cs-fixer and phan both exit 0.

Other runs:

  • On php:8.3-cli-alpine (curl 8.22, OpenSSL 3.5.8, close to the customer's setup), the full unit suite passed: OK (57 tests, 138 assertions).
  • With Guzzle 6.5.8 installed in a scratch copy, RetryTest passed: OK (9 tests, 72 assertions). That confirms errno is in the handler context and the body is rewound on Guzzle 6 as well.

Risks

  • Latency on failure: a call that keeps failing now takes up to 3 attempts plus at most about 300ms of backoff before it throws. Calls that succeed don't change.
  • No duplicate writes: POST, PUT, PATCH and DELETE are never retried after the request may have been sent. They are retried only when the connection was never established.
  • Timeouts unchanged: errno 28 is never retried, so the effective timeout doesn't grow because of timeouts.
  • Custom clients unaffected: if you call setHttpClient(), your client is used exactly as you configured it. You can opt in with Client::retryMiddleware().
  • Non-curl handler: if ext-curl is missing, Guzzle's stream handler gives no errno, so no retries happen. Behaviour stays as it is today.

🤖 Generated with Claude Code

Load balancers in front of the API occasionally drop a connection,
which surfaces as cURL error 56 "unexpected eof while reading". The
default HTTP client now retries up to 2 times with a short jittered
backoff: any method when the connection was never established
(cURL 6, 7, 35), and only GET, HEAD and OPTIONS when the connection
dropped after the request may have been sent (cURL 52, 55, 56).
HTTP responses and timeouts are never retried.

Clients set with setHttpClient are unchanged; Client::retryMiddleware()
exposes the same policy for their handler stack.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
curl reports a connection closed after the request was sent as
RECV_ERROR (56) or GOT_NOTHING (52) depending on the curl/OpenSSL
build; the Ubuntu CI runner reports 52. The retry policy already
treats both alike, so assert either.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@nijeesh-stream

Copy link
Copy Markdown
Contributor Author

Closing without merging: CHA-5724 has been closed (no further customer reports). Keeping this PR and branch as a reference implementation in case the issue comes back.

Context for whoever picks it up:

  • Root cause sits in Google's front-end proxy (scheduled proxy_restarts drop connections); our edge closes connections cleanly. Google documents that clients must tolerate dropped connections.
  • No Stream server SDK retries by default. The generated SDKs share the CHA-2959 retry policy (off by default, GET/HEAD, transport errors + 429 with Retry-After, 3 attempts). This PR is stricter on safety (retries any method only when the request provably never left the client) but defaults ON, which differs from CHA-2959 — align with that policy before reviving.
  • How the drop surfaces varies by build (curl errno 52 vs 56; Ruby SSLError vs ConnectionFailed/EOFError); the tests were adjusted to assert behaviour, not error spelling.

Findings are documented on https://linear.app/stream/issue/CHA-5724

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant