From 3132db934456d5a6a82ae8a7328105d23bac837d Mon Sep 17 00:00:00 2001 From: Bao Nguyen Date: Fri, 14 Aug 2026 09:51:14 +0700 Subject: [PATCH] fix(MapView): don't leak unhandled rejections from native bridge calls `_setHandledMapChangedEvents` discarded the promise returned by `_runNativeMethod`. When the native view is torn down while the call is in flight - navigating away from the map, or any unmount - the bridge rejects with `Unknown reactTag: ` and React Native escalates it to a `Possible Unhandled Promise Rejection`. It is called from both `componentDidMount` and `componentDidUpdate`, and the latter re-fires on every render that changes a callback prop identity, so apps using inline handlers report the warning in very high volume. Add `_runNativeMethodDetached` to `NativeBridgeComponent` for calls whose result is intentionally discarded, and route the three fire-and-forget call sites through it: `MapView._setHandledMapChangedEvents`, `MapView.setSourceVisibility` and `PointAnnotation.refresh`. It reports failures through `console.warn` rather than rethrowing - every caller has already returned, so a rethrow would only recreate the unhandled rejection. `runNativeMethod` throws synchronously when the view handle is already gone, which a bare `.catch()` would miss, so both paths are handled. `_runPendingNativeMethods` had the same problem: it is `async` and all three of its callers invoke it fire-and-forget, so a rejection while draining the queued mount-time call escaped the same way. Catching per item also stops one failure from abandoning the rest of the queue. Fixes #3492 --- __tests__/components/MapView.test.js | 161 +++++++++++++++++++++++ src/components/MapView.tsx | 10 +- src/components/NativeBridgeComponent.tsx | 58 +++++++- src/components/PointAnnotation.tsx | 2 +- 4 files changed, 220 insertions(+), 11 deletions(-) diff --git a/__tests__/components/MapView.test.js b/__tests__/components/MapView.test.js index e580fe217e..76e2788568 100644 --- a/__tests__/components/MapView.test.js +++ b/__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( {}} />); + + 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: `. + 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( {}} />); + 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( {}} />); + + 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( {}} />); + 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( {}} />)).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( {}} />); + + 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( {}} />); + await flushRejections(); + + expect(runNativeMethod).toHaveBeenCalledTimes(1); + expect(unhandledRejection).not.toHaveBeenCalled(); + expect(warnSpy).not.toHaveBeenCalled(); + }); + }); }); diff --git a/src/components/MapView.tsx b/src/components/MapView.tsx index d7b5bf5029..44ff5cc29d 100644 --- a/src/components/MapView.tsx +++ b/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('setSourceVisibility', [ + this._runNativeMethodDetached('setSourceVisibility', this._nativeRef, [ visible, sourceId, sourceLayerId, diff --git a/src/components/NativeBridgeComponent.tsx b/src/components/NativeBridgeComponent.tsx index dbc372f34f..2eeecbf7dd 100644 --- a/src/components/NativeBridgeComponent.tsx +++ b/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(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: `. + */ + _runNativeMethodDetached( + 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; diff --git a/src/components/PointAnnotation.tsx b/src/components/PointAnnotation.tsx index 3c889e042a..b6d9582f4e 100644 --- a/src/components/PointAnnotation.tsx +++ b/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) {