Clean up invokeMethod error propagation (#2336)

Jigsaw-Code · Jan 21, 2025 · 0ddcf20 · 0ddcf20
1 parent 6723c85
commit 0ddcf20
Show file tree

Hide file tree

Showing 6 changed files with 71 additions and 63 deletions.
diff --git a/client/electron/go_plugin.ts b/client/electron/go_plugin.ts
@@ -32,7 +32,7 @@ let invokeMethodFunc: Function | undefined;
  * Ensure that the function signature and data structures are consistent with the C definitions
  * in `./client/go/outline/electron/go_plugin.go`.
  */
-export async function invokeMethod(
+export async function invokeGoMethod(
   method: string,
   input: string
 ): Promise<string> {

diff --git a/client/electron/index.ts b/client/electron/index.ts
@@ -35,7 +35,7 @@ import {
 import {autoUpdater} from 'electron-updater';
 
 import {lookupIp} from './connectivity';
-import {invokeMethod} from './go_plugin';
+import {invokeGoMethod} from './go_plugin';
 import {GoVpnTunnel} from './go_vpn_tunnel';
 import {installRoutingServices, RoutingDaemon} from './routing_service';
 import {TunnelStore} from './tunnel_store';
@@ -529,53 +529,59 @@ function main() {
   // If the function encounters an error, it throws an Error that can be parsed by the `PlatformError`.
   ipcMain.handle(
     'outline-ipc-invoke-method',
-    (_, method: string, params: string): Promise<string> =>
-      invokeMethod(method, params)
-  );
-
-  // 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 (_, request: StartRequestJson): 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).
-      if (currentTunnel) {
-        console.log('disconnecting from current server...');
-        currentTunnel.disconnect();
-        await currentTunnel.onceDisconnected;
-      }
+    async (_, method: string, params: string): Promise<string> => {
+      // TODO(fortuna): move all other IPCs here.
+      switch (method) {
+        case 'StartProxying': {
+          // 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
+          const request = JSON.parse(params) as StartRequestJson;
+          // 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).
+          if (currentTunnel) {
+            console.log('disconnecting from current server...');
+            currentTunnel.disconnect();
+            await currentTunnel.onceDisconnected;
+          }
+
+          console.log(`connecting to ${request.name} (${request.id})...`);
+
+          try {
+            await startVpn(request, false);
+            console.log(`connected to ${request.name} (${request.id})`);
+            await setupAutoLaunch(request);
+            // Auto-connect requires IPs; the hostname in here has already been resolved (see above).
+            tunnelStore.save(request).catch(() => {
+              console.error('Failed to store tunnel.');
+            });
+          } catch (e) {
+            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();
+            throw e;
+          }
+          break;
+        }
 
-      console.log(`connecting to ${request.name} (${request.id})...`);
+        case 'StopProxying':
+          // Disconnects from the current server, if any.
+          // TODO: refactor channel name and namespace to a constant
+          await stopVpn();
+          return '';
 
-      try {
-        await startVpn(request, false);
-        console.log(`connected to ${request.name} (${request.id})`);
-        await setupAutoLaunch(request);
-        // Auto-connect requires IPs; the hostname in here has already been resolved (see above).
-        tunnelStore.save(request).catch(() => {
-          console.error('Failed to store tunnel.');
-        });
-      } catch (e) {
-        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();
-        throw e;
+        default:
+          return await invokeGoMethod(method, params);
       }
     }
   );
 
-  // Disconnects from the current server, if any.
-  // TODO: refactor channel name and namespace to a constant
-  ipcMain.handle('outline-ipc-stop-proxying', stopVpn);
-
   // Install backend services and return the error code
   // TODO: refactor channel name and namespace to a constant
   ipcMain.handle('outline-ipc-install-outline-services', async () => {

diff --git a/client/electron/vpn_service.ts b/client/electron/vpn_service.ts
@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-import {invokeMethod} from './go_plugin';
+import {invokeGoMethod} from './go_plugin';
 import {
   StartRequestJson,
   TunnelStatus,
@@ -71,13 +71,13 @@ export async function establishVpn(request: StartRequestJson) {
     transport: request.config.transport,
   };
 
-  await invokeMethod('EstablishVPN', JSON.stringify(config));
+  await invokeGoMethod('EstablishVPN', JSON.stringify(config));
   statusCb?.(currentRequestId, TunnelStatus.CONNECTED);
 }
 
 export async function closeVpn(): Promise<void> {
   statusCb?.(currentRequestId!, TunnelStatus.DISCONNECTING);
-  await invokeMethod('CloseVPN', '');
+  await invokeGoMethod('CloseVPN', '');
   statusCb?.(currentRequestId!, TunnelStatus.DISCONNECTED);
 }
 

diff --git a/client/src/www/app/main.cordova.ts b/client/src/www/app/main.cordova.ts
@@ -39,6 +39,7 @@ import {
 import {AbstractUpdater} from './updater';
 import * as interceptors from './url_interceptor';
 import {NoOpVpnInstaller, VpnInstaller} from './vpn_installer';
+import {PlatformError} from '../model/platform_error';
 import {SentryErrorReporter, Tags} from '../shared/error_reporter';
 
 const hasDeviceSupport = cordova.platformId !== 'browser';
@@ -80,7 +81,7 @@ class CordovaMethodChannel implements MethodChannel {
       return pluginExecWithErrorCode('invokeMethod', methodName, params);
     } catch (e) {
       console.debug('invokeMethod failed', methodName, e);
-      throw e;
+      throw PlatformError.parseFrom(e);
     }
   }
 }

diff --git a/client/src/www/app/main.electron.ts b/client/src/www/app/main.electron.ts
@@ -28,6 +28,7 @@ import {AbstractUpdater} from './updater';
 import {UrlInterceptor} from './url_interceptor';
 import {VpnInstaller} from './vpn_installer';
 import {ErrorCode, OutlinePluginError} from '../model/errors';
+import {PlatformError} from '../model/platform_error';
 import {
   getSentryBrowserIntegrations,
   OutlineErrorReporter,
@@ -129,11 +130,15 @@ class ElectronErrorReporter implements OutlineErrorReporter {
 
 class ElectronMethodChannel implements MethodChannel {
   invokeMethod(methodName: string, params: string): Promise<string> {
-    return window.electron.methodChannel.invoke(
-      'invoke-method',
-      methodName,
-      params
-    );
+    try {
+      return window.electron.methodChannel.invoke(
+        'invoke-method',
+        methodName,
+        params
+      );
+    } catch (e) {
+      throw PlatformError.parseFrom(e);
+    }
   }
 }
 

diff --git a/client/src/www/app/outline_server_repository/vpn.electron.ts b/client/src/www/app/outline_server_repository/vpn.electron.ts
@@ -17,7 +17,7 @@ import {IpcRendererEvent} from 'electron/main';
 import {TunnelStatus} from './vpn';
 import {StartRequestJson} from './vpn';
 import {VpnApi} from './vpn';
-import {PlatformError} from '../../model/platform_error';
+import * as methodChannel from '../method_channel';
 
 export class ElectronVpnApi implements VpnApi {
   private statusChangeListener:
@@ -54,23 +54,19 @@ export class ElectronVpnApi implements VpnApi {
       return Promise.resolve();
     }
 
-    try {
-      await window.electron.methodChannel.invoke('start-proxying', request);
-    } catch (e) {
-      throw PlatformError.parseFrom(e);
-    }
+    await methodChannel
+      .getDefaultMethodChannel()
+      .invokeMethod('StartProxying', JSON.stringify(request));
   }
 
   async stop(id: string) {
     if (this.runningServerId !== id) {
       return;
     }
 
-    try {
-      await window.electron.methodChannel.invoke('stop-proxying');
-    } catch (e) {
-      console.error(`Failed to stop tunnel ${e}`);
-    }
+    await methodChannel
+      .getDefaultMethodChannel()
+      .invokeMethod('StopProxying', '');
   }
 
   async isRunning(id: string): Promise<boolean> {