feat: Add Messenger error reporting (#6605)

## Explanation

The `Messenger` class in `@metamask/messenger` has been updated to
accept an optional `captureException` constructor parameter. This is now
used to capture subscriber errors, instead of throwing them in a
`setTimeout` (which would cause a crash on mobile).

## References

This is the implementation of the 2nd option in this ADR:
https://github.com/MetaMask/decisions/blob/main/decisions/core/0016-core-classes-error-reporting.md#optional-captureexception-constructor-parameter-that-inherits-from-parent

Relates to #6613

## Checklist

- [x] I've updated the test suite for new or updated code as appropriate
- [x] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [x] I've communicated my changes to consumers by [updating changelogs
for packages I've
changed](https://github.com/MetaMask/core/tree/main/docs/contributing.md#updating-changelogs),
highlighting breaking changes as necessary
- [x] I've prepared draft pull requests for clients and consumer
packages to resolve any breaking changes

Author: Gudahtt

packages/messenger/CHANGELOG.md

@@ -7,6 +7,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- Add `captureException` constructor parameter ([#6605](https://github.com/MetaMask/core/pull/6605))
+  - This function will be used to capture any errors thrown from subscribers.
+  - If this is unset but a parent is provided, `captureException` is inherited from the parent.
+
+### Changed
+
+- Stop re-throwing subscriber errors in a `setTimeout` ([#6605](https://github.com/MetaMask/core/pull/6605))
+  - Instead errors are captured with `captureException`, or logged to the console.
+
 ## [0.2.0]
 
 ### Added

packages/messenger/src/Messenger.test.ts

@@ -810,26 +810,89 @@ describe('Messenger', () => {
       expect(selector.callCount).toBe(4);
     });
 
-    it('throws subscriber errors in a timeout', () => {
-      const setTimeoutStub = sinon.stub(globalThis, 'setTimeout');
+    it('captures subscriber errors using captureException', () => {
+      const captureException = jest.fn();
       type MessageEvent = { type: 'Fixture:message'; payload: [string] };
       const messenger = new Messenger<'Fixture', never, MessageEvent>({
+        captureException,
         namespace: 'Fixture',
       });
+      const exampleError = new Error('Example error');
 
-      const handler = sinon.stub().throws(() => new Error('Example error'));
+      const handler = sinon.stub().throws(() => exampleError);
       messenger.subscribe('Fixture:message', handler);
 
       expect(() => messenger.publish('Fixture:message', 'hello')).not.toThrow();
-      expect(setTimeoutStub.callCount).toBe(1);
-      const onTimeout = setTimeoutStub.firstCall.args[0];
-      expect(() => onTimeout()).toThrow('Example error');
+      expect(captureException).toHaveBeenCalledTimes(1);
+      expect(captureException).toHaveBeenCalledWith(exampleError);
+    });
+
+    it('captures subscriber thrown non-errors using captureException', () => {
+      const captureException = jest.fn();
+      type MessageEvent = { type: 'Fixture:message'; payload: [string] };
+      const messenger = new Messenger<'Fixture', never, MessageEvent>({
+        captureException,
+        namespace: 'Fixture',
+      });
+      const exampleException = 'Non-error thrown value';
+
+      const handler = sinon.stub().throws(() => exampleException);
+      messenger.subscribe('Fixture:message', handler);
+
+      expect(() => messenger.publish('Fixture:message', 'hello')).not.toThrow();
+      expect(captureException).toHaveBeenCalledTimes(1);
+      expect(captureException).toHaveBeenCalledWith(
+        new Error(exampleException),
+      );
+    });
+
+    it('captures subscriber errors using inherited captureException', () => {
+      const captureException = jest.fn();
+      type MessageEvent = { type: 'Fixture:message'; payload: [string] };
+      const parentMessenger = new Messenger<'Parent', never, MessageEvent>({
+        captureException,
+        namespace: 'Parent',
+      });
+      const messenger = new Messenger<
+        'Fixture',
+        never,
+        MessageEvent,
+        typeof parentMessenger
+      >({
+        namespace: 'Fixture',
+        parent: parentMessenger,
+      });
+      const exampleError = new Error('Example error');
+
+      const handler = sinon.stub().throws(() => exampleError);
+      messenger.subscribe('Fixture:message', handler);
+
+      expect(() => messenger.publish('Fixture:message', 'hello')).not.toThrow();
+      expect(captureException).toHaveBeenCalledTimes(1);
+      expect(captureException).toHaveBeenCalledWith(exampleError);
+    });
+
+    it('logs subscriber errors to console if no captureException provided', () => {
+      const consoleError = jest.fn();
+      jest.spyOn(console, 'error').mockImplementation(consoleError);
+      type MessageEvent = { type: 'Fixture:message'; payload: [string] };
+      const messenger = new Messenger<'Fixture', never, MessageEvent>({
+        namespace: 'Fixture',
+      });
+      const exampleError = new Error('Example error');
+
+      const handler = sinon.stub().throws(() => exampleError);
+      messenger.subscribe('Fixture:message', handler);
+
+      expect(() => messenger.publish('Fixture:message', 'hello')).not.toThrow();
+      expect(consoleError).toHaveBeenCalledTimes(1);
+      expect(consoleError).toHaveBeenCalledWith(exampleError);
     });
 
     it('continues calling subscribers when one throws', () => {
-      const setTimeoutStub = sinon.stub(globalThis, 'setTimeout');
       type MessageEvent = { type: 'Fixture:message'; payload: [string] };
       const messenger = new Messenger<'Fixture', never, MessageEvent>({
+        captureException: jest.fn(),
         namespace: 'Fixture',
       });
 
@@ -844,9 +907,6 @@ describe('Messenger', () => {
       expect(handler1.callCount).toBe(1);
       expect(handler2.calledWithExactly('hello')).toBe(true);
       expect(handler2.callCount).toBe(1);
-      expect(setTimeoutStub.callCount).toBe(1);
-      const onTimeout = setTimeoutStub.firstCall.args[0];
-      expect(() => onTimeout()).toThrow('Example error');
     });
 
     it('does not call subscriber after unsubscribing', () => {

packages/messenger/src/Messenger.ts

@@ -177,6 +177,7 @@ type DelegatedMessenger = Pick<
   | '_internalRegisterDelegatedActionHandler'
   | '_internalRegisterDelegatedInitialEventPayload'
   | '_internalUnregisterDelegatedActionHandler'
+  | 'captureException'
 >;
 
 type StripNamespace<Namespaced extends NamespacedName> =
@@ -254,20 +255,30 @@ export class Messenger<
     unknown | undefined
   >();
 
+  /**
+   * Reports an error to an error monitoring service.
+   *
+   * @param error - The error to report.
+   */
+  readonly captureException?: (error: Error) => void;
+
   /**
    * Construct a messenger.
    *
    * If a parent messenger is given, all actions and events under this messenger's namespace will
    * be delegated to the parent automatically.
    *
    * @param args - Constructor arguments
+   * @param args.captureException - Reports an error to an error monitoring service.
    * @param args.namespace - The messenger namespace.
    * @param args.parent - The parent messenger.
    */
   constructor({
+    captureException,
     namespace,
     parent,
   }: {
+    captureException?: (error: Error) => void;
     namespace: Namespace;
     parent?: Action['type'] extends MessengerActions<Parent>['type']
       ? Event['type'] extends MessengerEvents<Parent>['type']
@@ -277,6 +288,7 @@ export class Messenger<
   }) {
     this.#namespace = namespace;
     this.#parent = parent;
+    this.captureException = captureException ?? this.#parent?.captureException;
   }
 
   /**
@@ -529,11 +541,14 @@ export class Messenger<
             (handler as GenericEventHandler)(...payload);
           }
         } catch (error) {
-          // Throw error after timeout so that it is capured as a console error
-          // (and by Sentry) without interrupting the event publishing.
-          setTimeout(() => {
-            throw error;
-          });
+          // Capture error without interrupting the event publishing.
+          if (this.captureException) {
+            this.captureException(
+              error instanceof Error ? error : new Error(String(error)),
+            );
+          } else {
+            console.error(error);
+          }
         }
       }
     }