From 3a34e82d24f03c264a2c5b927239dc6968c9bee3 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Sat, 11 Apr 2026 22:21:18 +1200 Subject: [PATCH] Better handling of refused requests. --- async-http.gemspec | 6 ++-- lib/async/http/client.rb | 4 +-- lib/async/http/protocol/http1/client.rb | 23 +++++------- lib/async/http/protocol/http2/response.rb | 2 +- lib/async/http/protocol/request.rb | 4 --- releases.md | 7 ++++ .../http2/request_failed_on_refused_stream.rb | 36 +++++++++++++++++++ 7 files changed, 58 insertions(+), 24 deletions(-) create mode 100644 test/async/http/protocol/http2/request_failed_on_refused_stream.rb diff --git a/async-http.gemspec b/async-http.gemspec index b182c9d..4d578ac 100644 --- a/async-http.gemspec +++ b/async-http.gemspec @@ -29,9 +29,9 @@ Gem::Specification.new do |spec| spec.add_dependency "io-endpoint", "~> 0.14" spec.add_dependency "io-stream", "~> 0.6" spec.add_dependency "metrics", "~> 0.12" - spec.add_dependency "protocol-http", "~> 0.58" - spec.add_dependency "protocol-http1", "~> 0.36" - spec.add_dependency "protocol-http2", "~> 0.22" + spec.add_dependency "protocol-http", "~> 0.62" + spec.add_dependency "protocol-http1", "~> 0.39" + spec.add_dependency "protocol-http2", "~> 0.26" spec.add_dependency "protocol-url", "~> 0.2" spec.add_dependency "traces", "~> 0.10" end diff --git a/lib/async/http/client.rb b/lib/async/http/client.rb index a3e5dc1..56bc9f6 100755 --- a/lib/async/http/client.rb +++ b/lib/async/http/client.rb @@ -114,8 +114,8 @@ def call(request) # This signals that the ensure block below should not try to release the connection, because it's bound into the response which will be returned: connection = nil return response - rescue Protocol::RequestFailed - # This is a specific case where the entire request wasn't sent before a failure occurred. So, we can even resend non-idempotent requests. + rescue ::Protocol::HTTP::RefusedError + # This is a specific case where the request was not processed by the server. So, we can resend even non-idempotent requests. if connection @pool.release(connection) connection = nil diff --git a/lib/async/http/protocol/http1/client.rb b/lib/async/http/protocol/http1/client.rb index f202723..105a2ca 100644 --- a/lib/async/http/protocol/http1/client.rb +++ b/lib/async/http/protocol/http1/client.rb @@ -38,22 +38,17 @@ def call(request, task: Task.current) headers = request.headers.header # We carefully interpret https://tools.ietf.org/html/rfc7230#section-6.3.1 to implement this correctly. - begin - target = request.path - authority = request.authority - - # If we are using a CONNECT request, we need to use the authority as the target: - if request.connect? - target = authority - authority = nil - end - - write_request(authority, request.method, target, @version, headers) - rescue - # If we fail to fully write the request and body, we can retry this request. - raise RequestFailed + target = request.path + authority = request.authority + + # If we are using a CONNECT request, we need to use the authority as the target: + if request.connect? + target = authority + authority = nil end + write_request(authority, request.method, target, @version, headers) + if request.body? body = request.body diff --git a/lib/async/http/protocol/http2/response.rb b/lib/async/http/protocol/http2/response.rb index 53bb7b0..359554e 100644 --- a/lib/async/http/protocol/http2/response.rb +++ b/lib/async/http/protocol/http2/response.rb @@ -261,7 +261,7 @@ def send_request(request) begin @stream.send_headers(headers) rescue - raise RequestFailed + raise ::Protocol::HTTP::RefusedError end @stream.send_body(request.body, trailer) diff --git a/lib/async/http/protocol/request.rb b/lib/async/http/protocol/request.rb index 7a7156a..d88814d 100644 --- a/lib/async/http/protocol/request.rb +++ b/lib/async/http/protocol/request.rb @@ -11,10 +11,6 @@ module Async module HTTP module Protocol - # Failed to send the request. The request body has NOT been consumed (i.e. #read) and you should retry the request. - class RequestFailed < StandardError - end - # An incoming HTTP request generated by server protocol implementations. class Request < ::Protocol::HTTP::Request # @returns [Connection | Nil] The underlying protocol connection. diff --git a/releases.md b/releases.md index f5f88fa..c1da621 100644 --- a/releases.md +++ b/releases.md @@ -1,5 +1,12 @@ # Releases +## Unreleased + + - Use `Protocol::HTTP::RefusedError` for safe retry of requests not processed by the server, including non-idempotent methods like PUT. + - Remove `Async::HTTP::Protocol::RequestFailed` in favour of `Protocol::HTTP::RefusedError`. + - HTTP/1: Delegate request write failure handling to `protocol-http1`. + - HTTP/2: Handle GOAWAY and REFUSED_STREAM via `protocol-http2`, enabling automatic retry of unprocessed requests. + ## v0.94.3 - Fix response body leak in HTTP/2 server when stream is reset before `send_response` completes (e.g. client-side gRPC cancellation). The response body's `close` was never called, leaking any resources tied to body lifecycle (such as `rack.response_finished` callbacks and utilization metrics). diff --git a/test/async/http/protocol/http2/request_failed_on_refused_stream.rb b/test/async/http/protocol/http2/request_failed_on_refused_stream.rb new file mode 100644 index 0000000..a928c04 --- /dev/null +++ b/test/async/http/protocol/http2/request_failed_on_refused_stream.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +# Released under the MIT License. +# Copyright, 2026, by Samuel Williams. + +require "async/http/protocol/http2" +require "sus/fixtures/async/http" + +describe Async::HTTP::Protocol::HTTP2 do + with "REFUSED_STREAM converts to RefusedError" do + include Sus::Fixtures::Async::HTTP::ServerContext + let(:protocol) {subject} + + let(:request_count) {Async::Variable.new} + + let(:app) do + request_count = self.request_count + count = 0 + + Protocol::HTTP::Middleware.for do |request| + count += 1 + request_count.value = count + + Protocol::HTTP::Response[200, {}, ["OK"]] + end + end + + it "retries non-idempotent request" do + response = client.put("/", {}, ["Hello"]) + expect(response).to be(:success?) + + count = Async::Task.current.with_timeout(1.0){request_count.wait} + expect(count).to be == 1 + end + end +end