Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 8 additions & 25 deletions react_on_rails_pro/lib/react_on_rails_pro/request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@ class Request # rubocop:disable Metrics/ClassLength
class << self
def reset_connection
@connection&.close
@connection_without_retries&.close
@connection = create_connection
@connection_without_retries = create_connection(enable_retries: false)
end

def render_code(path, js_code, send_bundle)
Expand Down Expand Up @@ -84,29 +82,17 @@ def asset_exists_on_vm_renderer?(filename)

private

# NOTE: We maintain two separate HTTP connection pools to handle streaming vs non-streaming requests.
# This doubles the memory footprint (e.g., if renderer_http_pool_size is 10, we use 20 total connections).
# This tradeoff is acceptable to prevent body duplication in streaming responses.

def connection
@connection ||= create_connection
end

def connection_without_retries
@connection_without_retries ||= create_connection(enable_retries: false)
end

def perform_request(path, **post_options) # rubocop:disable Metrics/AbcSize,Metrics/CyclomaticComplexity,Metrics/PerceivedComplexity
# For streaming requests, use connection without retries to prevent body duplication
# The StreamRequest class handles retries properly by starting fresh requests
conn = post_options[:stream] ? connection_without_retries : connection

def perform_request(path, **post_options) # rubocop:disable Metrics/AbcSize,Metrics/CyclomaticComplexity
available_retries = ReactOnRailsPro.configuration.renderer_request_retry_limit
retry_request = true
while retry_request
begin
start_time = Time.now
response = conn.post(path, **post_options)
response = connection.post(path, **post_options)
raise response.error if response.is_a?(HTTPX::ErrorResponse)

request_time = Time.now - start_time
Expand Down Expand Up @@ -231,20 +217,17 @@ def common_form_data
ReactOnRailsPro::Utils.common_form_data
end

def create_connection(enable_retries: true)
def create_connection
url = ReactOnRailsPro.configuration.renderer_url
Rails.logger.info do
"[ReactOnRailsPro] Setting up Node Renderer connection to #{url}"
end

http_client = HTTPX
# For persistent connections we want retries,
# so the requests don't just fail if the other side closes the connection
# https://honeyryderchuck.gitlab.io/httpx/wiki/Persistent
# However, for streaming requests, retries cause body duplication
# See https://github.com/shakacode/react_on_rails/issues/1895
http_client = http_client.plugin(:retries, max_retries: 1, retry_change_requests: true) if enable_retries
http_client
HTTPX
# For persistent connections we want retries,
# so the requests don't just fail if the other side closes the connection
# https://honeyryderchuck.gitlab.io/httpx/wiki/Persistent
.plugin(:retries, max_retries: 1, retry_change_requests: true)
.plugin(:stream)
# See https://www.rubydoc.info/gems/httpx/1.3.3/HTTPX%2FOptions:initialize for the available options
.with(
Expand Down
3 changes: 0 additions & 3 deletions react_on_rails_pro/lib/react_on_rails_pro/stream_request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,6 @@ def each_chunk(&block)

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

Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,6 @@ def response; end
def mock_request_and_response(mock_chunks = chunks, count: 1)
# Reset connection instance variables to ensure clean state for tests
ReactOnRailsPro::Request.instance_variable_set(:@connection, nil)
ReactOnRailsPro::Request.instance_variable_set(:@connection_without_retries, nil)
original_httpx_plugin = HTTPX.method(:plugin)
allow(HTTPX).to receive(:plugin) do |*args|
original_httpx_plugin.call(:mock_stream).plugin(*args)
Expand Down
27 changes: 0 additions & 27 deletions react_on_rails_pro/spec/react_on_rails_pro/request_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -194,32 +194,5 @@
expect(mocked_block).not_to have_received(:call)
end
end

it "does not use HTTPx retries plugin for streaming requests to prevent body duplication" do
# This test verifies the fix for https://github.com/shakacode/react_on_rails/issues/1895
# When streaming requests encounter connection errors mid-transmission, HTTPx retries
# would cause body duplication because partial chunks are already sent to the client.
# The StreamRequest class handles retries properly by starting fresh requests.

# Reset connections to ensure we're using a fresh connection
described_class.reset_connection

# Trigger a streaming request
mock_streaming_response(render_full_url, 200) do |yielder|
yielder.call("Test chunk\n")
end

stream = described_class.render_code_as_stream("/render", "console.log('test');", is_rsc_payload: false)
chunks = []
stream.each_chunk { |chunk| chunks << chunk }

# Verify that the streaming request completed successfully
expect(chunks).to eq(["Test chunk"])

# Verify that the connection_without_retries was created
# by checking that a connection was created with retries disabled
connection_without_retries = described_class.send(:connection_without_retries)
expect(connection_without_retries).to be_a(HTTPX::Session)
end
end
end
Loading