From f3318446f8b71e2e43329ac599ac33f548835169 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 1 May 2023 17:33:48 -0500 Subject: [PATCH] Expose child errors that only occur in stderr log output (#205) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Who knows why we can't capture these errors via the more conventional `child.on('error', (err) => { })` listener 🤷 ### Before ``` RethrownError: Failed to render Hydrogen to string. In order to reproduce, feed in these arguments into `renderHydrogenToString(...)`: renderHydrogenToString arguments: { ... } at renderHydrogenToString (server/hydrogen-render/render-hydrogen-to-string.js:58:11) --- Original Error --- RethrownError: Child process exited with code 1 at assembleErrorAfterChildExitsWithErrors (server/child-process-runner/run-in-child-process.js:60:29) --- Original Error --- No child errors ``` ### After ``` RethrownError: Failed to render Hydrogen to string. In order to reproduce, feed in these arguments into `renderHydrogenToString(...)`: renderHydrogenToString arguments: { ... } at renderHydrogenToString (server/hydrogen-render/render-hydrogen-to-string.js:58:11) --- Original Error --- RethrownError: Child process exited with code 1 at assembleErrorAfterChildExitsWithErrors (server/child-process-runner/run-in-child-process.js:60:29) --- Original Error --- No child errors but there might be something in stderr=node:internal/modules/cjs/loader:936 throw err; ^ Error: Cannot find module '../lib/rethrown-error' Require stack: - server/child-process-runner/child-fork-script.js at Function.Module._resolveFilename (node:internal/modules/cjs/loader:933:15) at Function.Module._load (node:internal/modules/cjs/loader:778:27) at Module.require (node:internal/modules/cjs/loader:1005:19) at require (node:internal/modules/cjs/helpers:102:18) at Object. (server/child-process-runner/child-fork-script.js:8:23) at Module._compile (node:internal/modules/cjs/loader:1103:14) at Object.Module._extensions..js (node:internal/modules/cjs/loader:1155:10) at Module.load (node:internal/modules/cjs/loader:981:32) at Function.Module._load (node:internal/modules/cjs/loader:822:12) at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:77:12) { code: 'MODULE_NOT_FOUND', requireStack: [ 'server//child-process-runner//child-fork-script.js' ] } ``` --- .../run-in-child-process.js | 37 ++++++++++++------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/server/child-process-runner/run-in-child-process.js b/server/child-process-runner/run-in-child-process.js index 7006f72..2c1232c 100644 --- a/server/child-process-runner/run-in-child-process.js +++ b/server/child-process-runner/run-in-child-process.js @@ -26,7 +26,7 @@ if (!logOutputFromChildProcesses) { const resolvedChildForkScriptPath = require.resolve('./child-fork-script'); -function assembleErrorAfterChildExitsWithErrors(exitCode, childErrors) { +function assembleErrorAfterChildExitsWithErrors(exitCode, childErrors, childStdErr) { assert(childErrors); let extraErrorsMessage = ''; @@ -40,7 +40,9 @@ function assembleErrorAfterChildExitsWithErrors(exitCode, childErrors) { let childErrorToDisplay; if (childErrors.length === 0) { - childErrorToDisplay = new Error('No child errors'); + childErrorToDisplay = new Error( + `No child errors but there might be something in stderr=${childStdErr}` + ); // Clear the stack trace part of the stack string out because this is just a // note about the lack of errors, not an actual error and is just noisy with // that extra fluff. @@ -68,6 +70,7 @@ async function runInChildProcess(modulePath, runArguments, { timeout }) { try { let childErrors = []; let childExitCode = '(not set yet)'; + let childStdErr = ''; const controller = new AbortController(); const { signal } = controller; @@ -85,16 +88,18 @@ async function runInChildProcess(modulePath, runArguments, { timeout }) { }); // Since we have to use the `silent` option for the `stderr` stuff below, we - // should also print out the `stdout` to our main console. - if (logOutputFromChildProcesses) { - child.stdout.on('data', function (data) { + // should also print out the `stdout` to our main console if we want to see what's going on. + child.stdout.on('data', function (data) { + if (logOutputFromChildProcesses) { console.log('Child printed something to stdout:', String(data)); - }); - - child.stderr.on('data', function (data) { + } + }); + child.stderr.on('data', function (data) { + if (logOutputFromChildProcesses) { console.log('Child printed something to stderr:', String(data)); - }); - } + } + childStdErr += data; + }); // Pass the runArguments to the child by sending instead of via argv because // we will run into `Error: spawn E2BIG` and `Error: spawn ENAMETOOLONG` @@ -137,7 +142,8 @@ async function runInChildProcess(modulePath, runArguments, { timeout }) { } else { const childErrorSummary = assembleErrorAfterChildExitsWithErrors( childExitCode, - childErrors + childErrors, + childStdErr ); reject(childErrorSummary); } @@ -148,7 +154,8 @@ async function runInChildProcess(modulePath, runArguments, { timeout }) { if (err.name === 'AbortError') { const childErrorSummary = assembleErrorAfterChildExitsWithErrors( childExitCode, - childErrors + childErrors, + childStdErr ); reject( new RethrownError( @@ -163,7 +170,11 @@ async function runInChildProcess(modulePath, runArguments, { timeout }) { }); if (!returnedData) { - const childErrorSummary = assembleErrorAfterChildExitsWithErrors(childExitCode, childErrors); + const childErrorSummary = assembleErrorAfterChildExitsWithErrors( + childExitCode, + childErrors, + childStdErr + ); throw new RethrownError( `No \`returnedData\` sent from child process while running the module (${modulePath}). Any child errors? (${childErrors.length})`, childErrorSummary