Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 68 additions & 0 deletions web/packages/teleterm/src/mainProcess/ipcSerializer.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/**
* Teleport
* Copyright (C) 2025 Gravitational, Inc.
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

import { TshdRpcError } from 'teleterm/services/tshd/cloneableClient';

import { deserializeError, serializeError } from './ipcSerializer';

test('serializes and deserializes regular error', () => {
const err = new Error('Boom');
err.name = 'CustomError';

const serialized = serializeError(err);
expect(serialized instanceof Error).toBe(false);
expect(serialized.message).toBe('Boom');
expect(serialized.name).toBe('CustomError');
expect(serialized.stack).toBe(err.stack);
expect(serialized.cause).toBe(err.cause);

const cloned = structuredClone(serialized);
const deserialized = deserializeError(cloned);
expect(deserialized instanceof Error).toBe(true);
expect(deserialized.message).toBe('Boom');
expect(deserialized.name).toBe('CustomError');
expect(deserialized.stack).toBe(err.stack);
expect(deserialized.cause).toBe(err.cause);
});

test('serializes and deserializes tshd error', () => {
const err: TshdRpcError = {
name: 'TshdRpcError',
message: 'Could not found',
toString: () => 'Could not found',
cause: '',
stack: '',
code: 'NOT_FOUND',
isResolvableWithRelogin: false,
};

const serialized = serializeError(err);
expect(serialized instanceof Error).toBe(false);
expect(serialized.message).toBe('Could not found');
expect(serialized.name).toBe('TshdRpcError');
expect(serialized['isResolvableWithRelogin']).toBe(false);
expect(serialized['code']).toBe('NOT_FOUND');

const cloned = structuredClone(serialized);
const deserialized = deserializeError(cloned);
expect(deserialized instanceof Error).toBe(true);
expect(deserialized.message).toBe('Could not found');
expect(deserialized.name).toBe('TshdRpcError');
expect(deserialized['isResolvableWithRelogin']).toBe(false);
expect(deserialized['code']).toBe('NOT_FOUND');
});
34 changes: 26 additions & 8 deletions web/packages/teleterm/src/mainProcess/ipcSerializer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,23 +21,41 @@ export type SerializedError = {
message: string;
stack?: string;
cause?: unknown;
toStringResult?: string;
};

/** Serializes an Error into a plain object for transport through Electron IPC. */
export function serializeError(error: Error): SerializedError {
const {
name,
cause,
stack,
message,
// functions must be skipped, otherwise structuredClone will fail to clone the object
// eslint-disable-next-line unused-imports/no-unused-vars
toString,
...enumerableFields
} = error;
return {
name: error.name,
message: error.message,
cause: error.cause,
stack: error.stack,
name,
message,
cause,
stack,
// Calling the destructured function directly could result in the following error:
// Method Error.prototype.toString called on incompatible receiver undefined
toStringResult: error.toString?.(),
...enumerableFields,
};
}

/** Deserializes a plain object back into an Error instance. */
export function deserializeError(serialized: SerializedError): Error {
const error = new Error(serialized.message);
error.name = serialized.name;
error.cause = serialized.cause;
error.stack = serialized.stack;
const { name, cause, stack, message, toStringResult, ...rest } = serialized;
const error = new Error(message);
error.name = name;
error.cause = cause;
error.stack = stack;
error.toString = () => toStringResult;
Object.assign(error, rest);
return error;
}
73 changes: 47 additions & 26 deletions web/packages/teleterm/src/mainProcess/mainProcess.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {
app,
dialog,
ipcMain,
IpcMainInvokeEvent,
Menu,
MenuItemConstructorOptions,
nativeTheme,
Expand Down Expand Up @@ -415,17 +416,17 @@ export default class MainProcess {
event.returnValue = nativeTheme.shouldUseDarkColors;
});

ipcMain.handle('main-process-get-resolved-child-process-addresses', () => {
ipcHandle('main-process-get-resolved-child-process-addresses', () => {
return this.resolvedChildProcessAddresses;
});

ipcMain.handle('main-process-show-file-save-dialog', (_, filePath) =>
ipcHandle('main-process-show-file-save-dialog', (_, filePath) =>
dialog.showSaveDialog({
defaultPath: path.basename(filePath),
})
);

ipcMain.handle(
ipcHandle(
MainProcessIpc.SaveTextToFile,
async (
_,
Expand Down Expand Up @@ -464,7 +465,7 @@ export default class MainProcess {
}
);

ipcMain.handle(
ipcHandle(
MainProcessIpc.ForceFocusWindow,
async (
_,
Expand All @@ -485,7 +486,7 @@ export default class MainProcess {
// Used in the `tsh install` command on macOS to make the bundled tsh available in PATH.
// Returns true if tsh got successfully installed, false if the user closed the osascript
// prompt. Throws an error when osascript fails.
ipcMain.handle('main-process-symlink-tsh-macos', async () => {
ipcHandle('main-process-symlink-tsh-macos', async () => {
const source = this.settings.tshd.binaryPath;
const target = '/usr/local/bin/tsh';
const prompt =
Expand All @@ -507,7 +508,7 @@ export default class MainProcess {
}
});

ipcMain.handle('main-process-remove-tsh-symlink-macos', async () => {
ipcHandle('main-process-remove-tsh-symlink-macos', async () => {
const target = '/usr/local/bin/tsh';
const prompt =
'Teleport Connect wants to remove a symlink for tsh from /usr/local/bin.';
Expand All @@ -528,21 +529,21 @@ export default class MainProcess {
}
});

ipcMain.handle('main-process-open-config-file', async () => {
ipcHandle('main-process-open-config-file', async () => {
const path = this.configFileStorage.getFilePath();
await shell.openPath(path);
return path;
});

ipcMain.handle(MainProcessIpc.DownloadConnectMyComputerAgent, () =>
ipcHandle(MainProcessIpc.DownloadConnectMyComputerAgent, () =>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to test this by turning off my wifi while the agent was being downloaded and I got this:

Image
MAIN [fileDownloader] error: Error: Download failed: interrupted
    at DownloadItem.<anonymous> (/Users/rav/src/teleport/web/packages/teleterm/build/app/main/index.js:40820:29)
    at DownloadItem.emit (node:events:519:28)
Error occurred in handler for 'main-process-connect-my-computer-download-agent': TypeError: Method Error.prototype.toString called on incompatible receiver undefined
    at toString (<anonymous>)
    at serializeError (/Users/rav/src/teleport/web/packages/teleterm/build/app/main/index.js:55163:21)
    at /Users/rav/src/teleport/web/packages/teleterm/build/app/main/index.js:55987:23
    at process.processTicksAndRejections (node:internal/process/task_queues:105:5)
    at async Session.<anonymous> (node:electron/js2c/browser_init:2:107107)
(node:25525) UnhandledPromiseRejectionWarning: Error: Download failed: interrupted
    at DownloadItem.<anonymous> (/Users/rav/src/teleport/web/packages/teleterm/build/app/main/index.js:40820:29)
    at DownloadItem.emit (node:events:519:28)

Is this caused by this change? Would it be possible to add a test for that?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching it, turns out that calling destructured toString function can cause issues with this.

this.downloadAgentShared()
);

ipcMain.handle(MainProcessIpc.VerifyConnectMyComputerAgent, async () => {
ipcHandle(MainProcessIpc.VerifyConnectMyComputerAgent, async () => {
await verifyAgent(this.settings.agentBinaryPath);
});

ipcMain.handle(
ipcHandle(
'main-process-connect-my-computer-create-agent-config-file',
(_, args: CreateAgentConfigFileArgs) =>
createAgentConfigFile(this.settings, {
Expand All @@ -553,7 +554,7 @@ export default class MainProcess {
})
);

ipcMain.handle(
ipcHandle(
'main-process-connect-my-computer-is-agent-config-file-created',
async (
_,
Expand All @@ -563,7 +564,7 @@ export default class MainProcess {
) => isAgentConfigFileCreated(this.settings, args.rootClusterUri)
);

ipcMain.handle(
ipcHandle(
'main-process-connect-my-computer-kill-agent',
async (
_,
Expand All @@ -575,7 +576,7 @@ export default class MainProcess {
}
);

ipcMain.handle(
ipcHandle(
'main-process-connect-my-computer-remove-agent-directory',
(
_,
Expand All @@ -585,11 +586,11 @@ export default class MainProcess {
) => removeAgentDirectory(this.settings, args.rootClusterUri)
);

ipcMain.handle(MainProcessIpc.TryRemoveConnectMyComputerAgentBinary, () =>
ipcHandle(MainProcessIpc.TryRemoveConnectMyComputerAgentBinary, () =>
this.agentRunner.tryRemoveAgentBinary()
);

ipcMain.handle(
ipcHandle(
'main-process-connect-my-computer-run-agent',
async (
_,
Expand Down Expand Up @@ -625,7 +626,7 @@ export default class MainProcess {
}
);

ipcMain.handle(
ipcHandle(
'main-process-open-agent-logs-directory',
async (
_,
Expand All @@ -644,7 +645,7 @@ export default class MainProcess {
}
);

ipcMain.handle(
ipcHandle(
MainProcessIpc.SelectDirectoryForDesktopSession,
async (_, args: { desktopUri: string; login: string }) => {
const value = await dialog.showOpenDialog({
Expand Down Expand Up @@ -673,11 +674,11 @@ export default class MainProcess {
event.returnValue = this.appUpdater.supportsUpdates();
});

ipcMain.handle(MainProcessIpc.CheckForAppUpdates, () =>
ipcHandle(MainProcessIpc.CheckForAppUpdates, () =>
this.appUpdater.checkForUpdates()
);

ipcMain.handle(
ipcHandle(
MainProcessIpc.ChangeAppUpdatesManagingCluster,
(
event,
Expand All @@ -687,31 +688,31 @@ export default class MainProcess {
) => this.appUpdater.changeManagingCluster(args.clusterUri)
);

ipcMain.handle(MainProcessIpc.DownloadAppUpdate, () =>
ipcHandle(MainProcessIpc.DownloadAppUpdate, () =>
this.appUpdater.download()
);

ipcMain.handle(MainProcessIpc.CancelAppUpdateDownload, () =>
ipcHandle(MainProcessIpc.CancelAppUpdateDownload, () =>
this.appUpdater.cancelDownload()
);

ipcMain.handle(MainProcessIpc.QuiteAndInstallAppUpdate, () =>
ipcHandle(MainProcessIpc.QuiteAndInstallAppUpdate, () =>
this.appUpdater.quitAndInstall()
);

ipcMain.handle(MainProcessIpc.AddCluster, (ev, proxyAddress) =>
ipcHandle(MainProcessIpc.AddCluster, (ev, proxyAddress) =>
this.clusterLifecycleManager.addCluster(proxyAddress)
);

ipcMain.handle(MainProcessIpc.SyncRootClusters, () =>
ipcHandle(MainProcessIpc.SyncRootClusters, () =>
this.clusterLifecycleManager.syncRootClustersAndStartProfileWatcher()
);

ipcMain.handle(MainProcessIpc.SyncCluster, (_, args) =>
ipcHandle(MainProcessIpc.SyncCluster, (_, args) =>
this.clusterLifecycleManager.syncCluster(args.clusterUri)
);

ipcMain.handle(MainProcessIpc.Logout, async (_, args) => {
ipcHandle(MainProcessIpc.Logout, async (_, args) => {
await this.clusterLifecycleManager.logoutAndRemoveCluster(
args.clusterUri
);
Expand Down Expand Up @@ -1010,3 +1011,23 @@ function makeAppUpdaterStorage(fs: FileStorage): AppUpdaterStorage {
},
};
}

/**
* Handles requests sent via `ipcInvoke`.
* The renderer must send requests using `ipcInvoke` (not `ipcRenderer.invoke`).
*
* Use this instead of `ipcMain.handle`. It ensures full error serialization
* and prevents Electron from adding the generic message "Error invoking remote method".
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the JSDoc for ipcInvoke you mention ipcHandle which I think is pretty neat. We could mention ipcInvoke in the comment for ipcHandle too.

*/
function ipcHandle(
channel: string,
listener: (event: IpcMainInvokeEvent, ...args: any[]) => Promise<any> | any
): void {
ipcMain.handle(channel, async (...args) => {
try {
return { result: await Promise.try(listener, ...args) };
} catch (e) {
return { error: serializeError(e) };
}
});
}
Loading
Loading