diff --git a/web/packages/teleterm/src/main.ts b/web/packages/teleterm/src/main.ts index 1bee6575d321a..6e6a4e3411a96 100644 --- a/web/packages/teleterm/src/main.ts +++ b/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 { 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 { 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 { 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); +} diff --git a/web/packages/teleterm/src/mainProcess/mainProcess.ts b/web/packages/teleterm/src/mainProcess/mainProcess.ts index 3ab45ad12b3df..c480605a280c6 100644 --- a/web/packages/teleterm/src/mainProcess/mainProcess.ts +++ b/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 { @@ -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()], diff --git a/web/packages/teleterm/src/mainProcess/resolveNetworkAddress.ts b/web/packages/teleterm/src/mainProcess/resolveNetworkAddress.ts index ef039840f3ac4..ab3d62fc07bb0 100644 --- a/web/packages/teleterm/src/mainProcess/resolveNetworkAddress.ts +++ b/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,