feat(client/electron): propagate structured errors from Go to TypeScript

client/electron/go_vpn_tunnel.ts

@@ -21,7 +21,7 @@ import {ChildProcessHelper, ProcessTerminatedExitCodeError, ProcessTerminatedSig
 import {RoutingDaemon} from './routing_service';
 import {VpnTunnel} from './vpn_tunnel';
 import {ShadowsocksSessionConfig, TunnelStatus} from '../src/www/app/tunnel';
-import {ErrorCode, fromErrorCode, UnexpectedPluginError} from '../src/www/model/errors';
+import {ErrorCode} from '../src/www/model/errors';
 
 const isLinux = platform() === 'linux';
 const isWindows = platform() === 'win32';
@@ -47,6 +47,7 @@ const DNS_RESOLVERS = ['1.1.1.1', '9.9.9.9'];
 // about the others.
 export class GoVpnTunnel implements VpnTunnel {
   private readonly tun2socks: GoTun2socks;
+  private readonly connectivityChecker: GoTun2socks;
 
   // See #resumeListener.
   private disconnected = false;
@@ -62,6 +63,7 @@ export class GoVpnTunnel implements VpnTunnel {
 
   constructor(private readonly routing: RoutingDaemon, private config: ShadowsocksSessionConfig) {
     this.tun2socks = new GoTun2socks(config);
+    this.connectivityChecker = new GoTun2socks(config);
 
     // This promise, tied to both helper process' exits, is key to the instance's
     // lifecycle:
@@ -79,6 +81,7 @@ export class GoVpnTunnel implements VpnTunnel {
   // processes
   enableDebugMode() {
     this.tun2socks.enableDebugMode();
+    this.connectivityChecker.enableDebugMode();
   }
 
   // Fulfills once all three helpers have started successfully.
@@ -96,15 +99,12 @@ export class GoVpnTunnel implements VpnTunnel {
     });
 
     if (checkProxyConnectivity) {
-      this.isUdpEnabled = await checkConnectivity(this.config);
+      this.isUdpEnabled = await checkConnectivity(this.connectivityChecker);
     }
     console.log(`UDP support: ${this.isUdpEnabled}`);
 
-    // Don't await here because we want to launch both binaries
-    this.tun2socks.start(this.isUdpEnabled);
-
     console.log('starting routing daemon');
-    await this.routing.start();
+    await Promise.all([this.tun2socks.start(this.isUdpEnabled), this.routing.start()]);
   }
 
   networkChanged(status: TunnelStatus) {
@@ -131,24 +131,24 @@ export class GoVpnTunnel implements VpnTunnel {
     console.log('stopped tun2socks in preparation for suspend');
   }
 
-  private resumeListener() {
+  private async resumeListener() {
     if (this.disconnected) {
       // NOTE: Cannot remove resume listeners - Electron bug?
       console.error('resume event invoked but this tunnel is terminated - doing nothing');
       return;
     }
 
     console.log('restarting tun2socks after resume');
-    this.tun2socks.start(this.isUdpEnabled);
-
-    // Check if UDP support has changed; if so, silently restart.
-    this.updateUdpSupport();
+    await Promise.all([
+      this.tun2socks.start(this.isUdpEnabled),
+      this.updateUdpSupport(), // Check if UDP support has changed; if so, silently restart.
+    ]);
   }
 
   private async updateUdpSupport() {
     const wasUdpEnabled = this.isUdpEnabled;
     try {
-      this.isUdpEnabled = await checkConnectivity(this.config);
+      this.isUdpEnabled = await checkConnectivity(this.connectivityChecker);
     } catch (e) {
       console.error(`connectivity check failed: ${e}`);
       return;
@@ -161,7 +161,7 @@ export class GoVpnTunnel implements VpnTunnel {
 
     // Restart tun2socks.
     await this.tun2socks.stop();
-    this.tun2socks.start(this.isUdpEnabled);
+    await this.tun2socks.start(this.isUdpEnabled);
   }
 
   // Use #onceDisconnected to be notified when the tunnel terminates.
@@ -216,13 +216,22 @@ export class GoVpnTunnel implements VpnTunnel {
 // outline-go-tun2socks is a Go program that processes IP traffic from a TUN/TAP device
 // and relays it to a Shadowsocks proxy server.
 class GoTun2socks {
+  // Resolved when Tun2socks prints "tun2socks running" to stdout
+  // Call `monitorStarted` to set this field
+  private whenStarted: Promise<void>;
   private stopRequested = false;
   private readonly process: ChildProcessHelper;
 
   constructor(private readonly config: ShadowsocksSessionConfig) {
     this.process = new ChildProcessHelper(pathToEmbeddedTun2socksBinary());
   }
 
+  /**
+   * Starts tun2socks process, and waits for it to launch successfully.
+   * Success is confirmed when the phrase "tun2socks running" is detected in the `stdout`.
+   * Otherwise, an error containing a JSON-formatted message will be thrown.
+   * @param isUdpEnabled Indicates whether the remote Shadowsocks server supports UDP.
+   */
   async start(isUdpEnabled: boolean): Promise<void> {
     // ./tun2socks.exe \
     //   -tunName outline-tap0 -tunDNS 1.1.1.1,9.9.9.9 \
@@ -246,27 +255,46 @@ class GoTun2socks {
       args.push('-dnsFallback');
     }
 
-    this.stopRequested = false;
-    let autoRestart = false;
-    do {
-      if (autoRestart) {
-        console.warn(`tun2socks exited unexpectedly. Restarting...`);
-      }
-      autoRestart = false;
-      this.process.onStdErr = (data?: string | Buffer) => {
+    const whenProcessEnded = this.launchWithAutoRestart(args);
+
+    // Either started successfully, or terminated exceptionally
+    return Promise.race([this.whenStarted, whenProcessEnded]);
+  }
+
+  private monitorStarted(): Promise<void> {
+    return (this.whenStarted = new Promise(resolve => {
+      this.process.onStdOut = (data?: string | Buffer) => {
         if (data?.toString().includes('tun2socks running')) {
-          console.debug('tun2socks started');
-          autoRestart = true;
-          this.process.onStdErr = undefined;
+          console.debug('[tun2socks] - started');
+          this.process.onStdOut = null;
+          resolve();
         }
       };
+    }));
+  }
+
+  private async launchWithAutoRestart(args: string[]): Promise<void> {
+    console.debug('[tun2socks] - starting to route network traffic ...');
+    let restarting = false;
+    let lastError: Error | null = null;
+    do {
+      if (restarting) {
+        console.warn('[tun2socks] - exited unexpectedly; restarting ...');
+      }
+      restarting = false;
+      this.monitorStarted().then(() => (restarting = true));
       try {
+        lastError = null;
         await this.process.launch(args);
-        console.info('tun2socks exited with no errors');
+        console.info('[tun2socks] - exited with no errors');
       } catch (e) {
-        console.error(`tun2socks terminated due to ${e}`);
+        console.error('[tun2socks] - terminated due to:', e);
+        lastError = e;
       }
-    } while (!this.stopRequested && autoRestart);
+    } while (!this.stopRequested && restarting);
+    if (lastError) {
+      throw lastError;
+    }
   }
 
   stop() {
@@ -280,7 +308,7 @@ class GoTun2socks {
    * -tun* and -dnsFallback options have no effect on this mode.
    */
   checkConnectivity() {
-    console.debug('using tun2socks to check connectivity');
+    console.debug('[tun2socks] - checking connectivity ...');
     return this.process.launch([
       '-proxyHost',
       this.config.host || '',
@@ -305,18 +333,17 @@ class GoTun2socks {
 // `config`. Checks whether proxy server is reachable, whether the network and proxy support UDP
 // forwarding and validates the proxy credentials. Resolves with a boolean indicating whether UDP
 // forwarding is supported. Throws if the checks fail or if the process fails to start.
-async function checkConnectivity(config: ShadowsocksSessionConfig) {
+async function checkConnectivity(tun2socks: GoTun2socks) {
   try {
-    await new GoTun2socks(config).checkConnectivity();
+    await tun2socks.checkConnectivity();
     return true;
   } catch (e) {
-    console.error(`connectivity check error: ${e}`);
+    console.error('connectivity check error:', e);
     if (e instanceof ProcessTerminatedExitCodeError) {
       if (e.exitCode === ErrorCode.UDP_RELAY_NOT_ENABLED) {
         return false;
       }
-      throw fromErrorCode(e.exitCode);
     }
-    throw new UnexpectedPluginError();
+    throw e;
   }
 }

client/electron/index.ts

@@ -24,7 +24,7 @@ import autoLaunch = require('auto-launch'); // tslint:disable-line
 import {app, BrowserWindow, ipcMain, Menu, MenuItemConstructorOptions, nativeImage, shell, Tray} from 'electron';
 import {autoUpdater} from 'electron-updater';
 
-import {lookupIp} from "./connectivity";
+import {lookupIp} from './connectivity';
 import {GoVpnTunnel} from './go_vpn_tunnel';
 import {installRoutingServices, RoutingDaemon} from './routing_service';
 import {TunnelStore, SerializableTunnel} from './tunnel_store';
@@ -142,7 +142,7 @@ function setupWindow(): void {
   // The ideal solution would be: either electron-builder supports the app icon; or we add
   // dpi-aware features to this app.
   if (isLinux) {
-    mainWindow.setIcon(path.join(app.getAppPath(), 'output', 'client', 'electron', 'icons', 'png', '64x64.png'));
+    mainWindow.setIcon(path.join(app.getAppPath(), 'client', 'electron', 'icons', 'png', '64x64.png'));
   }
 
   const pathToIndexHtml = path.join(app.getAppPath(), 'client', 'www', 'index_electron.html');
@@ -450,11 +450,17 @@ function main() {
     mainWindow?.webContents.send('outline-ipc-push-clipboard');
   });
 
-  // Connects to the specified server.
+  // Connects to a proxy server specified by a config.
+  //
+  // If any issues occur, an Error will be thrown, which you can try-catch around
+  // `ipcRenderer.invoke`. But you should avoid depending on the specific error type.
+  // Instead, you should use its message property (which would probably be a JSON representation
+  // of a PlatformError). See https://github.com/electron/electron/issues/24427.
+  //
   // TODO: refactor channel name and namespace to a constant
   ipcMain.handle(
     'outline-ipc-start-proxying',
-    async (_, args: {config: ShadowsocksSessionConfig; id: string}): Promise<errors.ErrorCode> => {
+    async (_, args: {config: ShadowsocksSessionConfig; id: string}): Promise<void> => {
       // TODO: Rather than first disconnecting, implement a more efficient switchover (as well as
       //       being faster, this would help prevent traffic leaks - the Cordova clients already do
       //       this).
@@ -480,13 +486,11 @@ function main() {
         tunnelStore.save(args).catch(() => {
           console.error('Failed to store tunnel.');
         });
-
-        return errors.ErrorCode.NO_ERROR;
       } catch (e) {
-        console.error(`could not connect: ${e.name} (${e.message})`);
+        console.error('could not connect:', e);
         // clean up the state, no need to await because stopVpn might throw another error which can be ignored
         stopVpn();
-        return errors.toErrorCode(e);
+        throw e;
       }
     }
   );

client/electron/preload.ts

@@ -46,8 +46,25 @@ export class ElectronRendererMethodChannel {
 
   // TODO: replace the `any` with a better type once we unify the IPC call framework
   /* eslint-disable  @typescript-eslint/no-explicit-any */
-  readonly invoke = (channel: string, ...args: unknown[]): Promise<any> =>
-    ipcRenderer.invoke(`${this.namespace}-${channel}`, ...args);
+  readonly invoke = async (channel: string, ...args: unknown[]): Promise<any> => {
+    const ipcName = `${this.namespace}-${channel}`;
+    try {
+      await ipcRenderer.invoke(ipcName, ...args);
+    } catch (e) {
+      // Normalize the error message to what's being thrown in the IPC itself
+      //   e.message == "Error invoking remote method 'xxx': <error name>: <actual message>"
+      // https://github.com/electron/electron/blob/v31.0.0/lib/renderer/api/ipc-renderer.ts#L22
+      if (typeof e?.message === 'string') {
+        const errPattern = new RegExp(`'${ipcName}':\\s*(?<name>[^:]+):\\s*(?<message>.*)`, 's');
+        const groups = e.message.match(errPattern)?.groups;
+        if (typeof groups?.['name'] === 'string' && typeof groups?.['message'] === 'string') {
+          e.name = groups['name'];
+          e.message = groups['message'];
+        }
+      }
+      throw e;
+    }
+  };
 
   readonly on = (channel: string, listener: (e: IpcRendererEvent, ...args: unknown[]) => void): void => {
     ipcRenderer.on(`${this.namespace}-${channel}`, listener);