fix(MapView): don't leak unhandled rejections from native bridge calls

__tests__/components/MapView.test.js

@@ -2,6 +2,18 @@ import * as React from 'react';
 import { render } from '@testing-library/react-native';
 
 import MapView from '../../src/components/MapView';
+import NativeMapViewModule from '../../src/specs/NativeMapViewModule';
+
+// `_runNativeMethod` lives on the class produced by `NativeBridgeComponent`,
+// which `MapView` extends - it is not an own property of `MapView.prototype`.
+const bridgePrototype = Object.getPrototypeOf(MapView.prototype);
+
+// Let the microtask queue drain and give node a macrotask turn, which is when
+// an unhandled rejection would be reported.
+const flushRejections = async () => {
+  await new Promise((resolve) => setImmediate(resolve));
+  await new Promise((resolve) => setImmediate(resolve));
+};
 
 describe('MapView', () => {
   test('renders with testID', () => {
@@ -13,4 +25,153 @@ describe('MapView', () => {
       getByTestId(expectedTestId);
     }).not.toThrow();
   });
+
+  describe('setHandledMapChangedEvents', () => {
+    let unhandledRejection;
+    let warnSpy;
+
+    beforeEach(() => {
+      unhandledRejection = jest.fn();
+      process.on('unhandledRejection', unhandledRejection);
+      warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {});
+    });
+
+    afterEach(() => {
+      process.off('unhandledRejection', unhandledRejection);
+      jest.restoreAllMocks();
+    });
+
+    // Regression guard for the fix itself: on a plain render the native ref is
+    // not resolved yet, so the call is queued rather than sent to the native
+    // module. The queued branch must still hand back a real promise, otherwise
+    // attaching a rejection handler to it would throw.
+    test('queues the call while the native ref is unresolved', () => {
+      const nativeSpy = jest.spyOn(
+        NativeMapViewModule,
+        'setHandledMapChangedEvents',
+      );
+      const ref = React.createRef();
+
+      render(<MapView ref={ref} onMapIdle={() => {}} />);
+
+      expect(ref.current._nativeRef).toBeUndefined();
+      expect(nativeSpy).not.toHaveBeenCalled();
+      expect(ref.current._preRefMapMethodQueue).toHaveLength(1);
+      expect(
+        ref.current._runNativeMethod('setHandledMapChangedEvents', undefined, [
+          [],
+        ]),
+      ).toBeInstanceOf(Promise);
+    });
+
+    // https://github.com/rnmapbox/maps/issues/3492 - unmounting the map while
+    // the call is in flight rejects with `Unknown reactTag: <n>`.
+    test('does not leak an unhandled rejection on mount', async () => {
+      let error;
+      jest.spyOn(bridgePrototype, '_runNativeMethod').mockImplementation(() => {
+        // Built here so the stack points at the real bridge call site.
+        error = new Error('Unknown reactTag: 123');
+        return Promise.reject(error);
+      });
+
+      render(<MapView onMapIdle={() => {}} />);
+      await flushRejections();
+
+      expect(unhandledRejection).not.toHaveBeenCalled();
+      expect(warnSpy).toHaveBeenCalledWith(
+        expect.stringContaining('setHandledMapChangedEvents'),
+        error,
+      );
+    });
+
+    test('does not leak an unhandled rejection on update', async () => {
+      const ref = React.createRef();
+      const { rerender } = render(<MapView ref={ref} onMapIdle={() => {}} />);
+
+      let error;
+      const runNativeMethod = jest
+        .spyOn(bridgePrototype, '_runNativeMethod')
+        .mockImplementation(() => {
+          // Built here so the stack points at the real bridge call site.
+          error = new Error('Unknown reactTag: 123');
+          return Promise.reject(error);
+        });
+
+      // A new inline handler changes the callback prop identity, which is what
+      // makes `componentDidUpdate` resend the handled events.
+      rerender(<MapView ref={ref} onMapIdle={() => {}} />);
+      await flushRejections();
+
+      expect(runNativeMethod).toHaveBeenCalledWith(
+        'setHandledMapChangedEvents',
+        undefined,
+        [expect.arrayContaining([expect.any(String)])],
+      );
+      expect(unhandledRejection).not.toHaveBeenCalled();
+      expect(warnSpy).toHaveBeenCalledWith(
+        expect.stringContaining('setHandledMapChangedEvents'),
+        error,
+      );
+    });
+
+    // `runNativeMethod` throws synchronously when the view handle has already
+    // gone, so a bare `.catch()` would not cover it.
+    test('reports a synchronous failure instead of throwing', async () => {
+      const error = new Error('Could not find handle for native ref');
+      jest
+        .spyOn(bridgePrototype, '_runNativeMethod')
+        .mockImplementation(() => {
+          throw error;
+        });
+
+      expect(() => render(<MapView onMapIdle={() => {}} />)).not.toThrow();
+      await flushRejections();
+
+      expect(unhandledRejection).not.toHaveBeenCalled();
+      expect(warnSpy).toHaveBeenCalledWith(
+        expect.stringContaining('setHandledMapChangedEvents'),
+        error,
+      );
+    });
+
+    // The mount-time call is queued until the native ref lands, so it is
+    // drained by `_runPendingNativeMethods` - an async method every caller
+    // invokes fire-and-forget, which leaks the rejection just the same.
+    test('does not leak an unhandled rejection while draining the queue', async () => {
+      const ref = React.createRef();
+      render(<MapView ref={ref} onMapIdle={() => {}} />);
+
+      expect(ref.current._preRefMapMethodQueue).toHaveLength(1);
+
+      let error;
+      jest.spyOn(bridgePrototype, '_runNativeMethod').mockImplementation(() => {
+        error = new Error('Unknown reactTag: 123');
+        return Promise.reject(error);
+      });
+
+      ref.current._setNativeRef({});
+      await flushRejections();
+
+      expect(unhandledRejection).not.toHaveBeenCalled();
+      expect(warnSpy).toHaveBeenCalledWith(
+        expect.stringContaining('setHandledMapChangedEvents'),
+        error,
+      );
+    });
+
+    // Non-vacuity: the tests above must not pass merely because every outcome
+    // is being swallowed - a successful call still resolves and stays quiet.
+    test('stays quiet when the native call succeeds', async () => {
+      const runNativeMethod = jest
+        .spyOn(bridgePrototype, '_runNativeMethod')
+        .mockImplementation(() => Promise.resolve(undefined));
+
+      render(<MapView onMapIdle={() => {}} />);
+      await flushRejections();
+
+      expect(runNativeMethod).toHaveBeenCalledTimes(1);
+      expect(unhandledRejection).not.toHaveBeenCalled();
+      expect(warnSpy).not.toHaveBeenCalled();
+    });
+  });
 });

src/components/MapView.tsx

@@ -672,9 +672,11 @@ class MapView extends NativeBridgeComponent(
       );
     }
 
-    this._runNativeMethod('setHandledMapChangedEvents', this._nativeRef, [
-      events,
-    ]);
+    this._runNativeMethodDetached(
+      'setHandledMapChangedEvents',
+      this._nativeRef,
+      [events],
+    );
   }
 
   /**
@@ -937,7 +939,7 @@ class MapView extends NativeBridgeComponent(
     sourceId: string,
     sourceLayerId: string | null = null,
   ) {
-    this._runNative<void>('setSourceVisibility', [
+    this._runNativeMethodDetached('setSourceVisibility', this._nativeRef, [
       visible,
       sourceId,
       sourceLayerId,

src/components/NativeBridgeComponent.tsx

@@ -29,18 +29,39 @@ const NativeBridgeComponent = <
       this._preRefMapMethodQueue = [];
     }
 
+    /**
+     * Reports a native method that failed with no caller left to handle it.
+     *
+     * Rethrowing is not an option here: every caller of the methods below
+     * discards the promise, so a rethrow would only recreate the unhandled
+     * rejection this is meant to remove.
+     */
+    _warnNativeMethodFailed(methodName: string, error: unknown) {
+      console.warn(
+        `rnmapbox/maps: native method ${methodName} failed - this is expected if the view was detached while the call was in flight:`,
+        error,
+      );
+    }
+
     async _runPendingNativeMethods<RefType>(nativeRef: RefType) {
       if (nativeRef) {
         while (this._preRefMapMethodQueue.length > 0) {
           const item = this._preRefMapMethodQueue.pop();
 
           if (item && item.method && item.resolver) {
-            const res = await this._runNativeMethod(
-              item.method.name,
-              nativeRef,
-              item.method.args,
-            );
-            item.resolver(res);
+            // Every caller invokes this fire-and-forget, so a rejection here
+            // would escape as an unhandled rejection. Catching per item also
+            // keeps one failure from abandoning the rest of the queue.
+            try {
+              const res = await this._runNativeMethod(
+                item.method.name,
+                nativeRef,
+                item.method.args,
+              );
+              item.resolver(res);
+            } catch (error) {
+              this._warnNativeMethodFailed(item.method.name, error);
+            }
           }
         }
       }
@@ -62,6 +83,31 @@ const NativeBridgeComponent = <
 
       return runNativeMethod(this._turboModule, methodName, nativeRef, args);
     }
+
+    /**
+     * Runs a native method whose result is intentionally discarded.
+     *
+     * Callers that drop the promise on the floor leak an unhandled rejection
+     * whenever the native view goes away while the call is in flight - a tab
+     * switch, or any unmount - because the bridge then rejects with
+     * `Unknown reactTag: <n>`.
+     */
+    _runNativeMethodDetached<RefType>(
+      methodName: string,
+      nativeRef: RefType | undefined,
+      args: NativeArg[] = [],
+    ): void {
+      const onError = (error: unknown) =>
+        this._warnNativeMethodFailed(methodName, error);
+
+      try {
+        // `runNativeMethod` throws synchronously when the view handle is
+        // already gone, so the try/catch is not redundant with `.catch`.
+        this._runNativeMethod(methodName, nativeRef, args).catch(onError);
+      } catch (error) {
+        onError(error);
+      }
+    }
   };
 
 export default NativeBridgeComponent;

src/components/PointAnnotation.tsx

@@ -209,7 +209,7 @@ class PointAnnotation extends NativeBridgeComponent(
    * Call this for example from Image#onLoad.
    */
   refresh() {
-    this._runNativeMethod('refresh', this._nativeRef, []);
+    this._runNativeMethodDetached('refresh', this._nativeRef, []);
   }
 
   _setNativeRef(nativeRef: NativePointAnnotationRef) {