From a87c1e632c80d7212b71cb2be95c18fb6037d078 Mon Sep 17 00:00:00 2001 From: Abanoub Ghadban Date: Thu, 31 Oct 2024 19:22:59 +0300 Subject: [PATCH] Handle errors happen during streaming components (#1648) * handle errors happen during streaming components * remove debugging statements * linting * emit errors when an error happen and refactor * add unit tests for streamServerRenderedReactComponent * linting * make requested changes * make a condition simpler Co-authored-by: Alexey Romanov <104450112+alexeyr-ci@users.noreply.github.com> * fix variable name Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * tiny changes * linting * update CHANGELOG.md * rename ensureError function to convertToError * update CHANELOG.md * update CHANELOG.md * update CHANELOG.md --------- Co-authored-by: Alexey Romanov <104450112+alexeyr-ci@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- CHANGELOG.md | 15 +- lib/react_on_rails/helper.rb | 42 +++-- .../react_component/render_options.rb | 12 ++ .../src/serverRenderReactComponent.ts | 146 +++++++++++----- node_package/src/types/index.ts | 1 + ...treamServerRenderedReactComponent.test.jsx | 156 ++++++++++++++++++ package.json | 4 +- yarn.lock | 33 ++-- 8 files changed, 331 insertions(+), 78 deletions(-) create mode 100644 node_package/tests/streamServerRenderedReactComponent.test.jsx diff --git a/CHANGELOG.md b/CHANGELOG.md index e82c3d1fe..93dc9e73b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,15 +18,14 @@ Please follow the recommendations outlined at [keepachangelog.com](http://keepac ### [Unreleased] Changes since the last non-beta release. -### Added - - Added support for replaying console logs that occur during server rendering of streamed React components. This enables debugging of server-side rendering issues by capturing and displaying console output on the client and on the server output. [PR #1647](https://github.com/shakacode/react_on_rails/pull/1647) by [AbanoubGhadban](https://github.com/AbanoubGhadban). - -### Added +#### Added(https://github.com/AbanoubGhadban). - Added streaming server rendering support: - - New `stream_react_component` helper for adding streamed components to views - - New `streamServerRenderedReactComponent` function in the react-on-rails package that uses React 18's `renderToPipeableStream` API - - Enables progressive page loading and improved performance for server-rendered React components - [PR #1633](https://github.com/shakacode/react_on_rails/pull/1633) by [AbanoubGhadban](https://github.com/AbanoubGhadban). + - [PR #1633](https://github.com/shakacode/react_on_rails/pull/1633) by [AbanoubGhadban](https://github.com/AbanoubGhadban). + - New `stream_react_component` helper for adding streamed components to views + - New `streamServerRenderedReactComponent` function in the react-on-rails package that uses React 18's `renderToPipeableStream` API + - Enables progressive page loading and improved performance for server-rendered React components + - Added support for replaying console logs that occur during server rendering of streamed React components. This enables debugging of server-side rendering issues by capturing and displaying console output on the client and on the server output. [PR #1647](https://github.com/shakacode/react_on_rails/pull/1647) by [AbanoubGhadban](https://github.com/AbanoubGhadban). + - Added support for handling errors happening during server rendering of streamed React components. It handles errors that happen during the initial render and errors that happen inside suspense boundaries. [PR #1648](https://github.com/shakacode/react_on_rails/pull/1648) by [AbanoubGhadban](https://github.com/AbanoubGhadban). #### Changed - Console replay script generation now awaits the render request promise before generating, allowing it to capture console logs from asynchronous operations. This requires using a version of the Node renderer that supports replaying async console logs. [PR #1649](https://github.com/shakacode/react_on_rails/pull/1649) by [AbanoubGhadban](https://github.com/AbanoubGhadban). diff --git a/lib/react_on_rails/helper.rb b/lib/react_on_rails/helper.rb index 2a9e26c5d..1e179ad45 100644 --- a/lib/react_on_rails/helper.rb +++ b/lib/react_on_rails/helper.rb @@ -570,6 +570,25 @@ def props_string(props) props.is_a?(String) ? props : props.to_json end + def raise_prerender_error(json_result, react_component_name, props, js_code) + raise ReactOnRails::PrerenderError.new( + component_name: react_component_name, + props: sanitized_props_string(props), + err: nil, + js_code: js_code, + console_messages: json_result["consoleReplayScript"] + ) + end + + def should_raise_streaming_prerender_error?(chunk_json_result, render_options) + chunk_json_result["hasErrors"] && + (if chunk_json_result["isShellReady"] + render_options.raise_non_shell_server_rendering_errors + else + render_options.raise_on_prerender_error + end) + end + # Returns object with values that are NOT html_safe! def server_rendered_react_component(render_options) return { "html" => "", "consoleReplayScript" => "" } unless render_options.prerender @@ -617,19 +636,18 @@ def server_rendered_react_component(render_options) js_code: js_code) end - # TODO: handle errors for streams - return result if render_options.stream? - - if result["hasErrors"] && render_options.raise_on_prerender_error - # We caught this exception on our backtrace handler - raise ReactOnRails::PrerenderError.new(component_name: react_component_name, - # Sanitize as this might be browser logged - props: sanitized_props_string(props), - err: nil, - js_code: js_code, - console_messages: result["consoleReplayScript"]) - + if render_options.stream? + result.transform do |chunk_json_result| + if should_raise_streaming_prerender_error?(chunk_json_result, render_options) + raise_prerender_error(chunk_json_result, react_component_name, props, js_code) + end + # It doesn't make any transformation, it listens and raises error if a chunk has errors + chunk_json_result + end + elsif result["hasErrors"] && render_options.raise_on_prerender_error + raise_prerender_error(result, react_component_name, props, js_code) end + result end diff --git a/lib/react_on_rails/react_component/render_options.rb b/lib/react_on_rails/react_component/render_options.rb index 8bb8536ed..f93ba85c2 100644 --- a/lib/react_on_rails/react_component/render_options.rb +++ b/lib/react_on_rails/react_component/render_options.rb @@ -87,6 +87,10 @@ def raise_on_prerender_error retrieve_configuration_value_for(:raise_on_prerender_error) end + def raise_non_shell_server_rendering_errors + retrieve_react_on_rails_pro_config_value_for(:raise_non_shell_server_rendering_errors) + end + def logging_on_server retrieve_configuration_value_for(:logging_on_server) end @@ -128,6 +132,14 @@ def retrieve_configuration_value_for(key) ReactOnRails.configuration.public_send(key) end end + + def retrieve_react_on_rails_pro_config_value_for(key) + options.fetch(key) do + return nil unless ReactOnRails::Utils.react_on_rails_pro? + + ReactOnRailsPro.configuration.public_send(key) + end + end end end end diff --git a/node_package/src/serverRenderReactComponent.ts b/node_package/src/serverRenderReactComponent.ts index dd21e92d3..43d46bf23 100644 --- a/node_package/src/serverRenderReactComponent.ts +++ b/node_package/src/serverRenderReactComponent.ts @@ -1,5 +1,5 @@ -import ReactDOMServer from 'react-dom/server'; -import { PassThrough, Readable, Transform } from 'stream'; +import ReactDOMServer, { type PipeableStream } from 'react-dom/server'; +import { PassThrough, Readable } from 'stream'; import type { ReactElement } from 'react'; import ComponentRegistry from './ComponentRegistry'; @@ -15,6 +15,11 @@ type RenderState = { error?: RenderingError; }; +type StreamRenderState = Omit & { + result: null | Readable; + isShellReady: boolean; +}; + type RenderOptions = { componentName: string; domNodeId?: string; @@ -22,6 +27,10 @@ type RenderOptions = { renderingReturnsPromises: boolean; }; +function convertToError(e: unknown): Error { + return e instanceof Error ? e : new Error(String(e)); +} + function validateComponent(componentObj: RegisteredComponent, componentName: string) { if (componentObj.isRenderer) { throw new Error(`Detected a renderer while server rendering component '${componentName}'. See https://github.com/shakacode/react_on_rails#renderer-functions`); @@ -87,7 +96,7 @@ function handleRenderingError(e: unknown, options: { componentName: string, thro if (options.throwJsErrors) { throw e; } - const error = e instanceof Error ? e : new Error(String(e)); + const error = convertToError(e); return { hasErrors: true, result: handleError({ e: error, name: options.componentName, serverSide: true }), @@ -95,12 +104,13 @@ function handleRenderingError(e: unknown, options: { componentName: string, thro }; } -function createResultObject(html: string | null, consoleReplayScript: string, renderState: RenderState): RenderResult { +function createResultObject(html: string | null, consoleReplayScript: string, renderState: RenderState | StreamRenderState): RenderResult { return { html, consoleReplayScript, hasErrors: renderState.hasErrors, renderingError: renderState.error && { message: renderState.error.message, stack: renderState.error.stack }, + isShellReady: 'isShellReady' in renderState ? renderState.isShellReady : undefined, }; } @@ -195,17 +205,102 @@ const serverRenderReactComponent: typeof serverRenderReactComponentInternal = (o const stringToStream = (str: string): Readable => { const stream = new PassThrough(); - stream.push(str); - stream.push(null); + stream.write(str); + stream.end(); return stream; }; +const transformRenderStreamChunksToResultObject = (renderState: StreamRenderState) => { + const consoleHistory = console.history; + let previouslyReplayedConsoleMessages = 0; + + const transformStream = new PassThrough({ + transform(chunk, _, callback) { + const htmlChunk = chunk.toString(); + const consoleReplayScript = buildConsoleReplay(consoleHistory, previouslyReplayedConsoleMessages); + previouslyReplayedConsoleMessages = consoleHistory?.length || 0; + + const jsonChunk = JSON.stringify(createResultObject(htmlChunk, consoleReplayScript, renderState)); + + this.push(`${jsonChunk}\n`); + callback(); + } + }); + + let pipedStream: PipeableStream | null = null; + const pipeToTransform = (pipeableStream: PipeableStream) => { + pipeableStream.pipe(transformStream); + pipedStream = pipeableStream; + }; + // We need to wrap the transformStream in a Readable stream to properly handle errors: + // 1. If we returned transformStream directly, we couldn't emit errors into it externally + // 2. If an error is emitted into the transformStream, it would cause the render to fail + // 3. By wrapping in Readable.from(), we can explicitly emit errors into the readableStream without affecting the transformStream + // Note: Readable.from can merge multiple chunks into a single chunk, so we need to ensure that we can separate them later + const readableStream = Readable.from(transformStream); + + const writeChunk = (chunk: string) => transformStream.write(chunk); + const emitError = (error: unknown) => readableStream.emit('error', error); + const endStream = () => { + transformStream.end(); + pipedStream?.abort(); + } + return { readableStream, pipeToTransform, writeChunk, emitError, endStream }; +} + +const streamRenderReactComponent = (reactRenderingResult: ReactElement, options: RenderParams) => { + const { name: componentName, throwJsErrors } = options; + const renderState: StreamRenderState = { + result: null, + hasErrors: false, + isShellReady: false + }; + + const { + readableStream, + pipeToTransform, + writeChunk, + emitError, + endStream + } = transformRenderStreamChunksToResultObject(renderState); + + const renderingStream = ReactDOMServer.renderToPipeableStream(reactRenderingResult, { + onShellError(e) { + const error = convertToError(e); + renderState.hasErrors = true; + renderState.error = error; + + if (throwJsErrors) { + emitError(error); + } + + const errorHtml = handleError({ e: error, name: componentName, serverSide: true }); + writeChunk(errorHtml); + endStream(); + }, + onShellReady() { + renderState.isShellReady = true; + pipeToTransform(renderingStream); + }, + onError(e) { + if (!renderState.isShellReady) { + return; + } + const error = convertToError(e); + if (throwJsErrors) { + emitError(error); + } + renderState.hasErrors = true; + renderState.error = error; + }, + }); + + return readableStream; +} + export const streamServerRenderedReactComponent = (options: RenderParams): Readable => { const { name: componentName, domNodeId, trace, props, railsContext, throwJsErrors } = options; - let renderResult: null | Readable = null; - let previouslyReplayedConsoleMessages: number = 0; - try { const componentObj = ComponentRegistry.get(componentName); validateComponent(componentObj, componentName); @@ -222,40 +317,17 @@ export const streamServerRenderedReactComponent = (options: RenderParams): Reada throw new Error('Server rendering of streams is not supported for server render hashes or promises.'); } - const consoleHistory = console.history; - const transformStream = new Transform({ - transform(chunk, _, callback) { - const htmlChunk = chunk.toString(); - const consoleReplayScript = buildConsoleReplay(consoleHistory, previouslyReplayedConsoleMessages); - previouslyReplayedConsoleMessages = consoleHistory?.length || 0; - - const jsonChunk = JSON.stringify({ - html: htmlChunk, - consoleReplayScript, - }); - - this.push(jsonChunk); - callback(); - } - }); - - ReactDOMServer.renderToPipeableStream(reactRenderingResult).pipe(transformStream); - - renderResult = transformStream; + return streamRenderReactComponent(reactRenderingResult, options); } catch (e) { if (throwJsErrors) { throw e; } - const error = e instanceof Error ? e : new Error(String(e)); - renderResult = stringToStream(handleError({ - e: error, - name: componentName, - serverSide: true, - })); + const error = convertToError(e); + const htmlResult = handleError({ e: error, name: componentName, serverSide: true }); + const jsonResult = JSON.stringify(createResultObject(htmlResult, buildConsoleReplay(), { hasErrors: true, error, result: null })); + return stringToStream(jsonResult); } - - return renderResult; }; export default serverRenderReactComponent; diff --git a/node_package/src/types/index.ts b/node_package/src/types/index.ts index 2f808dc06..a8f7ddffc 100644 --- a/node_package/src/types/index.ts +++ b/node_package/src/types/index.ts @@ -138,6 +138,7 @@ export interface RenderResult { consoleReplayScript: string; hasErrors: boolean; renderingError?: RenderingError; + isShellReady?: boolean; } // from react-dom 18 diff --git a/node_package/tests/streamServerRenderedReactComponent.test.jsx b/node_package/tests/streamServerRenderedReactComponent.test.jsx new file mode 100644 index 000000000..276ff1243 --- /dev/null +++ b/node_package/tests/streamServerRenderedReactComponent.test.jsx @@ -0,0 +1,156 @@ +/** + * @jest-environment node + */ + +import React, { Suspense } from 'react'; +import PropTypes from 'prop-types'; +import { streamServerRenderedReactComponent } from '../src/serverRenderReactComponent'; +import ComponentRegistry from '../src/ComponentRegistry'; + +const AsyncContent = async ({ throwAsyncError }) => { + await new Promise((resolve) => setTimeout(resolve, 0)); + if (throwAsyncError) { + throw new Error('Async Error'); + } + return
Async Content
; +}; + +const TestComponentForStreaming = ({ throwSyncError, throwAsyncError }) => { + if (throwSyncError) { + throw new Error('Sync Error'); + } + + return ( +
+

Header In The Shell

+ Loading...
}> + + + + ); +}; + +TestComponentForStreaming.propTypes = { + throwSyncError: PropTypes.bool, + throwAsyncError: PropTypes.bool, +}; + +describe('streamServerRenderedReactComponent', () => { + beforeEach(() => { + ComponentRegistry.components().clear(); + }); + + const expectStreamChunk = (chunk) => { + expect(typeof chunk).toBe('string'); + const jsonChunk = JSON.parse(chunk); + expect(typeof jsonChunk.html).toBe('string'); + expect(typeof jsonChunk.consoleReplayScript).toBe('string'); + expect(typeof jsonChunk.hasErrors).toBe('boolean'); + expect(typeof jsonChunk.isShellReady).toBe('boolean'); + return jsonChunk; + }; + + const setupStreamTest = ({ + throwSyncError = false, + throwJsErrors = false, + throwAsyncError = false, + } = {}) => { + ComponentRegistry.register({ TestComponentForStreaming }); + const renderResult = streamServerRenderedReactComponent({ + name: 'TestComponentForStreaming', + domNodeId: 'myDomId', + trace: false, + props: { throwSyncError, throwAsyncError }, + throwJsErrors, + }); + + const chunks = []; + renderResult.on('data', (chunk) => { + const decodedText = new TextDecoder().decode(chunk); + chunks.push(expectStreamChunk(decodedText)); + }); + + return { renderResult, chunks }; + }; + + it('streamServerRenderedReactComponent streams the rendered component', async () => { + const { renderResult, chunks } = setupStreamTest(); + await new Promise((resolve) => renderResult.on('end', resolve)); + + expect(chunks).toHaveLength(2); + expect(chunks[0].html).toContain('Header In The Shell'); + expect(chunks[0].consoleReplayScript).toBe(''); + expect(chunks[0].hasErrors).toBe(false); + expect(chunks[0].isShellReady).toBe(true); + expect(chunks[1].html).toContain('Async Content'); + expect(chunks[1].consoleReplayScript).toBe(''); + expect(chunks[1].hasErrors).toBe(false); + expect(chunks[1].isShellReady).toBe(true); + }); + + it('emits an error if there is an error in the shell and throwJsErrors is true', async () => { + const { renderResult, chunks } = setupStreamTest({ throwSyncError: true, throwJsErrors: true }); + const onError = jest.fn(); + renderResult.on('error', onError); + await new Promise((resolve) => renderResult.on('end', resolve)); + + expect(onError).toHaveBeenCalled(); + expect(chunks).toHaveLength(1); + expect(chunks[0].html).toMatch(/
Exception in rendering[.\s\S]*Sync Error[.\s\S]*<\/pre>/);
+    expect(chunks[0].consoleReplayScript).toBe('');
+    expect(chunks[0].hasErrors).toBe(true);
+    expect(chunks[0].isShellReady).toBe(false);
+  });
+
+  it("doesn't emit an error if there is an error in the shell and throwJsErrors is false", async () => {
+    const { renderResult, chunks } = setupStreamTest({ throwSyncError: true, throwJsErrors: false });
+    const onError = jest.fn();
+    renderResult.on('error', onError);
+    await new Promise((resolve) => renderResult.on('end', resolve));
+
+    expect(onError).not.toHaveBeenCalled();
+    expect(chunks).toHaveLength(1);
+    expect(chunks[0].html).toMatch(/
Exception in rendering[.\s\S]*Sync Error[.\s\S]*<\/pre>/);
+    expect(chunks[0].consoleReplayScript).toBe('');
+    expect(chunks[0].hasErrors).toBe(true);
+    expect(chunks[0].isShellReady).toBe(false);
+  });
+
+  it('emits an error if there is an error in the async content and throwJsErrors is true', async () => {
+    const { renderResult, chunks } = setupStreamTest({ throwAsyncError: true, throwJsErrors: true });
+    const onError = jest.fn();
+    renderResult.on('error', onError);
+    await new Promise((resolve) => renderResult.on('end', resolve));
+
+    expect(onError).toHaveBeenCalled();
+    expect(chunks).toHaveLength(2);
+    expect(chunks[0].html).toContain('Header In The Shell');
+    expect(chunks[0].consoleReplayScript).toBe('');
+    expect(chunks[0].hasErrors).toBe(false);
+    expect(chunks[0].isShellReady).toBe(true);
+    // Script that fallbacks the render to client side
+    expect(chunks[1].html).toMatch(/