From 585e5dc669e95049ac6df2c4a5924f06b8538ee7 Mon Sep 17 00:00:00 2001 From: Abanoub Ghadban Date: Tue, 24 Sep 2024 17:07:55 +0300 Subject: [PATCH 1/4] add support for console replay of console logs happen in async operations get console replay messages after promise resolves or rejects use isPromise to check if the result is a promise make consoleReplay function accept the list of logs to be added to the script make consoleHistory argument of consoleReplay optional add comments add comments update comment remove FlowFixMe comment --- node_package/src/buildConsoleReplay.ts | 12 ++++--- .../src/serverRenderReactComponent.ts | 32 +++++++++++-------- 2 files changed, 25 insertions(+), 19 deletions(-) diff --git a/node_package/src/buildConsoleReplay.ts b/node_package/src/buildConsoleReplay.ts index c1fdafcea..f39cec428 100644 --- a/node_package/src/buildConsoleReplay.ts +++ b/node_package/src/buildConsoleReplay.ts @@ -9,13 +9,15 @@ declare global { } } -export function consoleReplay(): string { +export function consoleReplay(customConsoleHistory: typeof console['history'] | undefined = undefined): string { // console.history is a global polyfill used in server rendering. - if (!(console.history instanceof Array)) { + const consoleHistory = customConsoleHistory ?? console.history; + + if (!(Array.isArray(consoleHistory))) { return ''; } - const lines = console.history.map(msg => { + const lines = consoleHistory.map(msg => { const stringifiedList = msg.arguments.map(arg => { let val: string; try { @@ -42,6 +44,6 @@ export function consoleReplay(): string { return lines.join('\n'); } -export default function buildConsoleReplay(): string { - return RenderUtils.wrapInScriptTags('consoleReplayLog', consoleReplay()); +export default function buildConsoleReplay(customConsoleHistory: typeof console['history'] | undefined = undefined): string { + return RenderUtils.wrapInScriptTags('consoleReplayLog', consoleReplay(customConsoleHistory)); } diff --git a/node_package/src/serverRenderReactComponent.ts b/node_package/src/serverRenderReactComponent.ts index fec68e231..9974edc75 100644 --- a/node_package/src/serverRenderReactComponent.ts +++ b/node_package/src/serverRenderReactComponent.ts @@ -105,16 +105,22 @@ function createResultObject(html: string | null, consoleReplayScript: string, re async function createPromiseResult( renderState: RenderState & { result: Promise }, - consoleReplayScript: string, componentName: string, throwJsErrors: boolean ): Promise { + // Capture console history before awaiting the promise + // Node renderer will reset the global console.history after executing the synchronous part of the request. + // It resets it only if replayServerAsyncOperationLogs renderer config is set to false. + // In both cases, we need to keep a reference to console.history to avoid losing console logs in case of reset. + const consoleHistory = console.history; try { const html = await renderState.result; + const consoleReplayScript = buildConsoleReplay(consoleHistory); return createResultObject(html, consoleReplayScript, renderState); } catch (e: unknown) { const errorRenderState = handleRenderingError(e, { componentName, throwJsErrors }); - return createResultObject(errorRenderState.result, consoleReplayScript, errorRenderState); + const consoleReplayScript = buildConsoleReplay(consoleHistory); + return createResultObject(errorRenderState.result, consoleReplayScript, renderState); } } @@ -123,20 +129,12 @@ function createFinalResult( componentName: string, throwJsErrors: boolean ): null | string | Promise { - // Console history is stored globally in `console.history`. - // If node renderer is handling a render request that returns a promise, - // It can handle another request while awaiting the promise. - // To prevent cross-request console logs leakage between these requests, - // we build the consoleReplayScript before awaiting any promises. - // The console history is reset after the synchronous part of each request. - // This causes console logs happening during async operations to not be captured. - const consoleReplayScript = buildConsoleReplay(); - const { result } = renderState; if (isPromise(result)) { - return createPromiseResult({ ...renderState, result }, consoleReplayScript, componentName, throwJsErrors); + return createPromiseResult({ ...renderState, result }, componentName, throwJsErrors); } + const consoleReplayScript = buildConsoleReplay(); return JSON.stringify(createResultObject(result, consoleReplayScript, renderState)); } @@ -183,13 +181,19 @@ function serverRenderReactComponentInternal(options: RenderParams): null | strin } const serverRenderReactComponent: typeof serverRenderReactComponentInternal = (options) => { + let result: string | Promise | null = null; try { - return serverRenderReactComponentInternal(options); + result = serverRenderReactComponentInternal(options); } finally { // Reset console history after each render. // See `RubyEmbeddedJavaScript.console_polyfill` for initialization. - console.history = []; + // We don't need to clear the console history if the result is a promise + // Promises only supported in node renderer and node renderer takes care of cleanining console history + if (typeof result === 'string') { + console.history = []; + } } + return result; }; export default serverRenderReactComponent; From 2697ef729d209c71230a94a0522360ad4cdea92d Mon Sep 17 00:00:00 2001 From: Abanoub Ghadban Date: Tue, 15 Oct 2024 15:21:46 +0300 Subject: [PATCH 2/4] call clear console history from the generated render code --- lib/react_on_rails/server_rendering_js_code.rb | 1 + node_package/src/serverRenderReactComponent.ts | 18 +----------------- 2 files changed, 2 insertions(+), 17 deletions(-) diff --git a/lib/react_on_rails/server_rendering_js_code.rb b/lib/react_on_rails/server_rendering_js_code.rb index dc807b7a2..efd3494ce 100644 --- a/lib/react_on_rails/server_rendering_js_code.rb +++ b/lib/react_on_rails/server_rendering_js_code.rb @@ -40,6 +40,7 @@ def render(props_string, rails_context, redux_stores, react_component_name, rend var railsContext = #{rails_context}; #{redux_stores} var props = #{props_string}; + console.history = []; return ReactOnRails.serverRenderReactComponent({ name: '#{react_component_name}', domNodeId: '#{render_options.dom_id}', diff --git a/node_package/src/serverRenderReactComponent.ts b/node_package/src/serverRenderReactComponent.ts index 9974edc75..6749f64c4 100644 --- a/node_package/src/serverRenderReactComponent.ts +++ b/node_package/src/serverRenderReactComponent.ts @@ -138,7 +138,7 @@ function createFinalResult( return JSON.stringify(createResultObject(result, consoleReplayScript, renderState)); } -function serverRenderReactComponentInternal(options: RenderParams): null | string | Promise { +function serverRenderReactComponent(options: RenderParams): null | string | Promise { const { name: componentName, domNodeId, trace, props, railsContext, renderingReturnsPromises, throwJsErrors } = options; let renderState: RenderState = { @@ -180,20 +180,4 @@ function serverRenderReactComponentInternal(options: RenderParams): null | strin return createFinalResult(renderState, componentName, throwJsErrors); } -const serverRenderReactComponent: typeof serverRenderReactComponentInternal = (options) => { - let result: string | Promise | null = null; - try { - result = serverRenderReactComponentInternal(options); - } finally { - // Reset console history after each render. - // See `RubyEmbeddedJavaScript.console_polyfill` for initialization. - // We don't need to clear the console history if the result is a promise - // Promises only supported in node renderer and node renderer takes care of cleanining console history - if (typeof result === 'string') { - console.history = []; - } - } - return result; -}; - export default serverRenderReactComponent; From 4caf257829081cf29ab60e9096f94c4343c13089 Mon Sep 17 00:00:00 2001 From: Abanoub Ghadban Date: Sat, 19 Oct 2024 20:47:07 +0300 Subject: [PATCH 3/4] Revert "call clear console history from the generated render code" This reverts commit 2697ef729d209c71230a94a0522360ad4cdea92d. --- lib/react_on_rails/server_rendering_js_code.rb | 1 - node_package/src/serverRenderReactComponent.ts | 18 +++++++++++++++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/lib/react_on_rails/server_rendering_js_code.rb b/lib/react_on_rails/server_rendering_js_code.rb index efd3494ce..dc807b7a2 100644 --- a/lib/react_on_rails/server_rendering_js_code.rb +++ b/lib/react_on_rails/server_rendering_js_code.rb @@ -40,7 +40,6 @@ def render(props_string, rails_context, redux_stores, react_component_name, rend var railsContext = #{rails_context}; #{redux_stores} var props = #{props_string}; - console.history = []; return ReactOnRails.serverRenderReactComponent({ name: '#{react_component_name}', domNodeId: '#{render_options.dom_id}', diff --git a/node_package/src/serverRenderReactComponent.ts b/node_package/src/serverRenderReactComponent.ts index 6749f64c4..9974edc75 100644 --- a/node_package/src/serverRenderReactComponent.ts +++ b/node_package/src/serverRenderReactComponent.ts @@ -138,7 +138,7 @@ function createFinalResult( return JSON.stringify(createResultObject(result, consoleReplayScript, renderState)); } -function serverRenderReactComponent(options: RenderParams): null | string | Promise { +function serverRenderReactComponentInternal(options: RenderParams): null | string | Promise { const { name: componentName, domNodeId, trace, props, railsContext, renderingReturnsPromises, throwJsErrors } = options; let renderState: RenderState = { @@ -180,4 +180,20 @@ function serverRenderReactComponent(options: RenderParams): null | string | Prom return createFinalResult(renderState, componentName, throwJsErrors); } +const serverRenderReactComponent: typeof serverRenderReactComponentInternal = (options) => { + let result: string | Promise | null = null; + try { + result = serverRenderReactComponentInternal(options); + } finally { + // Reset console history after each render. + // See `RubyEmbeddedJavaScript.console_polyfill` for initialization. + // We don't need to clear the console history if the result is a promise + // Promises only supported in node renderer and node renderer takes care of cleanining console history + if (typeof result === 'string') { + console.history = []; + } + } + return result; +}; + export default serverRenderReactComponent; From 86d426282c8588c6781331f4bb832652904b5bf1 Mon Sep 17 00:00:00 2001 From: Abanoub Ghadban Date: Sat, 19 Oct 2024 20:50:39 +0300 Subject: [PATCH 4/4] add comment about clearing console history --- node_package/src/serverRenderReactComponent.ts | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/node_package/src/serverRenderReactComponent.ts b/node_package/src/serverRenderReactComponent.ts index 9974edc75..bbc1b53ef 100644 --- a/node_package/src/serverRenderReactComponent.ts +++ b/node_package/src/serverRenderReactComponent.ts @@ -181,19 +181,15 @@ function serverRenderReactComponentInternal(options: RenderParams): null | strin } const serverRenderReactComponent: typeof serverRenderReactComponentInternal = (options) => { - let result: string | Promise | null = null; try { - result = serverRenderReactComponentInternal(options); + return serverRenderReactComponentInternal(options); } finally { // Reset console history after each render. // See `RubyEmbeddedJavaScript.console_polyfill` for initialization. - // We don't need to clear the console history if the result is a promise - // Promises only supported in node renderer and node renderer takes care of cleanining console history - if (typeof result === 'string') { - console.history = []; - } + // This is necessary when ExecJS and old versions of node renderer are used. + // New versions of node renderer reset the console history automatically. + console.history = []; } - return result; }; export default serverRenderReactComponent;