cross-document error propagation (#755)

apps/extension/src/approve-origin.ts

@@ -52,18 +52,19 @@ export const approveOrigin = async ({
 
       if ('error' in res)
         throw errorFromJson(res.error as JsonValue, undefined, ConnectError.from(res));
+      else if (res.data != null) {
+        // TODO: is there a race condition here?
+        // if something has written after our initial read, we'll clobber them
+        void localExtStorage.set('knownSites', [
+          {
+            ...res.data,
+            date: Date.now(),
+          },
+          ...irrelevant,
+        ]);
+      }
 
-      // TODO: is there a race condition here?
-      // if something has written after our initial read, we'll clobber them
-      void localExtStorage.set('knownSites', [
-        {
-          ...res.data,
-          date: Date.now(),
-        },
-        ...irrelevant,
-      ]);
-
-      return Boolean(res.data.choice === UserChoice.Approved);
+      return res.data?.choice === UserChoice.Approved;
     }
   }
 };

apps/extension/src/approve-transaction.ts

@@ -23,14 +23,19 @@ export const approveTransaction = async (
       transactionView: new TransactionView(transactionView).toJson() as Jsonified<TransactionView>,
     },
   });
+
   if ('error' in res)
     throw errorFromJson(res.error as JsonValue, undefined, ConnectError.from(res));
+  else if (res.data != null) {
+    const resAuthorizeRequest = AuthorizeRequest.fromJson(res.data.authorizeRequest);
+    const resTransactionView = TransactionView.fromJson(res.data.transactionView);
 
-  const resAuthorizeRequest = AuthorizeRequest.fromJson(res.data.authorizeRequest);
-  const resTransactionView = TransactionView.fromJson(res.data.transactionView);
-
-  if (!authorizeRequest.equals(resAuthorizeRequest) || !transactionView.equals(resTransactionView))
-    throw new Error('Invalid response from popup');
+    if (
+      !authorizeRequest.equals(resAuthorizeRequest) ||
+      !transactionView.equals(resTransactionView)
+    )
+      throw new Error('Invalid response from popup');
+  }
 
-  return res.data.choice;
+  return res.data?.choice;
 };

apps/extension/src/entry/offscreen-handler.ts

@@ -1,53 +1,51 @@
-import type { JsonValue } from '@bufbuild/protobuf';
+import { ConnectError } from '@connectrpc/connect';
+import { errorToJson } from '@connectrpc/connect/protocol-connect';
 import {
   ActionBuildRequest,
+  ActionBuildResponse,
   isActionBuildRequest,
   isOffscreenRequest,
 } from '@penumbra-zone/types/src/internal-msg/offscreen';
 
 chrome.runtime.onMessage.addListener((req, _sender, respond) => {
   if (!isOffscreenRequest(req)) return false;
-  if (isActionBuildRequest(req.request)) {
-    const { type, request } = req;
+  const { type, request } = req;
+  if (isActionBuildRequest(request)) {
     void (async () => {
-      const response = spawnWorker(request);
-      const res = await response
-        .then(data => ({ type, data }))
-        .catch((e: Error) => ({
+      try {
+        const data = await spawnActionBuildWorker(request);
+        respond({ type, data });
+      } catch (e) {
+        respond({
           type,
-          error: `Offscreen: ${e.message}`,
-        }));
-      respond(res);
+          error: errorToJson(ConnectError.from(e), undefined),
+        });
+      }
     })();
+    return true;
   }
-  return true;
+  return false;
 });
 
-const spawnWorker = (req: ActionBuildRequest): Promise<JsonValue> => {
-  return new Promise((resolve, reject) => {
-    const worker = new Worker(new URL('../wasm-build-action.ts', import.meta.url));
+const spawnActionBuildWorker = (req: ActionBuildRequest) => {
+  const worker = new Worker(new URL('../wasm-build-action.ts', import.meta.url));
+  return new Promise<ActionBuildResponse>((resolve, reject) => {
+    const onWorkerMessage = (e: MessageEvent) => resolve(e.data as ActionBuildResponse);
 
-    const onWorkerMessage = (e: MessageEvent) => {
-      resolve(e.data as JsonValue);
-      worker.removeEventListener('error', onWorkerError);
-      worker.terminate();
-    };
-
-    const onWorkerError = (ev: ErrorEvent) => {
-      const { filename, lineno, colno, message } = ev;
+    const onWorkerError = ({ error, filename, lineno, colno, message }: ErrorEvent) =>
       reject(
-        ev.error instanceof Error
-          ? ev.error
+        error instanceof Error
+          ? error
           : new Error(`Worker ErrorEvent ${filename}:${lineno}:${colno} ${message}`),
       );
-      worker.removeEventListener('message', onWorkerMessage);
-      worker.terminate();
-    };
+
+    const onWorkerMessageError = (ev: MessageEvent) => reject(ConnectError.from(ev.data ?? ev));
 
     worker.addEventListener('message', onWorkerMessage, { once: true });
     worker.addEventListener('error', onWorkerError, { once: true });
+    worker.addEventListener('messageerror', onWorkerMessageError, { once: true });
 
     // Send data to web worker
     worker.postMessage(req);
-  });
+  }).finally(() => worker.terminate());
 };

apps/extension/src/message/popup.ts

@@ -20,7 +20,7 @@ export type PopupResponse<T extends PopupMessage = PopupMessage> = InternalRespo
 export type OriginApproval = InternalMessage<
   PopupType.OriginApproval,
   { origin: string; favIconUrl?: string; title?: string; lastRequest?: number },
-  { origin: string; choice: UserChoice }
+  null | { origin: string; choice: UserChoice }
 >;
 
 export type TxApproval = InternalMessage<
@@ -29,7 +29,7 @@ export type TxApproval = InternalMessage<
     authorizeRequest: Jsonified<AuthorizeRequest>;
     transactionView: Jsonified<TransactionView>;
   },
-  {
+  null | {
     authorizeRequest: Jsonified<AuthorizeRequest>;
     transactionView: Jsonified<TransactionView>;
     choice: UserChoice;

apps/extension/src/popup.ts

@@ -6,14 +6,22 @@ import type {
   InternalResponse,
 } from '@penumbra-zone/types/src/internal-msg/shared';
 import { Code, ConnectError } from '@connectrpc/connect';
+import { isChromeResponderDroppedError } from '@penumbra-zone/types/src/internal-msg/chrome-error';
 
 export const popup = async <M extends PopupMessage>(
   req: PopupRequest<M>,
 ): Promise<PopupResponse<M>> => {
   await spawnPopup(req.type);
   // We have to wait for React to bootup, navigate to the page, and render the components
-  await new Promise(resolve => setTimeout(resolve, 1000));
-  return chrome.runtime.sendMessage<InternalRequest<M>, InternalResponse<M>>(req);
+  await new Promise(resolve => setTimeout(resolve, 800));
+  return chrome.runtime.sendMessage<InternalRequest<M>, InternalResponse<M>>(req).catch(e => {
+    if (isChromeResponderDroppedError(e))
+      return {
+        type: req.type,
+        data: null,
+      };
+    else throw e;
+  });
 };
 
 const spawnExtensionPopup = async (path: string) => {
@@ -45,7 +53,9 @@ const spawnDetachedPopup = async (path: string) => {
 const throwIfAlreadyOpen = (path: string) =>
   chrome.runtime
     .getContexts({
-      documentUrls: [chrome.runtime.getURL(path)],
+      documentUrls: [
+        path.startsWith(chrome.runtime.getURL('')) ? path : chrome.runtime.getURL(path),
+      ],
     })
     .then(popupContexts => {
       if (popupContexts.length) throw Error('Popup already open');

apps/extension/src/state/tx-approval.ts

@@ -42,7 +42,6 @@ export interface TxApprovalSlice {
     req: InternalRequest<TxApproval>,
     responder: (m: InternalResponse<TxApproval>) => void,
   ) => Promise<void>;
-  handleCloseWindow: () => void;
 
   setChoice: (choice: UserChoice) => void;
 
@@ -84,15 +83,6 @@ export const createTxApprovalSlice = (): SliceCreator<TxApprovalSlice> => (set,
 
       state.txApproval.choice = undefined;
     });
-
-    window.onbeforeunload = get().txApproval.handleCloseWindow;
-  },
-
-  handleCloseWindow: () => {
-    set(state => {
-      state.txApproval.choice = UserChoice.Ignored;
-    });
-    get().txApproval.sendResponse();
   },
 
   setChoice: choice => {
@@ -107,7 +97,6 @@ export const createTxApprovalSlice = (): SliceCreator<TxApprovalSlice> => (set,
       choice,
       transactionView: transactionViewString,
       authorizeRequest: authorizeRequestString,
-      handleCloseWindow,
     } = get().txApproval;
 
     if (!responder) throw new Error('No responder');
@@ -149,8 +138,6 @@ export const createTxApprovalSlice = (): SliceCreator<TxApprovalSlice> => (set,
         state.txApproval.asPublic = undefined;
         state.txApproval.transactionClassification = undefined;
       });
-
-      if (window.onbeforeunload === handleCloseWindow) window.onbeforeunload = null;
     }
   },
 });

apps/minifront/src/state/helpers.ts

@@ -148,18 +148,11 @@ const getTxId = (tx: Transaction | PartialMessage<Transaction>) =>
     inner => new TransactionId({ inner }),
   );
 
-/**
- * @todo: The error flow between extension <-> webapp needs to be refactored a
- * bit. Right now, if we throw a `ConnectError` with `Code.PermissionDenied` (as
- * we do in the approver), it gets swallowed by ConnectRPC's internals and
- * rethrown as a string.  This means that the original code is lost, although
- * the stringified error message still contains `[permission_denied]`. So we'll
- * (somewhat hackily) check the stringified error message for now; but in the
- * future, we need ot get the error flow working properly so that we can
- * actually check `e.code === Code.PermissionDenied`.
- */
+// We don't have ConnectError in this scope, so we only detect standard Error.
+// Any ConnectError code is named at the beginning of the message value.
+
 export const userDeniedTransaction = (e: unknown): boolean =>
-  typeof e === 'string' && e.includes('[permission_denied]');
+  e instanceof Error && e.message.startsWith('[permission_denied]');
 
 export const unauthenticated = (e: unknown): boolean =>
-  typeof e === 'string' && e.includes('[unauthenticated]');
+  e instanceof Error && e.message.startsWith('[unauthenticated]');

packages/router/src/grpc/offscreen-client.ts

@@ -9,6 +9,8 @@ import {
   OffscreenMessage,
 } from '@penumbra-zone/types/src/internal-msg/offscreen';
 import { InternalRequest, InternalResponse } from '@penumbra-zone/types/src/internal-msg/shared';
+import { ConnectError } from '@connectrpc/connect';
+import type { Jsonified } from '@penumbra-zone/types/src/jsonified';
 
 const OFFSCREEN_DOCUMENT_PATH = '/offscreen.html';
 
@@ -44,7 +46,7 @@ const releaseOffscreen = async () => {
 
 const sendOffscreenMessage = async <T extends OffscreenMessage>(
   req: InternalRequest<T>,
-): Promise<InternalResponse<ActionBuildMessage>> =>
+): Promise<InternalResponse<T>> =>
   chrome.runtime.sendMessage<InternalRequest<T>, InternalResponse<T>>(req);
 
 /**
@@ -62,22 +64,27 @@ const buildActions = (
   // eslint-disable-next-line @typescript-eslint/no-floating-promises
   const buildTasks = transactionPlan.actions.map(async (_, actionPlanIndex) => {
     await active;
-    const buildRes = await Promise.race([
-      cancel,
-      sendOffscreenMessage<ActionBuildMessage>({
-        type: 'BUILD_ACTION',
-        request: {
-          transactionPlan: transactionPlan.toJson(),
-          witness: witness.toJson(),
-          fullViewingKey,
-          actionPlanIndex,
-        } as ActionBuildRequest,
-      }),
-    ]);
-    if ('error' in buildRes) throw new Error(String(buildRes.error));
+    const buildRes = await sendOffscreenMessage<ActionBuildMessage>({
+      type: 'BUILD_ACTION',
+      request: {
+        transactionPlan: transactionPlan.toJson() as Jsonified<TransactionPlan>,
+        witness: witness.toJson() as Jsonified<WitnessData>,
+        fullViewingKey,
+        actionPlanIndex,
+      } satisfies ActionBuildRequest,
+    });
+    if ('error' in buildRes) throw ConnectError.from(buildRes.error);
     return Action.fromJson(buildRes.data);
   });
-  void Promise.all(buildTasks).finally(() => void releaseOffscreen());
+
+  void Promise.race([Promise.all(buildTasks), cancel])
+    .catch(
+      // if we don't suppress this, we log errors when a user denies approval.
+      // real failures are already conveyed by the individual promises.
+      () => null,
+    )
+    .finally(() => void releaseOffscreen());
+
   return buildTasks;
 };
 

packages/transport-chrome/session-client.ts

@@ -16,14 +16,15 @@
 
 import {
   isTransportError,
-  isTransportEvent,
   isTransportMessage,
   isTransportStream,
   TransportStream,
 } from '@penumbra-zone/transport-dom/src/messages';
 import { ChannelLabel, nameConnection } from './channel-names';
 import { isTransportInitChannel, TransportInitChannel } from './message';
 import { PortStreamSink, PortStreamSource } from './stream';
+import { Code, ConnectError } from '@connectrpc/connect';
+import { errorToJson } from '@connectrpc/connect/protocol-connect';
 
 export class CRSessionClient {
   private static singleton?: CRSessionClient;
@@ -60,13 +61,10 @@ export class CRSessionClient {
 
   private disconnect = () => {
     this.clientPort.removeEventListener('message', this.clientListener);
-    this.clientPort.addEventListener('message', (ev: MessageEvent<unknown>) => {
-      if (isTransportEvent(ev.data)) {
-        const { requestId } = ev.data;
-        this.clientPort.postMessage({ requestId, error: 'Connection closed' });
-      }
+    this.clientPort.postMessage({
+      error: errorToJson(new ConnectError('Connection closed', Code.Unavailable), undefined),
     });
-    this.clientPort.postMessage({ error: { code: 'unavailable', message: 'Connection closed' } });
+    this.clientPort.close();
   };
 
   private clientListener = (ev: MessageEvent<unknown>) => {
@@ -76,7 +74,7 @@ export class CRSessionClient {
         this.servicePort.postMessage(this.requestChannelStream(ev.data));
       else console.warn('Unknown item from client', ev.data);
     } catch (e) {
-      this.clientPort.postMessage({ error: String(e) });
+      this.clientPort.postMessage({ error: errorToJson(ConnectError.from(e), undefined) });
     }
   };
 
@@ -87,7 +85,7 @@ export class CRSessionClient {
         this.clientPort.postMessage(...this.acceptChannelStreamResponse(m));
       else console.warn('Unknown item from service', m);
     } catch (e) {
-      this.clientPort.postMessage({ error: String(e) });
+      this.clientPort.postMessage({ error: errorToJson(ConnectError.from(e), undefined) });
     }
   };
 
@@ -101,7 +99,7 @@ export class CRSessionClient {
     const sinkListener = (p: chrome.runtime.Port) => {
       if (p.name !== channel) return;
       chrome.runtime.onConnect.removeListener(sinkListener);
-      stream.pipeTo(new WritableStream(new PortStreamSink(p))).catch(() => null);
+      void stream.pipeTo(new WritableStream(new PortStreamSink(p))).catch(() => null);
     };
     chrome.runtime.onConnect.addListener(sinkListener);
     return { requestId, channel } satisfies TransportInitChannel;

packages/transport-chrome/session-manager.ts

@@ -154,7 +154,7 @@ export class CRSessionManager {
     const sinkListener = (p: chrome.runtime.Port) => {
       if (p.name !== channel) return;
       chrome.runtime.onConnect.removeListener(sinkListener);
-      stream.pipeTo(new WritableStream(new PortStreamSink(p)), { signal }).catch(() => null);
+      void stream.pipeTo(new WritableStream(new PortStreamSink(p)), { signal }).catch(() => null);
     };
     chrome.runtime.onConnect.addListener(sinkListener);
     return { requestId, channel };