Skip to content

Translate HTTP protocol failures into Faraday connection errors - #61

Merged
samuel-williams-shopify merged 3 commits into
mainfrom
handle-remote-error
Sep 18, 2026
Merged

samuel-williams-shopify merged 3 commits into
mainfrom
handle-remote-error

Conversation

@samuel-williams-shopify

@samuel-williams-shopify samuel-williams-shopify commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

An HTTP/2 reset after response headers raises Protocol::HTTP2::StreamError during body consumption. It currently escapes the adapter without becoming Faraday::ConnectionFailed, so retry middleware configured for that exception misses the failure.

Translate Protocol::HTTP::Error into Faraday::ConnectionFailed, preserving the original exception as its cause. This common base covers HTTP/1 and HTTP/2 errors, including RemoteError, RefusedError, and StreamError, without enumerating subclasses in the adapter.

The adapter adds no retries. The broader translation includes locally detected protocol errors and does not imply that retrying is safe or useful. The documentation demonstrates an explicit faraday-retry policy for bodyless GET/HEAD requests and explains compounded attempts, request-body replayability, and partial streamed output.

Regression tests retain the response-body failure path using close_write(error) and enumerate concrete cases: RemoteError, HTTP/2 stream resets with INTERNAL_ERROR and CANCEL, RefusedError, HTTP/1 ContentLengthError, HTTP/2 GoawayError, and the common/HTTP1/HTTP2 base errors. Each verifies a successful middleware retry and preservation of the exact original cause. The existing async-http >= 0.99 requirement remains.

Validation: 37 tests passed (192 assertions); RuboCop passed (21 files).

Fixes socketry/async-http#231.

Comment thread context/getting-started.md Outdated
require "faraday/retry"

connection = Faraday.new(...) do |builder|
builder.request :retry, exceptions: [Faraday::ConnectionFailed]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this somewhat redundant with async-http's internal retries of socket failures? might this cause issues for PUT body streams that may have been drained? i have no qualms with the documentation, just trying to learn 🙂

@samuel-williams-shopify samuel-williams-shopify changed the title Handle remote HTTP errors Translate HTTP/2 stream failures into Faraday connection errors Sep 18, 2026
@samuel-williams-shopify samuel-williams-shopify changed the title Translate HTTP/2 stream failures into Faraday connection errors Translate HTTP protocol failures into Faraday connection errors Sep 18, 2026
@samuel-williams-shopify
samuel-williams-shopify merged commit 4afcadc into main Sep 18, 2026
22 of 28 checks passed
@samuel-williams-shopify
samuel-williams-shopify deleted the handle-remote-error branch September 18, 2026 11:08
@ioquatix ioquatix added this to the v0.24.0 milestone Sep 18, 2026
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.

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

3 participants