Skip to content

HTTP/2: server RST_STREAM with INTERNAL_ERROR isn't covered by the existing retry mechanism #231

Closed
socketry/async-http-faraday
#61

Description

@jakeonfire

Following up from #220 as suggested — this is the "Internal error!" variant, captured in production.

Versions: async-http 0.98.1, protocol-http2 0.26.2, protocol-http 0.64.0, async-http-faraday 0.22.2, async 2.43.0, Ruby 4.0.6.

I don't think async-http is at fault here. The Facebook Graph API sends complete 200 response headers and then immediately resets the stream before a single DATA frame — their backend giving up mid-response. This is a first attempt on a healthy, long-lived connection, no GOAWAY nearby, one open stream, 681ms end to end:

write  #<HeadersFrame stream_id=5573 flags=5 373b>            # END_STREAM|END_HEADERS, bodyless GET
read   #<WindowUpdateFrame stream_id=5573 payload=10420224>
read   #<HeadersFrame stream_id=5573 flags=4 278b>            # :status 200, content-type: application/json, no content-length
read   #<ResetStreamFrame stream_id=5573 flags=0 payload=2>   # INTERNAL_ERROR
close! stream_id=5573 error_code=2 state=half_closed_local open_streams=1 elapsed_ms=681

The request was GET https://graph.facebook.com/v24.0/me/posts?fields=...&limit=40. It isn't throttling — the response carried x-app-usage: {"call_count":0,"total_cputime":0,"total_time":0}. It isn't endpoint-specific either; we see the same on /me/photos. Frequency is roughly 5-10/hour across our fleet.

The question is whether the existing retry mechanism should cover it. Today Client#call rescues RefusedError and the socket errors, both gated on request.rewind!/request.retry!, but StreamError propagates to the caller. There is already precedent for treating a stream-level error as retryable transport noise — Stream#close! converts REFUSED_STREAM into Protocol::HTTP::RefusedError for exactly that reason. INTERNAL_ERROR looks like the same category: the request is gone, nothing usable reached the application, and request.retry! would keep unsafe methods out of it.

I would suggest keeping it narrow rather than retrying every StreamError. PROTOCOL_ERROR in particular should keep propagating — retrying it would have masked the body-rewind bug from #220 rather than surfacing it.

Two caveats worth noting:

  1. The reset arrives after :status 200, so the request was genuinely processed server-side. That is fine for idempotent methods, which is what retry! already enforces, but it is a different situation from REFUSED_STREAM.
  2. The retry block only wraps make_response, so this would only help when the reset lands before the caller receives the response object — as it did here, since the reading fiber processed HEADERS and RST_STREAM back to back. A reset arriving well into the body would still surface during body reading, outside the retry. So this would not be a general guarantee.

A note on the trace format: I extended the framer logging from #220 rather than using it verbatim. Alongside the frame reads/writes it logs the decoded request line and the response status/headers — the frame logs are HPACK-opaque, and these connections multiplex thousands of streams, so tying a reset back to its request needed the decoded view — plus stream state, open stream count, elapsed time at the reset, and the Client#call attempt number. Happy to share that initializer if it would be useful to anyone else debugging this.

This reproduces reliably for us, so I am glad to capture more traces or test a fix against production traffic whenever you want.

Activity

  1. ioquatix commented on Jul 28, 2026

    @ioquatix
    Member

    I think the proposal here is probably acceptable - if the remote end sends a stream reset with INTERNAL_ERROR - the risk is we would DDoS the remote end by retrying, if the remote end truely is failing for a deterministic reason. That being said, from the client point of view, our goal is to do what you ask, and to hell with the remote system, so retrying seems reasonable, but if it is deterministic, we would eventually fail. If it's non-deterministic, you may eventually get what you want. That said, I agree this is an upstream (Facebook) issue.

  2. ioquatix commented on Jul 29, 2026

    @ioquatix
    Member

    Can you try v0.99.0 it should now retry if possible, on StreamReset(INTERNAL_ERROR).

  3. jakeonfire commented on Jul 29, 2026

    @jakeonfire
    Author

    Thanks for the quick turnaround on 0.99.0. We deployed it and the INTERNAL_ERRORs are unchanged — still surfacing as Protocol::HTTP2::StreamError, with no Protocol::HTTP::RemoteError anywhere in our error tracking, so the new conversion isn't being reached.

    I think the promise refactor in the same release moved this case out of its path.

    In 0.98.1, wait waited on a notification and then checked @exception afterwards, so a reset arriving before the waiting fiber resumed meant the exception won and surfaced from wait:

    async/http/protocol/http2/client.rb:56     read_response -> response.wait
    async/http/protocol/http2/response.rb:173  wait -> @stream.wait
    async/http/protocol/http2/response.rb:123  raise @exception
    

    In 0.99.0, receive_initial_headers ends with @ready.resolve(nil), and Async::Promise#reject starts with return if @resolved — so the subsequent @ready.reject(error) in closed is a no-op. Since this server sends complete :status 200 headers and only then resets the stream, wait now returns successfully, Client#call returns the response, and the error surfaces later when the body is read:

    async/http/faraday/adapter.rb:219          save_response(env, response.status, encoded_body(response), ...)
    async/http/faraday/adapter.rb:261          body = response.read
    protocol/http/body/readable.rb:96          while chunk = self.read
    async/http/protocol/http2/input.rb:27
    protocol/http/body/writable.rb:71          raise @error
    

    So the rescue ::Protocol::HTTP2::StreamError in wait is never entered for this shape, and by the time the error appears the response has already been returned from Client#call, past the retry block.

    The fix should still cover a reset arriving before any response headers, since the promise gets rejected first — it's specifically the "headers, then reset" ordering that slips through.

    I don't see a clean answer from where I'm sitting: retrying isn't possible once the response object is out, and always preferring the rejection over the resolution would break legitimate streaming responses that fail partway. One option, if it's useful, would be converting the error stored on the body as well, so applications can rescue a single RemoteError type regardless of where the reset lands.

    Versions: async-http 0.99.0, protocol-http 0.66.0, protocol-http2 0.26.2, async 2.44.0, async-http-faraday 0.22.2, Ruby 4.0.6.

    Happy to grab more traces or test any patch against production traffic — this still reproduces for us several times an hour.

    🤖 Reply via Claude

  4. samuel-williams-shopify commented on Jul 30, 2026

    @samuel-williams-shopify
    Contributor

    Thanks for testing v0.99.0 and for the detailed analysis. I agree with your diagnosis: once the response headers have resolved readiness, a later reset is delivered through the response body, outside Client#call's retry scope.

    I opened #236 to cover that path by mapping an HTTP/2 StreamError(INTERNAL_ERROR) raised while reading the body to Protocol::HTTP::RemoteError, preserving the original StreamError as Exception#cause. This gives callers one protocol-independent failure type regardless of whether the reset arrives before or after the response headers.

    I don't think async-http can safely provide unconditional retries once a response has been returned. Body bytes may already have reached application code, the server may already have processed the request, and the library cannot know whether repeating either the request or caller-side processing is safe. We discussed executing an explicit response-buffering block inside Client#call's retry scope, which could make a fully buffered idempotent request retryable, but that API would need very clear semantics because the block could execute multiple times.

    For now, the intended boundary is:

    • retry eligible transport failures that occur before returning the response;
    • normalize a peer INTERNAL_ERROR during later body reads as RemoteError;
    • preserve the protocol-specific cause; and
    • let the caller or adapter apply an explicit retry policy.

    I also opened socketry/async-http-faraday#61, which maps RemoteError to Faraday::ConnectionFailed and demonstrates using faraday-retry for idempotent requests. That keeps retry count and policy visible at the application integration layer.

    We may use RemoteError for other clearly remote-originated failures in future, such as the HTTP/3 equivalent or a prematurely truncated HTTP/1 response, while keeping protocol violations, valid HTTP 5xx responses, cancellation, and RefusedError distinct.

  5. samuel-williams-shopify commented on Sep 18, 2026

    @samuel-williams-shopify
    Contributor

    Following up with the released fix: async-http-faraday 0.24.0 includes socketry/async-http-faraday#61.

    This supersedes the body-error wrapping proposal in my earlier comment. We closed #236 without merging it: Body#read can raise many kinds of exceptions, and wrapping only INTERNAL_ERROR as RemoteError would not provide a general retry-safety contract or bring body consumption into Client#call's retry scope.

    Instead, the Faraday adapter now translates Protocol::HTTP::Error and its subclasses into Faraday::ConnectionFailed, preserving the original exception as cause. This covers HTTP/1 and HTTP/2 errors, including the reported StreamError(INTERNAL_ERROR) raised after response headers, without requiring an additional wrapper in async-http. The regression tests retain the close_write(error) body-read failure path and explicitly cover INTERNAL_ERROR, CANCEL, and other protocol errors.

    The adapter does not add retries automatically. After upgrading, install faraday-retry and configure an explicit policy, for example for bodyless GET/HEAD requests whose incomplete responses can be discarded:

    require "async/http/faraday"
    require "faraday/retry"
    
    connection = Faraday.new("https://graph.facebook.com") do |builder|
    	builder.request :retry, max: 2, interval: 0.1, backoff_factor: 2,
    		methods: [:get, :head], exceptions: [Faraday::ConnectionFailed]
    	builder.adapter :async_http
    end

    Faraday's retry scope includes the adapter's response-body consumption, so this handles the headers-then-reset ordering described here. The broader error translation includes locally detected protocol errors too; it does not guarantee that retrying is safe or useful. Requests with bodies need replayable bodies, and streaming callbacks must account for partial output. Each Faraday attempt may also include async-http's existing internal retries.

    @jakeonfire, please try 0.24.0 with an explicit retry policy and let us know whether it handles the failures you are seeing.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions