[v18] Connect: Stop execution of main process on child process spawn error

web/packages/teleterm/src/main.ts

@@ -23,6 +23,7 @@ import path from 'node:path';
 import { app, dialog, globalShortcut, nativeTheme, shell } from 'electron';
 
 import { CUSTOM_PROTOCOL } from 'shared/deepLinks';
+import { ensureError } from 'shared/utils/error';
 
 import { parseDeepLink } from 'teleterm/deepLinks';
 import Logger from 'teleterm/logger';
@@ -83,19 +84,30 @@ async function initializeApp(): Promise<void> {
   const windowsManager = new WindowsManager(appStateFileStorage, settings);
 
   process.on('uncaughtException', (error, origin) => {
-    logger.error(origin, error);
-    app.quit();
+    logger.error('Uncaught exception', origin, error);
+    showDialogWithError(`Uncaught exception (${origin} origin)`, error);
+    app.exit(1);
   });
 
-  // init main process
-  const mainProcess = MainProcess.create({
-    settings,
-    logger,
-    configService,
-    appStateFileStorage,
-    configFileStorage,
-    windowsManager,
-  });
+  let mainProcess: MainProcess;
+  try {
+    mainProcess = MainProcess.create({
+      settings,
+      logger,
+      configService,
+      appStateFileStorage,
+      configFileStorage,
+      windowsManager,
+    });
+  } catch (error) {
+    const message = 'Could not initialize the main process';
+    logger.error(message, error);
+    showDialogWithError(message, error);
+    // app.exit(1) isn't equivalent to throwing an error, use an explicit return to stop further
+    // execution. See https://github.com/gravitational/teleport/issues/56272.
+    app.exit(1);
+    return;
+  }
 
   //TODO(gzdunek): Make sure this is not needed after migrating to Vite.
   app.on(
@@ -163,14 +175,10 @@ async function initializeApp(): Promise<void> {
       allowList: rootClusterProxyHostAllowList,
     });
   })().catch(error => {
-    const message =
-      'Could not initialize tsh daemon client in the main process';
+    const message = 'Could not initialize the tshd client in the main process';
     logger.error(message, error);
-    dialog.showErrorBox(
-      'Error during main process startup',
-      `${message}: ${error}`
-    );
-    app.quit();
+    showDialogWithError(message, error);
+    app.exit(1);
   });
 
   app
@@ -189,13 +197,10 @@ async function initializeApp(): Promise<void> {
       windowsManager.createWindow();
     })
     .catch(error => {
-      const message = 'Could not initialize the app';
+      const message = 'Could not create the main app window';
       logger.error(message, error);
-      dialog.showErrorBox(
-        'Error during app initialization',
-        `${message}: ${error}`
-      );
-      app.quit();
+      showDialogWithError(message, error);
+      app.exit(1);
     });
 
   // Limit navigation capabilities to reduce the attack surface.
@@ -418,3 +423,10 @@ function launchDeepLink(
   // Otherwise the app would receive focus but nothing would be visible in the UI.
   windowsManager.launchDeepLink(result);
 }
+
+function showDialogWithError(title: string, unknownError: unknown) {
+  const error = ensureError(unknownError);
+  // V8 includes the error message in the stack, so there's no need to append stack to message.
+  const content = error.stack || error.message;
+  dialog.showErrorBox(title, content);
+}

web/packages/teleterm/src/mainProcess/mainProcess.ts

@@ -137,6 +137,13 @@ export default class MainProcess {
     );
   }
 
+  /**
+   * create starts necessary child processes such as tsh daemon and the shared process. It also sets
+   * up IPC handlers and resolves the network addresses under which the child processes set up gRPC
+   * servers.
+   *
+   * create might throw an error if spawning a child process fails, see initTshd for more details.
+   */
   static create(opts: Options) {
     const instance = new MainProcess(opts);
     instance.init();
@@ -162,15 +169,10 @@ export default class MainProcess {
   private init() {
     this.updateAboutPanelIfNeeded();
     this.setAppMenu();
-    try {
-      this.initTshd();
-      this.initSharedProcess();
-      this.initResolvingChildProcessAddresses();
-      this.initIpc();
-    } catch (err) {
-      this.logger.error('Failed to start main process: ', err.message);
-      app.exit(1);
-    }
+    this.initTshd();
+    this.initSharedProcess();
+    this.initResolvingChildProcessAddresses();
+    this.initIpc();
   }
 
   async getTshdClient(): Promise<TshdClient> {
@@ -191,6 +193,15 @@ export default class MainProcess {
     const { binaryPath, homeDir } = this.settings.tshd;
     this.logger.info(`Starting tsh daemon from ${binaryPath}`);
 
+    // spawn might either fail immediately by throwing an error or cause the error event to be emitted
+    // on the process value returned by spawn.
+    //
+    // Some spawn failures result in an error being thrown immediately such as EPERM on Windows [1].
+    // This might be related to the fact that in Node.js, an error event causes an error to be
+    // thrown if there are no listeners for the error event itself [2].
+    //
+    // [1] https://stackoverflow.com/a/42262771
+    // [2] https://nodejs.org/docs/latest-v22.x/api/errors.html#error-propagation-and-interception
     this.tshdProcess = spawn(
       binaryPath,
       ['daemon', 'start', ...this.getTshdFlags()],

web/packages/teleterm/src/mainProcess/resolveNetworkAddress.ts

@@ -73,7 +73,7 @@ function waitForMatchInStdout(
     let chunks = '';
 
     const timeout = setTimeout(() => {
-      rejectOnError(
+      rejectWithCleanup(
         new ResolveError(requestedAddress, process, 'the operation timed out')
       );
     }, timeoutMs);
@@ -94,11 +94,23 @@ function waitForMatchInStdout(
       }
     };
 
-    const rejectOnError = (error: Error) => {
+    const rejectWithCleanup = (error: Error) => {
       reject(error);
       removeListeners();
     };
 
+    const rejectOnError = (error: Error) => {
+      const errorToReport = error.message.includes(process.spawnfile)
+        ? error
+        : // Attach spawnfile so that the process can be identified without looking at the stacktrace.
+          // resolveNetworkAddress is the only function that ends up surfacing to the UI the error
+          // event of a process.
+          new Error(`${process.spawnfile}: ${error.message}`, {
+            cause: error,
+          });
+      rejectWithCleanup(errorToReport);
+    };
+
     const rejectOnClose = (code: number, signal: NodeJS.Signals) => {
       const codeOrSignal = [
         // code can be 0, so we cannot just check it the same way as the signal.
@@ -108,7 +120,7 @@ function waitForMatchInStdout(
         .filter(Boolean)
         .join(' ');
       const details = codeOrSignal ? ` with ${codeOrSignal}` : '';
-      rejectOnError(
+      rejectWithCleanup(
         new ResolveError(
           requestedAddress,
           process,