diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c6d318fd4..8c1cbb4edc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,8 @@ Changes since the last non-beta release. - **Use as Git dependency**: All packages can now be installed as Git dependencies. This is useful for development and testing purposes. See [CONTRIBUTING.md](./CONTRIBUTING.md#git-dependencies) for documentation. [PR #1873](https://github.com/shakacode/react_on_rails/pull/1873) by [alexeyr-ci2](https://github.com/alexeyr-ci2). +- **Body Duplication Bug On Streaming**: Fixed a bug that happens while streaming if the node renderer connection closed after streaming some chunks to the client. [PR #1995](https://github.com/shakacode/react_on_rails/pull/1995) by [AbanoubGhadban](https://github.com/AbanoubGhadban). + #### Breaking Changes - **React on Rails Core Package**: Several Pro-only methods have been removed from the core package and are now exclusively available in the `react-on-rails-pro` package. If you're using any of the following methods, you'll need to migrate to React on Rails Pro: diff --git a/lib/react_on_rails/helper.rb b/lib/react_on_rails/helper.rb index 92f4e0c375..8e1ec893d1 100644 --- a/lib/react_on_rails/helper.rb +++ b/lib/react_on_rails/helper.rb @@ -587,6 +587,15 @@ def server_rendered_react_component(render_options) # It doesn't make any transformation, it listens and raises error if a chunk has errors chunk_json_result end + + result.rescue do |err| + # This error came from the renderer + raise ReactOnRails::PrerenderError.new(component_name: react_component_name, + # Sanitize as this might be browser logged + props: sanitized_props_string(props), + err: err, + js_code: js_code) + end elsif result["hasErrors"] && render_options.raise_on_prerender_error raise_prerender_error(result, react_component_name, props, js_code) end diff --git a/react_on_rails_pro/lib/react_on_rails_pro/request.rb b/react_on_rails_pro/lib/react_on_rails_pro/request.rb index 8720cb39bf..cfe2c7ed53 100644 --- a/react_on_rails_pro/lib/react_on_rails_pro/request.rb +++ b/react_on_rails_pro/lib/react_on_rails_pro/request.rb @@ -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) @@ -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 @@ -231,20 +217,41 @@ 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, + # Official HTTPx docs says that we should use the retry_on option to decide if teh request should be retried or not + # However, HTTPx assumes that connection errors such as timeout error should be retried by default and it doesn't consider retry_on block at all at that case + # So, we have to do the following trick to avoid retries when a Timeout error happens while streaming a component + # If the streamed component returned any chunks, it shouldn't retry on errors, as it would cause page duplication + # The SSR-generated html will be written to the page two times in this case + retry_after: ->(request, response) do + if (request.stream.instance_variable_get(:@react_on_rails_received_first_chunk)) + e = response.error + raise ReactOnRailsPro::Error, "An error happened during server side render streaming of a component.\n" \ + "Original error:\n#{e}\n#{e.backtrace}" + end + + Rails.logger.info do + "[ReactOnRailsPro] An error happneding while making a request to the Node Renderer.\n" \ + "Error: #{response.error}.\n" \ + "Retrying by HTTPX \"retries\" plugin..." + end + # The retry_after block expects to return a delay to wait before retrying the request + # nil means no waiting delay + nil + end + ) .plugin(:stream) # See https://www.rubydoc.info/gems/httpx/1.3.3/HTTPX%2FOptions:initialize for the available options .with( diff --git a/react_on_rails_pro/lib/react_on_rails_pro/stream_request.rb b/react_on_rails_pro/lib/react_on_rails_pro/stream_request.rb index b9c3bf9bad..e0accae5cb 100644 --- a/react_on_rails_pro/lib/react_on_rails_pro/stream_request.rb +++ b/react_on_rails_pro/lib/react_on_rails_pro/stream_request.rb @@ -10,6 +10,7 @@ def initialize(component) # @param position [Symbol] The position of the chunk in the stream (:first, :middle, or :last) # The position parameter is used by actions that add content to the beginning or end of the stream @actions = [] # List to store all actions + @rescue_blocks = [] end # Add a prepend action @@ -39,27 +40,45 @@ def append self # Return self to allow chaining end + def rescue(&block) + @rescue_blocks << block + self # Return self to allow chaining + end + def handle_chunk(chunk, position) @actions.reduce(chunk) do |acc, action| action.call(acc, position) end end - def each_chunk - return enum_for(:each_chunk) unless block_given? + def each_chunk(&block) + return enum_for(:each_chunk) unless block first_chunk = true @component.each_chunk do |chunk| position = first_chunk ? :first : :middle modified_chunk = handle_chunk(chunk, position) - yield modified_chunk + block.call(modified_chunk) first_chunk = false end # The last chunk contains the append content after the transformation # All transformations are applied to the append content last_chunk = handle_chunk("", :last) - yield last_chunk unless last_chunk.empty? + block.call(last_chunk) unless last_chunk.empty? + rescue StandardError => err + current_error = err + rescue_block_index = 0 + while current_error.present? && (rescue_block_index < @rescue_blocks.size) + begin + @rescue_blocks[rescue_block_index].call(current_error, &block) + current_error = nil + rescue StandardError => inner_error + current_error = inner_error + end + rescue_block_index += 1 + end + raise current_error if current_error.present? end end @@ -75,9 +94,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) @@ -89,6 +105,9 @@ def each_chunk(&block) break rescue HTTPX::HTTPError => e send_bundle = handle_http_error(e, error_body, send_bundle) + rescue HTTPX::ReadTimeoutError => e + raise ReactOnRailsPro::Error, "Time out error while server side render streaming a component.\n" \ + "Original error:\n#{e}\n#{e.backtrace}" end end @@ -135,6 +154,7 @@ def loop_response_lines(response) line = "".b response.each do |chunk| + response.instance_variable_set(:@react_on_rails_received_first_chunk, true) line << chunk while (idx = line.index("\n")) diff --git a/react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/TestingStreamableComponent.jsx b/react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/TestingStreamableComponent.jsx deleted file mode 100644 index bd9076ef68..0000000000 --- a/react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/TestingStreamableComponent.jsx +++ /dev/null @@ -1,15 +0,0 @@ -'use client'; - -import React from 'react'; - -// Simple test component for streaming SSR tests -function TestingStreamableComponent({ helloWorldData }) { - return ( -