Skip to content

Commit d867965

Browse files
Revert "Fix body duplication in streaming requests on retry (#1895) (#1900)"
This reverts commit 9be387a.
1 parent 0b4dd06 commit d867965

File tree

5 files changed

+8
-71
lines changed

5 files changed

+8
-71
lines changed

react_on_rails_pro/lib/react_on_rails_pro/request.rb

Lines changed: 8 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,7 @@ class Request # rubocop:disable Metrics/ClassLength
99
class << self
1010
def reset_connection
1111
@connection&.close
12-
@connection_without_retries&.close
1312
@connection = create_connection
14-
@connection_without_retries = create_connection(enable_retries: false)
1513
end
1614

1715
def render_code(path, js_code, send_bundle)
@@ -84,29 +82,17 @@ def asset_exists_on_vm_renderer?(filename)
8482

8583
private
8684

87-
# NOTE: We maintain two separate HTTP connection pools to handle streaming vs non-streaming requests.
88-
# This doubles the memory footprint (e.g., if renderer_http_pool_size is 10, we use 20 total connections).
89-
# This tradeoff is acceptable to prevent body duplication in streaming responses.
90-
9185
def connection
9286
@connection ||= create_connection
9387
end
9488

95-
def connection_without_retries
96-
@connection_without_retries ||= create_connection(enable_retries: false)
97-
end
98-
99-
def perform_request(path, **post_options) # rubocop:disable Metrics/AbcSize,Metrics/CyclomaticComplexity,Metrics/PerceivedComplexity
100-
# For streaming requests, use connection without retries to prevent body duplication
101-
# The StreamRequest class handles retries properly by starting fresh requests
102-
conn = post_options[:stream] ? connection_without_retries : connection
103-
89+
def perform_request(path, **post_options) # rubocop:disable Metrics/AbcSize,Metrics/CyclomaticComplexity
10490
available_retries = ReactOnRailsPro.configuration.renderer_request_retry_limit
10591
retry_request = true
10692
while retry_request
10793
begin
10894
start_time = Time.now
109-
response = conn.post(path, **post_options)
95+
response = connection.post(path, **post_options)
11096
raise response.error if response.is_a?(HTTPX::ErrorResponse)
11197

11298
request_time = Time.now - start_time
@@ -231,20 +217,17 @@ def common_form_data
231217
ReactOnRailsPro::Utils.common_form_data
232218
end
233219

234-
def create_connection(enable_retries: true)
220+
def create_connection
235221
url = ReactOnRailsPro.configuration.renderer_url
236222
Rails.logger.info do
237223
"[ReactOnRailsPro] Setting up Node Renderer connection to #{url}"
238224
end
239225

240-
http_client = HTTPX
241-
# For persistent connections we want retries,
242-
# so the requests don't just fail if the other side closes the connection
243-
# https://honeyryderchuck.gitlab.io/httpx/wiki/Persistent
244-
# However, for streaming requests, retries cause body duplication
245-
# See https://github.com/shakacode/react_on_rails/issues/1895
246-
http_client = http_client.plugin(:retries, max_retries: 1, retry_change_requests: true) if enable_retries
247-
http_client
226+
HTTPX
227+
# For persistent connections we want retries,
228+
# so the requests don't just fail if the other side closes the connection
229+
# https://honeyryderchuck.gitlab.io/httpx/wiki/Persistent
230+
.plugin(:retries, max_retries: 1, retry_change_requests: true)
248231
.plugin(:stream)
249232
# See https://www.rubydoc.info/gems/httpx/1.3.3/HTTPX%2FOptions:initialize for the available options
250233
.with(

react_on_rails_pro/lib/react_on_rails_pro/stream_request.rb

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,6 @@ def each_chunk(&block)
7575

7676
send_bundle = false
7777
error_body = +""
78-
# Retry logic for streaming requests is handled here by starting fresh requests.
79-
# The HTTPx connection used for streaming has retries disabled (see Request#connection_without_retries)
80-
# to prevent body duplication when partial chunks are already sent to the client.
8178
loop do
8279
stream_response = @request_executor.call(send_bundle)
8380

react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/TestingStreamableComponent.jsx

Lines changed: 0 additions & 15 deletions
This file was deleted.

react_on_rails_pro/spec/dummy/spec/helpers/react_on_rails_pro_helper_spec.rb

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,6 @@ def response; end
330330
def mock_request_and_response(mock_chunks = chunks, count: 1)
331331
# Reset connection instance variables to ensure clean state for tests
332332
ReactOnRailsPro::Request.instance_variable_set(:@connection, nil)
333-
ReactOnRailsPro::Request.instance_variable_set(:@connection_without_retries, nil)
334333
original_httpx_plugin = HTTPX.method(:plugin)
335334
allow(HTTPX).to receive(:plugin) do |*args|
336335
original_httpx_plugin.call(:mock_stream).plugin(*args)

react_on_rails_pro/spec/react_on_rails_pro/request_spec.rb

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -194,32 +194,5 @@
194194
expect(mocked_block).not_to have_received(:call)
195195
end
196196
end
197-
198-
it "does not use HTTPx retries plugin for streaming requests to prevent body duplication" do
199-
# This test verifies the fix for https://github.com/shakacode/react_on_rails/issues/1895
200-
# When streaming requests encounter connection errors mid-transmission, HTTPx retries
201-
# would cause body duplication because partial chunks are already sent to the client.
202-
# The StreamRequest class handles retries properly by starting fresh requests.
203-
204-
# Reset connections to ensure we're using a fresh connection
205-
described_class.reset_connection
206-
207-
# Trigger a streaming request
208-
mock_streaming_response(render_full_url, 200) do |yielder|
209-
yielder.call("Test chunk\n")
210-
end
211-
212-
stream = described_class.render_code_as_stream("/render", "console.log('test');", is_rsc_payload: false)
213-
chunks = []
214-
stream.each_chunk { |chunk| chunks << chunk }
215-
216-
# Verify that the streaming request completed successfully
217-
expect(chunks).to eq(["Test chunk"])
218-
219-
# Verify that the connection_without_retries was created
220-
# by checking that a connection was created with retries disabled
221-
connection_without_retries = described_class.send(:connection_without_retries)
222-
expect(connection_without_retries).to be_a(HTTPX::Session)
223-
end
224197
end
225198
end

0 commit comments

Comments
 (0)