From b82e57c2810ea56bcb5a078ea225de16d39e79f2 Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Sun, 19 Nov 2023 22:35:42 +0000 Subject: [PATCH 1/2] ad test for integration of webmock with follow_redirects and stream plugins --- integration_tests/webmock_test.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/integration_tests/webmock_test.rb b/integration_tests/webmock_test.rb index 540724cd..08be0521 100644 --- a/integration_tests/webmock_test.rb +++ b/integration_tests/webmock_test.rb @@ -214,6 +214,17 @@ class WebmockTest < Minitest::Test assert_not_requested(:get, "http://#{httpbin}") end + def test_webmock_follow_redirects_with_stream_plugin + session = HTTPX.plugin(:follow_redirects).plugin(:stream) + redirect_url = "#{MOCK_URL_HTTP}/redirect" + initial_request = stub_request(:get, MOCK_URL_HTTP).to_return(status: 302, headers: { location: redirect_url }) + redirect_request = stub_request(:get, redirect_url) + + session.get(MOCK_URL_HTTP, stream: true).each.to_a.join + assert_requested(initial_request) + assert_requested(redirect_request) + end + private def assert_raise_with_message(e, message, &block) From ee66b7e5ccb8b2835bcbe5d98606dd739a76abc5 Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Sun, 19 Nov 2023 23:58:27 +0000 Subject: [PATCH 2/2] stream plugin fix: do not preempt request while stream requests are lazy, they were being nonetheless enqueued, before any function would be called. this was not great behaviour, as they could perhaps never been called, it also interfered with how other plugins inferred finished responses, such as the webmock adapter and follow_redirects. Another flaw in the grpc plugin was fixed as a result, given that bidirectional streams were actually being buffered --- lib/httpx/plugins/grpc.rb | 2 +- lib/httpx/plugins/grpc/grpc_encoding.rb | 9 +++++++-- lib/httpx/plugins/stream.rb | 25 ++++++------------------- sig/plugins/grpc/grpc_encoding.rbs | 6 +++++- sig/plugins/stream.rbs | 7 ++++++- 5 files changed, 25 insertions(+), 24 deletions(-) diff --git a/lib/httpx/plugins/grpc.rb b/lib/httpx/plugins/grpc.rb index 9ca6c561..24aa1637 100644 --- a/lib/httpx/plugins/grpc.rb +++ b/lib/httpx/plugins/grpc.rb @@ -215,7 +215,7 @@ module HTTPX **opts) grpc_request = build_grpc_request(rpc_method, input, deadline: deadline, metadata: metadata, **opts) response = request(grpc_request, **opts) - response.raise_for_status + response.raise_for_status unless opts[:stream] GRPC::Call.new(response) end diff --git a/lib/httpx/plugins/grpc/grpc_encoding.rb b/lib/httpx/plugins/grpc/grpc_encoding.rb index 70064bac..b6db45c4 100644 --- a/lib/httpx/plugins/grpc/grpc_encoding.rb +++ b/lib/httpx/plugins/grpc/grpc_encoding.rb @@ -36,7 +36,6 @@ module HTTPX class Inflater def initialize(response) - @encodings = response.headers.get("grpc-encoding") @response = response end @@ -49,7 +48,7 @@ module HTTPX encoded_data = message.byteslice(5..size + 5 - 1) if compressed == 1 - @encodings.reverse_each do |encoding| + grpc_encodings.reverse_each do |encoding| decoder = @response.body.class.initialize_inflater_by_encoding(encoding, @response, bytesize: encoded_data.bytesize) encoded_data = decoder.call(encoded_data) @@ -68,6 +67,12 @@ module HTTPX data end + + private + + def grpc_encodings + @grpc_encodings ||= @response.headers.get("grpc-encoding") + end end def self.encode(*args, **kwargs) diff --git a/lib/httpx/plugins/stream.rb b/lib/httpx/plugins/stream.rb index f1965274..79463585 100644 --- a/lib/httpx/plugins/stream.rb +++ b/lib/httpx/plugins/stream.rb @@ -2,10 +2,9 @@ module HTTPX class StreamResponse - def initialize(request, session, connections) + def initialize(request, session) @request = request @session = session - @connections = connections end def each(&block) @@ -16,19 +15,9 @@ module HTTPX begin @on_chunk = block - if @request.response - # if we've already started collecting the payload, yield it first - # before proceeding - body = @request.response.body - - body.each do |chunk| - on_chunk(chunk) - end - end - response.raise_for_status - response.close ensure + response.close if @response @on_chunk = nil end end @@ -69,9 +58,9 @@ module HTTPX private def response - @session.__send__(:receive_requests, [@request], @connections) until @request.response - - @request.response + @response ||= begin + @request.response || @session.request(@request) + end end def respond_to_missing?(meth, *args) @@ -105,9 +94,7 @@ module HTTPX request = requests.first - connections = _send_requests(requests) - - StreamResponse.new(request, self, connections) + StreamResponse.new(request, self) end end diff --git a/sig/plugins/grpc/grpc_encoding.rbs b/sig/plugins/grpc/grpc_encoding.rbs index 159a73e2..cfd3cfa4 100644 --- a/sig/plugins/grpc/grpc_encoding.rbs +++ b/sig/plugins/grpc/grpc_encoding.rbs @@ -21,11 +21,15 @@ module HTTPX class Inflater @response: Response - @encodings: Array[String] + @grpc_encodings: Array[String] def initialize: (Response | StreamResponse response) -> void def call: (String message) ?{ (String) -> void } -> String + + private + + def grpc_encodings: () -> Array[String] end end diff --git a/sig/plugins/stream.rbs b/sig/plugins/stream.rbs index e5bf2b43..0c56dddd 100644 --- a/sig/plugins/stream.rbs +++ b/sig/plugins/stream.rbs @@ -2,6 +2,10 @@ module HTTPX class StreamResponse include _ToS + @request: Request & RequestMethods + @session: sessionStream + @on_chunk: ^(String) -> void | nil + def each: () { (String) -> void } -> void | () -> Enumerable[String] @@ -10,10 +14,11 @@ module HTTPX def on_chunk: (string) -> void + def initialize: (Request, Session) -> void + private def response: () -> response - def initialize: (Request, Session, Array[Connection]) -> untyped end module Plugins