Make the `child_process` error catching more robust (`uncaughtException`) (#51)

Split off from https://github.com/matrix-org/matrix-public-archive/pull/43

Listen to `process.on('uncaughtException', ...)` and handle the async errors ourselves so it no longer fails the child process.

And if the process does exit with status code 1 (error), we have those underlying errors serialized and shown.

config/config.default.json

@@ -4,6 +4,7 @@
   "matrixServerUrl": "http://localhost:8008/",
   "archiveMessageLimit": 500,
   "requestTimeoutMs": 25000,
+  "logOutputFromChildProcesses": false,
   "UNCOMMENT_jaegerTracesEndpoint": "http://localhost:14268/api/traces",
   "testMatrixServerUrl1": "http://localhost:11008/",
   "testMatrixServerUrl2": "http://localhost:12008/",

server/hydrogen-render/1-render-hydrogen-to-string.js

@@ -10,11 +10,20 @@ const assert = require('assert');
 const RethrownError = require('../lib/rethrown-error');
 const { traceFunction } = require('../tracing/trace-utilities');
 
+const config = require('../lib/config');
+const logOutputFromChildProcesses = config.get('logOutputFromChildProcesses');
+
 // The render should be fast. If it's taking more than 5 seconds, something has
 // gone really wrong.
 const RENDER_TIMEOUT = 5000;
 
-async function renderHydrogenToString(options) {
+if (!logOutputFromChildProcesses) {
+  console.warn(
+    `Silencing logs from child processes (config.logOutputFromChildProcesses = ${logOutputFromChildProcesses})`
+  );
+}
+
+async function renderHydrogenToString(renderOptions) {
   try {
     let data = '';
     let childErrors = [];
@@ -25,36 +34,56 @@ async function renderHydrogenToString(options) {
     // we receive the SSR results.
     const child = fork(require.resolve('./2-render-hydrogen-to-string-fork-script'), [], {
       signal,
+      // Default to silencing logs from the child process. We already have
+      // proper instrumentation of any errors that might occur.
+      //
+      // This also makes `child.stderr` and `child.stdout` available
+      silent: true,
       //cwd: process.cwd(),
     });
 
-    // Pass the options to the child by sending instead of via argv because we
+    // 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) {
+        console.log('Child printed something to stdout:', String(data));
+      });
+
+      child.stderr.on('data', function (data) {
+        console.log('Child printed something to stderr:', String(data));
+      });
+    }
+
+    // Pass the renderOptions to the child by sending instead of via argv because we
     // will run into `Error: spawn E2BIG` and `Error: spawn ENAMETOOLONG` with
     // argv.
-    child.send(options);
+    child.send(renderOptions);
 
     // Stops the child process if it takes too long
     setTimeout(() => {
       controller.abort();
     }, RENDER_TIMEOUT);
 
-    // Collect the data passed back by the child
-    child.on('message', function (result) {
-      if (result.error) {
-        // De-serialize the error
-        const childError = new Error();
-        childError.name = result.name;
-        childError.message = result.message;
-        childError.stack = result.stack;
-        // We shouldn't really run into a situation where there are multiple
-        // errors but since this is just a message bus, it's possible.
-        childErrors.push(childError);
-      } else {
-        data += result.data;
-      }
-    });
-
     await new Promise((resolve, reject) => {
+      // Collect the data passed back by the child
+      child.on('message', function (result) {
+        if (result.error) {
+          // De-serialize the error
+          const childError = new Error();
+          childError.name = result.name;
+          childError.message = result.message;
+          childError.stack = result.stack;
+          // When an error happens while rendering Hydrogen, we only expect one
+          // error to come through here from the main line to render Hydrogen.
+          // But it's possible to get multiple errors from async out of context
+          // places since we also listen to `uncaughtException` and
+          // `unhandledRejection`.
+          childErrors.push(childError);
+        } else {
+          data += result.data;
+        }
+      });
+
       child.on('close', (exitCode) => {
         // Exited successfully
         if (exitCode === 0) {
@@ -65,13 +94,20 @@ async function renderHydrogenToString(options) {
             extraErrorsMessage = ` (somehow we saw ${
               childErrors.length
             } errors but we really always expect 1 error)\n${childErrors
-              .map((childError, index) => ` ${index}. ${childError.message} ${childError.stack}`)
+              .map((childError, index) => `${index}. ${childError.stack}`)
               .join('\n')}`;
           }
 
+          let childErrorToDisplay = new Error('No child errors');
+          if (childErrors.length === 1) {
+            childErrorToDisplay = childErrors[0];
+          } else if (childErrors.length > 1) {
+            childErrorToDisplay = new Error('Multiple child errors listed above ^');
+          }
+
           const error = new RethrownError(
             `Child process failed with exit code ${exitCode}${extraErrorsMessage}`,
-            childErrors[0] || new Error('No child errors')
+            childErrorToDisplay
           );
           reject(error);
         }
@@ -99,7 +135,7 @@ async function renderHydrogenToString(options) {
   } catch (err) {
     throw new RethrownError(
       `Failed to render Hydrogen to string. In order to reproduce, feed in these arguments into \`renderHydrogenToString(...)\`:\n    renderToString arguments: ${JSON.stringify(
-        arguments[0]
+        renderOptions
       )}`,
       err
     );

server/hydrogen-render/2-render-hydrogen-to-string-fork-script.js

@@ -6,14 +6,60 @@
 
 const assert = require('assert');
 
+const RethrownError = require('../lib/rethrown-error');
 const _renderHydrogenToStringUnsafe = require('./3-render-hydrogen-to-string-unsafe');
 
+// Serialize the error and send it back up to the parent process so we can
+// interact with it and know what happened when the process exits.
+async function serializeError(err) {
+  await new Promise((resolve) => {
+    process.send(
+      {
+        error: true,
+        name: err.name,
+        message: err.message,
+        stack: err.stack,
+      },
+      (sendErr) => {
+        if (sendErr) {
+          // We just log here instead of rejecting because it's more important
+          // to see the original error we are trying to send up. Let's just
+          // throw the original error below.
+          const sendErrWithDescription = new RethrownError(
+            'Failed to send error to the parent process',
+            sendErr
+          );
+          console.error(sendErrWithDescription);
+          // This will end up hitting the `unhandledRejection` handler and
+          // serializing this error instead (worth a shot) 🤷‍♀️
+          throw sendErrWithDescription;
+        }
+
+        resolve();
+      }
+    );
+  });
+}
+
+// We don't exit the process after encountering one of these because maybe it
+// doesn't matter to the main render process in Hydrogen.
+//
+// If we don't listen for these events, the child will exit with status code 1
+// (error) when they occur.
+process.on('uncaughtException', async (err /*, origin*/) => {
+  await serializeError(new RethrownError('uncaughtException in child process', err));
+});
+
+process.on('unhandledRejection', async (reason /*, promise*/) => {
+  await serializeError(new RethrownError('unhandledRejection in child process', reason));
+});
+
 // Only kick everything off once we receive the options. We pass in the options
 // this way instead of argv because we will run into `Error: spawn E2BIG` and
 // `Error: spawn ENAMETOOLONG` with argv.
-process.on('message', async (options) => {
+process.on('message', async (renderOptions) => {
   try {
-    const resultantHtml = await _renderHydrogenToStringUnsafe(options);
+    const resultantHtml = await _renderHydrogenToStringUnsafe(renderOptions);
 
     assert(resultantHtml, `No HTML returned from _renderHydrogenToStringUnsafe.`);
 
@@ -41,14 +87,10 @@ process.on('message', async (options) => {
       );
     });
   } catch (err) {
-    // Serialize the error and send it back up to the parent process so we can
+    // We need to wait for the error to completely send to the parent
-    // interact with it and know what happened when the process exits.
+    // process before we throw the error again and exit the process.
-    process.send({
+    await serializeError(err);
-      error: true,
+
-      name: err.name,
-      message: err.message,
-      stack: err.stack,
-    });
     // Throw the error so the process fails and exits
     throw err;
   }

server/hydrogen-render/3-render-hydrogen-to-string-unsafe.js

@@ -87,7 +87,8 @@ async function _renderHydrogenToStringUnsafe({ fromTimestamp, roomData, events,
   const hydrogenRenderScript = new vm.Script(hydrogenRenderScriptCode, {
     filename: '4-hydrogen-vm-render-script.js',
   });
-  // Note: The VM does not exit after the result is returned here
+  // Note: The VM does not exit after the result is returned here and is why
+  // this should be run in a `child_process` that we can exit.
   const vmResult = hydrogenRenderScript.runInContext(vmContext);
   // Wait for everything to render
   // (waiting on the promise returned from `4-hydrogen-vm-render-script.js`)

server/lib/rethrown-error.js

@@ -40,7 +40,7 @@ class RethrownError extends ExtendedError {
     const messageLines = (this.message.match(/\n/g) || []).length + 1;
 
     const indentedOriginalError = error.stack
-      .split('\n')
+      .split(/\r?\n/)
       .map((line) => `    ${line}`)
       .join('\n');
 

server/routes/install-routes.js

@@ -182,6 +182,16 @@ function installRoutes(app) {
         throw new Error('TODO: Redirect user to smaller hour range');
       }
 
+      // In development, if you're running into a hard to track down error with
+      // the render hydrogen stack and fighting against the multiple layers of
+      // complexity with `child_process `and `vm`; you can get away with removing
+      // the `child_process` part of it by using
+      // `3-render-hydrogen-to-string-unsafe` directly.
+      // ```js
+      // const _renderHydrogenToStringUnsafe = require('../hydrogen-render/3-render-hydrogen-to-string-unsafe');
+      // const hydrogenHtmlOutput = await _renderHydrogenToStringUnsafe({ /* renderData */ });
+      // ```
+      //
       const hydrogenHtmlOutput = await renderHydrogenToString({
         fromTimestamp,
         roomData,