diff --git a/.github/scripts/android-js-fatal.sh b/.github/scripts/android-js-fatal.sh new file mode 100644 index 00000000..e46b306b --- /dev/null +++ b/.github/scripts/android-js-fatal.sh @@ -0,0 +1,97 @@ +#!/usr/bin/env bash +# Asserts a fatal JS error produces one report, no duplicate from the rethrown JavascriptException, and no 5 s wait. +set -euo pipefail + +PACKAGE="com.reactnative" +ACTIVITY="$PACKAGE/.MainActivity" +APK="examples/sdk/reactNative/android/app/build/outputs/apk/release/app-release.apk" +MARKER="ci-js-fatal-$(date +%s)" +TRIGGER_URL="backtrace-example://ci-js-fatal?marker=$MARKER" +LOGCAT_OUT="/tmp/js-fatal-logcat.txt" +FORWARD_BOUND_S=7 +EXIT_BOUND_S=2 + +wait_for_log() { + local pattern="$1" deadline="$2" + for _ in $(seq 1 "$deadline"); do + if adb logcat -d | grep -qE "$pattern"; then + return 0 + fi + sleep 1 + done + + echo "::error::timed out after ${deadline}s waiting for: $pattern" + adb logcat -d | tail -50 + return 1 +} + +seconds_between() { + python3 -c "print(round(float('$2') - float('$1'), 2))" +} + +at_most() { + python3 -c "import sys; sys.exit(0 if float('$1') <= float('$2') else 1)" +} + +trap 'adb logcat -d -v epoch > "$LOGCAT_OUT" 2>/dev/null || true' EXIT + +adb wait-for-device +adb uninstall "$PACKAGE" >/dev/null 2>&1 || true +adb install -r "$APK" +adb shell pm clear "$PACKAGE" >/dev/null +adb logcat -c + +adb shell am start -n "$ACTIVITY" >/dev/null +wait_for_log "BT_CI_DRIVER_ARMED" 120 +PID="$(adb shell pidof "$PACKAGE" | tr -d '\r')" + +adb shell am start -n "$ACTIVITY" -a android.intent.action.VIEW -d "'$TRIGGER_URL'" >/dev/null +echo "fatal JS error fired: $MARKER, pid $PID" + +for _ in $(seq 1 30); do + if ! adb shell pidof "$PACKAGE" >/dev/null 2>&1; then + break + fi + sleep 1 +done +if adb shell pidof "$PACKAGE" >/dev/null 2>&1; then + echo "::error::the app is still running 30 s after the fatal JS error" + exit 1 +fi +sleep 1 +adb logcat -d -v epoch > "$LOGCAT_OUT" + +FIRED_AT="$(awk '/BT_CI_DRIVER firing: backtrace-example:\/\/ci-js-fatal/ { print $1; exit }' "$LOGCAT_OUT")" +RETHROWN_AT="$(awk -v pid="$PID" '$2 == pid && /AndroidRuntime: FATAL EXCEPTION/ { print $1; exit }' "$LOGCAT_OUT")" +DIED_AT="$(grep -F "Process $PACKAGE (pid $PID) has died" "$LOGCAT_OUT" | head -1 | awk '{ print $1 }' || true)" +if [ -z "$FIRED_AT" ] || [ -z "$RETHROWN_AT" ] || [ -z "$DIED_AT" ]; then + echo "::error::missing log lines: fired '$FIRED_AT', rethrown '$RETHROWN_AT', died '$DIED_AT'" + grep -E "BT_CI|AndroidRuntime|has died" "$LOGCAT_OUT" | tail -30 + exit 1 +fi + +FORWARD_S="$(seconds_between "$FIRED_AT" "$RETHROWN_AT")" +EXIT_S="$(seconds_between "$RETHROWN_AT" "$DIED_AT")" +echo "React Native rethrew ${FORWARD_S}s after the fatal JS error, the process died ${EXIT_S}s later" +if ! at_most "$FORWARD_S" "$FORWARD_BOUND_S"; then + echo "::error::the JS handler held the fatal error ${FORWARD_S}s, expected at most ${FORWARD_BOUND_S}s" + exit 1 +fi +if ! at_most "$EXIT_S" "$EXIT_BOUND_S"; then + echo "::error::the process died ${EXIT_S}s after the rethrow, expected at most ${EXIT_BOUND_S}s" + exit 1 +fi + +RECORDS="$(awk -v since="$FIRED_AT" '$1 >= since && /BT_CI_RECORD/ { sub(/.*BT_CI_RECORD /, ""); print }' "$LOGCAT_OUT")" +COUNT="$(printf '%s\n' "$RECORDS" | grep -c . || true)" +echo "reports stored: $COUNT" +printf '%s\n' "$RECORDS" +if [ "$COUNT" != "1" ]; then + echo "::error::expected exactly one stored report, found $COUNT" + exit 1 +fi +if ! printf '%s\n' "$RECORDS" | grep -qF "Error \"BT_CI_JS_FATAL $MARKER\""; then + echo "::error::the stored report is not the fatal JS error" + exit 1 +fi +echo "one report with the JS error, no JavascriptException duplicate, process died ${EXIT_S}s after the rethrow" diff --git a/.github/scripts/ci-crash-driver.js b/.github/scripts/ci-crash-driver.js index 99b3d8b1..40a67949 100644 --- a/.github/scripts/ci-crash-driver.js +++ b/.github/scripts/ci-crash-driver.js @@ -2,6 +2,14 @@ import { Linking } from 'react-native'; Linking.addEventListener('url', ({ url }) => { + const fatal = /^backtrace-example:\/\/ci-js-fatal\?marker=([A-Za-z0-9-]+)$/.exec(url ?? ''); + if (fatal) { + console.log(`BT_CI_DRIVER firing: ${url}`); + setTimeout(() => { + throw new Error(`BT_CI_JS_FATAL ${fatal[1]}`); + }, 0); + return; + } const match = /^backtrace-example:\/\/ci-native-crash\?marker=([A-Za-z0-9-]+)$/.exec(url ?? ''); if (!match) { return; @@ -12,4 +20,12 @@ Linking.addEventListener('url', ({ url }) => { client.crash(); }); +BacktraceClient.instance?.database?.on('added', (record) => { + if (record.type === 'report') { + console.log( + `BT_CI_RECORD ${record.data.classifiers?.[0]} ${JSON.stringify(record.data.attributes?.['error.message'] ?? '')}`, + ); + } +}); + console.log('BT_CI_DRIVER_ARMED'); diff --git a/.github/workflows/android.yml b/.github/workflows/android.yml index 4d04691a..40ea9353 100644 --- a/.github/workflows/android.yml +++ b/.github/workflows/android.yml @@ -113,7 +113,7 @@ jobs: disable-animations: false script: echo "Generated AVD snapshot for caching." - - name: Capture a native crash on the emulator, single APK then Play-style split install + - name: Capture a native crash, single APK then Play-style split install, and a fatal JS error on the emulator uses: reactivecircus/android-emulator-runner@v2 env: BUNDLETOOL_JAR: ${{ runner.temp }}/bundletool.jar @@ -127,6 +127,7 @@ jobs: script: | INSTALL=apk bash .github/scripts/android-native-crash.sh INSTALL=split bash .github/scripts/android-native-crash.sh + bash .github/scripts/android-js-fatal.sh - name: Upload crash evidence if: failure() @@ -136,5 +137,6 @@ jobs: path: | /tmp/native-crash-*.dmp /tmp/logcat-*.txt + /tmp/js-fatal-logcat.txt if-no-files-found: warn retention-days: 7 diff --git a/examples/sdk/reactNative/android/app/build.gradle b/examples/sdk/reactNative/android/app/build.gradle index bbb3d27a..5d05f8e0 100644 --- a/examples/sdk/reactNative/android/app/build.gradle +++ b/examples/sdk/reactNative/android/app/build.gradle @@ -110,6 +110,7 @@ android { buildTypes { debug { signingConfig signingConfigs.debug + buildConfigField "boolean", "USE_DEVELOPER_SUPPORT", "true" } release { // Caution! In production, you need to generate your own keystore file. @@ -117,6 +118,8 @@ android { signingConfig signingConfigs.debug // CI-only, so run-as can read the crashpad database. debuggable project.hasProperty("btCiDebuggable") + // BuildConfig.DEBUG is true in the debuggable CI build. + buildConfigField "boolean", "USE_DEVELOPER_SUPPORT", "false" minifyEnabled enableProguardInReleaseBuilds || project.hasProperty("btCiMinify") proguardFiles getDefaultProguardFile("proguard-android.txt"), "proguard-rules.pro" } diff --git a/examples/sdk/reactNative/android/app/src/main/java/com/reactnative/MainApplication.java b/examples/sdk/reactNative/android/app/src/main/java/com/reactnative/MainApplication.java index c053d405..a0b08560 100644 --- a/examples/sdk/reactNative/android/app/src/main/java/com/reactnative/MainApplication.java +++ b/examples/sdk/reactNative/android/app/src/main/java/com/reactnative/MainApplication.java @@ -32,7 +32,7 @@ public String getJSMainModuleName() { @Override public boolean getUseDeveloperSupport() { - return BuildConfig.DEBUG; + return BuildConfig.USE_DEVELOPER_SUPPORT; } @Override diff --git a/packages/react-native/android/src/main/java/backtrace/library/BacktraceAndroidBackgroundUnhandledExceptionHandler.java b/packages/react-native/android/src/main/java/backtrace/library/BacktraceAndroidBackgroundUnhandledExceptionHandler.java index 863842b2..cee798ff 100644 --- a/packages/react-native/android/src/main/java/backtrace/library/BacktraceAndroidBackgroundUnhandledExceptionHandler.java +++ b/packages/react-native/android/src/main/java/backtrace/library/BacktraceAndroidBackgroundUnhandledExceptionHandler.java @@ -9,10 +9,14 @@ import com.facebook.react.bridge.ReactApplicationContext; import com.facebook.react.bridge.ReactContextBaseJavaModule; import com.facebook.react.bridge.ReactMethod; +import com.facebook.react.common.JavascriptException; import com.facebook.react.module.annotations.ReactModule; import java.io.PrintWriter; import java.io.StringWriter; +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; @@ -37,6 +41,12 @@ public class BacktraceAndroidBackgroundUnhandledExceptionHandler extends ReactCo private static final long REPORT_PROCESSED_TIMEOUT_MS = 5000; + private static final long FATAL_ERROR_MARK_TIMEOUT_MS = 5000; + + private static final int MAX_FATAL_ERROR_MARKS = 8; + + private final List _fatalErrorMarks = new ArrayList<>(); + public static final String NAME = "BacktraceAndroidBackgroundUnhandledExceptionHandler"; public BacktraceAndroidBackgroundUnhandledExceptionHandler(ReactApplicationContext reactContext) { @@ -66,7 +76,11 @@ public void start(Callback callback) { @Override public synchronized void uncaughtException(final Thread thread, final Throwable throwable) { try { - report(throwable); + if (isReportedFatalError(throwable)) { + Log.d(LOG_TAG, "Skipping the JavascriptException of a fatal error already reported from JavaScript."); + } else { + report(throwable); + } } catch (RuntimeException | Error ex) { Log.w(LOG_TAG, "Failed to report the unhandled exception.", ex); } finally { @@ -77,6 +91,54 @@ public synchronized void uncaughtException(final Thread thread, final Throwable } } + // The interop layer rejects a void synchronous method. + @ReactMethod(isBlockingSynchronousMethod = true) + public boolean markFatalError(String message) { + synchronized (_fatalErrorMarks) { + removeExpiredMarks(); + if (_fatalErrorMarks.size() == MAX_FATAL_ERROR_MARKS) { + _fatalErrorMarks.remove(0); + } + _fatalErrorMarks.add(new FatalErrorMark(message, System.nanoTime())); + } + return true; + } + + private boolean isReportedFatalError(Throwable throwable) { + if (!(throwable instanceof JavascriptException)) { + return false; + } + String errorMessage = errorMessageOf((JavascriptException) throwable); + synchronized (_fatalErrorMarks) { + removeExpiredMarks(); + for (Iterator marks = _fatalErrorMarks.iterator(); marks.hasNext(); ) { + if (marks.next().matches(errorMessage)) { + marks.remove(); + return true; + } + } + } + return false; + } + + private void removeExpiredMarks() { + for (Iterator marks = _fatalErrorMarks.iterator(); marks.hasNext(); ) { + if (!marks.next().isRecent()) { + marks.remove(); + } + } + } + + // React Native appends ", stack:" and the JS frames to the error message. + private static String errorMessageOf(JavascriptException exception) { + String text = exception.getMessage(); + if (text == null) { + return ""; + } + int stackStart = text.indexOf(", stack:"); + return stackStart >= 0 ? text.substring(0, stackStart) : text; + } + private void report(Throwable throwable) { Callback callback = _callback.getAndSet(null); if (callback == null) { @@ -100,10 +162,33 @@ private void waitForReportProcessing(CountDownLatch reportProcessed) { } // not synchronized: the crashing thread holds this monitor while it waits - @ReactMethod - public void reportProcessed() { + @ReactMethod(isBlockingSynchronousMethod = true) + public boolean reportProcessed() { Log.d(LOG_TAG, "Unhandled exception report processed by the JavaScript side."); _reportProcessed.countDown(); + return true; + } + + private static final class FatalErrorMark { + private final String message; + private final long markedAtNanos; + + FatalErrorMark(String message, long markedAtNanos) { + this.message = message == null ? "" : message; + this.markedAtNanos = markedAtNanos; + } + + boolean matches(String errorMessage) { + if (message.isEmpty()) { + String trimmed = errorMessage.trim(); + return trimmed.isEmpty() || trimmed.endsWith(":"); + } + return errorMessage.contains(message); + } + + boolean isRecent() { + return System.nanoTime() - markedAtNanos <= TimeUnit.MILLISECONDS.toNanos(FATAL_ERROR_MARK_TIMEOUT_MS); + } } private static String stackTraceToString(StackTraceElement[] stackTrace) { diff --git a/packages/react-native/android/src/test/java/backtraceio/library/BacktraceAndroidBackgroundUnhandledExceptionHandlerFatalErrorTest.java b/packages/react-native/android/src/test/java/backtraceio/library/BacktraceAndroidBackgroundUnhandledExceptionHandlerFatalErrorTest.java new file mode 100644 index 00000000..90a986ad --- /dev/null +++ b/packages/react-native/android/src/test/java/backtraceio/library/BacktraceAndroidBackgroundUnhandledExceptionHandlerFatalErrorTest.java @@ -0,0 +1,148 @@ +package backtraceio.library; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; + +import com.facebook.react.bridge.Callback; +import com.facebook.react.common.JavascriptException; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.TimeUnit; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +public class BacktraceAndroidBackgroundUnhandledExceptionHandlerFatalErrorTest { + private static final long WAIT_BOUND_MS = 1000; + + private Thread.UncaughtExceptionHandler previousDefaultHandler; + private final List rootHandlerThrowables = new ArrayList<>(); + private final List callbackInvocations = new ArrayList<>(); + private BacktraceAndroidBackgroundUnhandledExceptionHandler handler; + + @Before + public void installHandler() { + previousDefaultHandler = Thread.getDefaultUncaughtExceptionHandler(); + Thread.setDefaultUncaughtExceptionHandler((thread, throwable) -> rootHandlerThrowables.add(throwable)); + handler = new BacktraceAndroidBackgroundUnhandledExceptionHandler(null); + Callback callback = args -> { + callbackInvocations.add(args); + handler.reportProcessed(); + }; + handler.start(callback); + } + + @After + public void restoreDefaultHandler() { + handler.stop(); + Thread.setDefaultUncaughtExceptionHandler(previousDefaultHandler); + } + + @Test + public void markedJavascriptExceptionSkipsTheReportAndTheWait() { + JavascriptException exception = new JavascriptException("Error: boom, stack:\nanonymous@1:26921"); + handler.markFatalError("boom"); + + long elapsedMs = timed(() -> handler.uncaughtException(Thread.currentThread(), exception)); + + assertEquals(0, callbackInvocations.size()); + assertEquals(1, rootHandlerThrowables.size()); + assertSame(exception, rootHandlerThrowables.get(0)); + assertTrue("forwarded after " + elapsedMs + " ms", elapsedMs < WAIT_BOUND_MS); + } + + @Test + public void javascriptExceptionWithAnotherMessageIsReported() { + JavascriptException exception = new JavascriptException("Error: render failed, stack:\nanonymous@1:1"); + handler.markFatalError("boom"); + + handler.uncaughtException(Thread.currentThread(), exception); + + assertEquals(1, callbackInvocations.size()); + assertEquals(JavascriptException.class.getName(), callbackInvocations.get(0)[0]); + assertSame(exception, rootHandlerThrowables.get(0)); + } + + @Test + public void markWithoutMessageSkipsOnlyAJavascriptExceptionWithoutMessage() { + handler.markFatalError(""); + handler.uncaughtException(Thread.currentThread(), new JavascriptException("Error: render failed, stack:\nanonymous@1:1")); + assertEquals(1, callbackInvocations.size()); + + handler.uncaughtException(Thread.currentThread(), new JavascriptException("Error: , stack:\nanonymous@1:1")); + handler.markFatalError(""); + handler.uncaughtException(Thread.currentThread(), new JavascriptException(", stack:\nhandleException@1:1")); + + assertEquals(1, callbackInvocations.size()); + assertEquals(3, rootHandlerThrowables.size()); + } + + @Test + public void markMatchesTheErrorMessageOnlyNotTheStack() { + handler.markFatalError("anonymous"); + + handler.uncaughtException(Thread.currentThread(), new JavascriptException("Error: render failed, stack:\nanonymous@1:1")); + + assertEquals(1, callbackInvocations.size()); + } + + @Test + public void twoMarksSetBeforeEitherRethrowSkipBoth() { + handler.markFatalError("first"); + handler.markFatalError("second"); + + handler.uncaughtException(Thread.currentThread(), new JavascriptException("Error: first, stack:\nanonymous@1:1")); + handler.uncaughtException(Thread.currentThread(), new JavascriptException("TypeError: second, stack:\nanonymous@1:1")); + + assertEquals(0, callbackInvocations.size()); + assertEquals(2, rootHandlerThrowables.size()); + } + + @Test + public void unmarkedJavascriptExceptionIsReported() { + JavascriptException exception = new JavascriptException("Error: boom"); + + handler.uncaughtException(Thread.currentThread(), exception); + + assertEquals(1, callbackInvocations.size()); + assertEquals(JavascriptException.class.getName(), callbackInvocations.get(0)[0]); + assertEquals(1, rootHandlerThrowables.size()); + assertSame(exception, rootHandlerThrowables.get(0)); + } + + @Test + public void markIsConsumedByTheFirstJavascriptException() { + handler.markFatalError("boom"); + handler.uncaughtException(Thread.currentThread(), new JavascriptException("Error: boom, stack:")); + + handler.uncaughtException(Thread.currentThread(), new JavascriptException("Error: boom, stack:")); + + assertEquals(1, callbackInvocations.size()); + assertEquals(2, rootHandlerThrowables.size()); + } + + @Test + public void markDoesNotApplyToOtherExceptions() { + RuntimeException exception = new IllegalStateException("boom"); + handler.markFatalError("boom"); + + handler.uncaughtException(Thread.currentThread(), exception); + + assertEquals(1, callbackInvocations.size()); + assertEquals(IllegalStateException.class.getName(), callbackInvocations.get(0)[0]); + assertSame(exception, rootHandlerThrowables.get(0)); + } + + @Test + public void synchronousMethodsReturnAValue() { + assertTrue(handler.markFatalError("boom")); + assertTrue(handler.reportProcessed()); + } + + private static long timed(Runnable runnable) { + long start = System.nanoTime(); + runnable.run(); + return TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start); + } +} diff --git a/packages/react-native/src/handlers/UnhandledExceptionHandler.ts b/packages/react-native/src/handlers/UnhandledExceptionHandler.ts index add2ae1b..7ab0d836 100644 --- a/packages/react-native/src/handlers/UnhandledExceptionHandler.ts +++ b/packages/react-native/src/handlers/UnhandledExceptionHandler.ts @@ -1,5 +1,6 @@ import { BacktraceReport, warnFailure } from '@backtrace/sdk-core'; import { format as prettyFormat } from 'pretty-format'; +import { AppState } from 'react-native'; import type { BacktraceClient } from '../BacktraceClient'; import { hermes } from '../common/hermesHelper'; import { CrashReporter } from '../crashReporter/CrashReporter'; @@ -16,6 +17,8 @@ const rejectionTracking = require('promise/setimmediate/rejection-tracking') as type GlobalErrorHandler = ReturnType; +const FATAL_REPORT_TIMEOUT_MS = 5000; + export class UnhandledExceptionHandler implements ExceptionHandler { protected enabled = true; @@ -26,6 +29,8 @@ export class UnhandledExceptionHandler implements ExceptionHandler { private _reportingManagedError = false; private _reportingRejection = false; private _warning = false; + private _pendingFatal?: Promise; + private _escalatedError?: unknown; public captureManagedErrors(client: BacktraceClient): void { if (!this.enabled || this._installedHandler) { @@ -36,36 +41,57 @@ export class UnhandledExceptionHandler implements ExceptionHandler { this._managedClient = client; const installedHandler: GlobalErrorHandler = (error: Error, fatal?: boolean) => { + if (this._escalatedError !== undefined && error === this._escalatedError) { + this._escalatedError = undefined; + throw error; + } + const managedClient = this._managedClient; - if (this.enabled && managedClient && !this._reportingManagedError) { - this._reportingManagedError = true; - - try { - // send() can throw before returning a promise. Observe rejected - // submissions without delaying the application's fatal handler. - void Promise.resolve( - managedClient.send(error, { - 'error.type': 'Unhandled exception', - fatal, - }), - ).catch(() => undefined); - - // iOS: RCTFatal turns a fatal into a native crash the reporter would double-report. - // Preserve native reporting when synchronous JS report construction fails. - if (fatal) { - CrashReporter.markFatalError(); - } - } catch (err) { - this.warnReportingFailure('failed to report an unhandled error', err); - } finally { - this._reportingManagedError = false; - } + if (!this.enabled || !managedClient || this._reportingManagedError) { + // Disposed and re-entered wrappers still forward. + // Exceptions from the application's original handler must keep their original behavior. + previousHandler(error, fatal); + return; } - // Disposed and re-entered wrappers still forward. - // Exceptions from the application's original handler must keep their original behavior. - previousHandler(error, fatal); + let submission: Promise | undefined; + this._reportingManagedError = true; + try { + // send() can throw before returning a promise. + submission = Promise.resolve( + managedClient.send(error, { + 'error.type': 'Unhandled exception', + fatal, + }), + ).catch(() => undefined); + } catch (err) { + this.warnReportingFailure('failed to report an unhandled error', err); + } finally { + this._reportingManagedError = false; + } + + if (!fatal || !submission) { + previousHandler(error, fatal); + return; + } + + // The previous handler ends the process only in release builds. + if (__DEV__) { + previousHandler(error, fatal); + return; + } + + if (this.isInBackground()) { + // JS timers are paused in the background. + this.markFatalErrorSafely(error); + previousHandler(error, fatal); + return; + } + + // The previous handler ends the process. + const settled = this.settle(submission, FATAL_REPORT_TIMEOUT_MS); + this.queueFatalForwarding(() => settled.then(() => this.forwardFatal(previousHandler, error, fatal))); }; try { @@ -142,6 +168,79 @@ export class UnhandledExceptionHandler implements ExceptionHandler { // could disable a newer tracker installed by another SDK. } + protected markFatalError: (message: string) => void = () => CrashReporter.markFatalError(); + + private queueFatalForwarding(forward: () => Promise | void): void { + const pending = (this._pendingFatal ?? Promise.resolve()).then(forward).then(() => { + if (this._pendingFatal === pending) { + this._pendingFatal = undefined; + } + }); + this._pendingFatal = pending; + } + + private forwardFatal(previousHandler: GlobalErrorHandler, error: Error, fatal: boolean): void { + this.markFatalErrorSafely(error); + try { + previousHandler(error, fatal); + } catch (err) { + // React Native's native fallback ends the process only for a throw out of the global handler. + this._escalatedError = err; + setTimeout(() => { + throw err; + }, 0); + } + } + + private markFatalErrorSafely(error: unknown): void { + try { + this.markFatalError(this.fatalErrorMessage(error)); + } catch (err) { + this.warnReportingFailure('failed to mark the fatal error for the native handler', err); + } + } + + // Mirrors the message React Native builds for the rethrown error. + private fatalErrorMessage(error: unknown): string { + if (error instanceof Error) { + return error.message ? String(error.message) : ''; + } + return error === undefined ? '' : String(error); + } + + private isInBackground(): boolean { + try { + return AppState.currentState === 'background'; + } catch { + return false; + } + } + + private settle(promise: Promise, timeoutMs: number): Promise { + return new Promise((resolve) => { + const done = () => { + clearTimeout(timer); + subscription?.remove(); + resolve(); + }; + const timer = setTimeout(done, timeoutMs); + const subscription = this.onBackground(done); + promise.then(done, done); + }); + } + + private onBackground(callback: () => void): { remove(): void } | undefined { + try { + return AppState.addEventListener('change', (state) => { + if (state === 'background') { + callback(); + } + }); + } catch { + return undefined; + } + } + private reportRejection(id: number, rejection: unknown = 'Unknown'): void { const client = this._rejectionClient; if (!this.enabled || !client || this._reportingRejection) { diff --git a/packages/react-native/src/handlers/android/AndroidUnhandledExceptionHandler.ts b/packages/react-native/src/handlers/android/AndroidUnhandledExceptionHandler.ts index f2beeb4c..74029894 100644 --- a/packages/react-native/src/handlers/android/AndroidUnhandledExceptionHandler.ts +++ b/packages/react-native/src/handlers/android/AndroidUnhandledExceptionHandler.ts @@ -5,6 +5,9 @@ import { DebuggerHelper } from '../../common/DebuggerHelper'; import { AndroidStackTraceConverter } from '../../converters/AndroidStackTraceConverter'; import { UnhandledExceptionHandler } from '../UnhandledExceptionHandler'; import { AndroidUnhandledException } from './AndroidUnhandledException'; + +const JAVASCRIPT_EXCEPTION = 'com.facebook.react.common.JavascriptException'; + export class AndroidUnhandledExceptionHandler extends UnhandledExceptionHandler { private readonly _unhandledExceptionHandler = NativeModules.BacktraceAndroidBackgroundUnhandledExceptionHandler; private readonly _androidStackTraceConverter = new AndroidStackTraceConverter(); @@ -27,7 +30,12 @@ export class AndroidUnhandledExceptionHandler extends UnhandledExceptionHandler [], ); report.addStackTrace('main', this._androidStackTraceConverter.convert(stackTrace)); - await client.send(report); + const submission = client.send(report); + if (classifier === JAVASCRIPT_EXCEPTION) { + // The upload's response needs the native modules thread, the one that crashed. + this._unhandledExceptionHandler.reportProcessed?.(); + } + await submission; } catch { // nothing to recover: the process is dying } finally { @@ -42,4 +50,9 @@ export class AndroidUnhandledExceptionHandler extends UnhandledExceptionHandler this._unhandledExceptionHandler.stop(); } } + + // React Native rethrows the fatal as a JavascriptException the Java handler must not report again. + protected markFatalError = (message: string) => { + this._unhandledExceptionHandler?.markFatalError?.(message); + }; } diff --git a/packages/react-native/tests/androidUnhandledExceptionHandlerTests.spec.ts b/packages/react-native/tests/androidUnhandledExceptionHandlerTests.spec.ts index 4dea7597..b78ad9cf 100644 --- a/packages/react-native/tests/androidUnhandledExceptionHandlerTests.spec.ts +++ b/packages/react-native/tests/androidUnhandledExceptionHandlerTests.spec.ts @@ -24,11 +24,13 @@ jest.mock('../src/common/DebuggerHelper', () => ({ })); import { NativeModules } from 'react-native'; +import { CrashReporter } from '../src/crashReporter/CrashReporter'; const nativeHandlerMock = { start: jest.fn(), stop: jest.fn(), reportProcessed: jest.fn(), + markFatalError: jest.fn(), }; NativeModules.BacktraceAndroidBackgroundUnhandledExceptionHandler = nativeHandlerMock; @@ -37,23 +39,39 @@ NativeModules.BacktraceAndroidBackgroundUnhandledExceptionHandler = nativeHandle const { AndroidUnhandledExceptionHandler } = require('../src/handlers/android/AndroidUnhandledExceptionHandler'); type NativeExceptionCallback = (classifier: string, message: string, stackTrace: string) => Promise; +type GlobalHandler = (error: Error, fatal?: boolean) => void; + +const flush = () => new Promise((resolve) => setImmediate(resolve)); describe('AndroidUnhandledExceptionHandler', () => { let originalErrorUtils: unknown; + let previousGlobalHandler: jest.Mock; + let registeredHandler: GlobalHandler; + let originalDev: PropertyDescriptor | undefined; beforeEach(() => { + originalDev = Object.getOwnPropertyDescriptor(globalThis, '__DEV__'); + Object.defineProperty(globalThis, '__DEV__', { configurable: true, value: false }); jest.clearAllMocks(); mockIsNativeBridgeEnabled.mockReturnValue(true); + previousGlobalHandler = jest.fn(); originalErrorUtils = (global as unknown as { ErrorUtils?: unknown }).ErrorUtils; (global as unknown as { ErrorUtils: unknown }).ErrorUtils = { - getGlobalHandler: () => jest.fn(), - setGlobalHandler: jest.fn(), + getGlobalHandler: () => previousGlobalHandler, + setGlobalHandler: (fn: GlobalHandler) => { + registeredHandler = fn; + }, }; }); afterEach(() => { (global as unknown as { ErrorUtils: unknown }).ErrorUtils = originalErrorUtils; + if (originalDev) { + Object.defineProperty(globalThis, '__DEV__', originalDev); + } else { + Reflect.deleteProperty(globalThis, '__DEV__'); + } }); function captureNativeCallback(client: BacktraceClient): NativeExceptionCallback { @@ -81,6 +99,16 @@ describe('AndroidUnhandledExceptionHandler', () => { expect(nativeHandlerMock.reportProcessed).toHaveBeenCalledTimes(1); }); + it('Should signal reportProcessed before the send settles for a JavascriptException', async () => { + const sendMock = jest.fn().mockReturnValue(new Promise(() => undefined)); + const callback = captureNativeCallback({ send: sendMock } as unknown as BacktraceClient); + + void callback('com.facebook.react.common.JavascriptException', 'Error: boom', 'a.b(C.java:1)'); + + expect(sendMock).toHaveBeenCalledTimes(1); + expect(nativeHandlerMock.reportProcessed).toHaveBeenCalledTimes(1); + }); + it('Should signal reportProcessed when the send fails', async () => { const sendMock = jest.fn().mockRejectedValue(new Error('offline')); const callback = captureNativeCallback({ send: sendMock } as unknown as BacktraceClient); @@ -108,6 +136,69 @@ describe('AndroidUnhandledExceptionHandler', () => { expect(nativeHandlerMock.start).not.toHaveBeenCalled(); }); + it('Should mark the fatal error on the Java handler, not the crash reporter, before forwarding it', async () => { + const sendMock = jest.fn().mockResolvedValue(undefined); + new AndroidUnhandledExceptionHandler().captureManagedErrors({ send: sendMock } as unknown as BacktraceClient); + const error = new Error('boom'); + + registeredHandler(error, true); + await flush(); + + expect(nativeHandlerMock.markFatalError).toHaveBeenCalledTimes(1); + expect(nativeHandlerMock.markFatalError).toHaveBeenCalledWith('boom'); + expect(CrashReporter.markFatalError).not.toHaveBeenCalled(); + expect(previousGlobalHandler).toHaveBeenCalledWith(error, true); + expect(nativeHandlerMock.markFatalError.mock.invocationCallOrder[0]).toBeLessThan( + previousGlobalHandler.mock.invocationCallOrder[0], + ); + }); + + it('Should pass the message of a thrown non-Error value to the Java handler', async () => { + new AndroidUnhandledExceptionHandler().captureManagedErrors({ + send: jest.fn().mockResolvedValue(undefined), + } as unknown as BacktraceClient); + + registeredHandler('plain failure' as unknown as Error, true); + await flush(); + + expect(nativeHandlerMock.markFatalError).toHaveBeenCalledWith('plain failure'); + }); + + it.each([ + ['undefined', undefined, ''], + ['an Error without a message', new Error(), ''], + ['null', null, 'null'], + ['the number 0', 0, '0'], + ])("Should pass React Native's message for %s to the Java handler", async (_label, thrown, expected) => { + new AndroidUnhandledExceptionHandler().captureManagedErrors({ + send: jest.fn().mockResolvedValue(undefined), + } as unknown as BacktraceClient); + + registeredHandler(thrown as unknown as Error, true); + await flush(); + + expect(nativeHandlerMock.markFatalError).toHaveBeenCalledWith(expected); + }); + + it('Should forward a fatal error when the Java handler module has no markFatalError', async () => { + const { markFatalError, ...olderModule } = nativeHandlerMock; + NativeModules.BacktraceAndroidBackgroundUnhandledExceptionHandler = olderModule; + try { + new AndroidUnhandledExceptionHandler().captureManagedErrors({ + send: jest.fn().mockResolvedValue(undefined), + } as unknown as BacktraceClient); + const error = new Error('boom'); + + registeredHandler(error, true); + await flush(); + + expect(markFatalError).not.toHaveBeenCalled(); + expect(previousGlobalHandler).toHaveBeenCalledWith(error, true); + } finally { + NativeModules.BacktraceAndroidBackgroundUnhandledExceptionHandler = nativeHandlerMock; + } + }); + it('Should stop the native handler on dispose', () => { const handler = new AndroidUnhandledExceptionHandler(); handler.captureManagedErrors({ send: jest.fn() } as unknown as BacktraceClient); diff --git a/packages/react-native/tests/errorHandlerSafetyTests.spec.ts b/packages/react-native/tests/errorHandlerSafetyTests.spec.ts index f5bef38c..9e4aaad3 100644 --- a/packages/react-native/tests/errorHandlerSafetyTests.spec.ts +++ b/packages/react-native/tests/errorHandlerSafetyTests.spec.ts @@ -25,6 +25,7 @@ const rejectionTracking = require('promise/setimmediate/rejection-tracking') as }; type Handler = (error: Error, fatal?: boolean) => void; +const flush = () => new Promise((resolve) => setImmediate(resolve)); type RejectionOptions = { allRejections: boolean; onUnhandled: (id: number, rejection?: unknown) => void; @@ -91,28 +92,71 @@ describe('UnhandledExceptionHandler safety', () => { expect(warn).toHaveBeenCalledWith('Backtrace: failed to report an unhandled error (Error)'); }); - it('observes asynchronous failures without delaying delegation or fatal marking', async () => { + it('delegates a fatal error once the failed submission settles and still marks it', async () => { send.mockRejectedValue(new Error('transport failure')); handler.captureManagedErrors(client); const error = new Error('application failure'); current(error, true); + expect(previous).not.toHaveBeenCalled(); + await flush(); expect(previous).toHaveBeenCalledTimes(1); expect(previous).toHaveBeenCalledWith(error, true); expect(CrashReporter.markFatalError).toHaveBeenCalledTimes(1); - // A rejected submission must be handled in the same turn, - // even if the process remains alive after the application's handler returns. - await new Promise((resolve) => setImmediate(resolve)); + expect(warn).not.toHaveBeenCalled(); }); - it('delegates even when native fatal marking fails', () => { + it('delegates even when native fatal marking fails', async () => { (CrashReporter.markFatalError as jest.Mock).mockImplementation(() => { throw new Error('native marker failed'); }); handler.captureManagedErrors(client); const error = new Error('application failure'); expect(() => current(error, true)).not.toThrow(); + await flush(); + expect(previous).toHaveBeenCalledTimes(1); + expect(previous).toHaveBeenCalledWith(error, true); + expect(warn).toHaveBeenCalledWith( + 'Backtrace: failed to mark the fatal error for the native handler (Error)', + ); + }); + + it('rethrows a failing previous fatal handler from a timer without reporting it again', async () => { + const failure = new Error('previous handler failure'); + previous.mockImplementation((_error: Error, fatal?: boolean) => { + if (fatal) { + throw failure; + } + }); + const realSetTimeout = setTimeout; + let escalation: (() => void) | undefined; + jest.spyOn(globalThis, 'setTimeout').mockImplementation(((fn: () => void, ms?: number) => { + if (ms === 0) { + escalation = fn; + return 0 as unknown as NodeJS.Timeout; + } + return realSetTimeout(fn, ms); + }) as typeof setTimeout); + handler.captureManagedErrors(client); + current(new Error('application failure'), true); + await flush(); + expect(previous).toHaveBeenCalledTimes(1); + expect(escalation).toBeDefined(); + expect(() => escalation?.()).toThrow(failure); + expect(() => current(failure, true)).toThrow(failure); + expect(send).toHaveBeenCalledTimes(1); + expect(previous).toHaveBeenCalledTimes(1); + }); + + it('forwards a fatal error synchronously when synchronous reporting fails', () => { + send.mockImplementation(() => { + throw new Error('report construction failure'); + }); + handler.captureManagedErrors(client); + const error = new Error('application failure'); + current(error, true); expect(previous).toHaveBeenCalledTimes(1); expect(previous).toHaveBeenCalledWith(error, true); + expect(CrashReporter.markFatalError).not.toHaveBeenCalled(); }); it('does not swallow exceptions from the previous global handler', () => { diff --git a/packages/react-native/tests/unhandledExceptionHandlerTests.spec.ts b/packages/react-native/tests/unhandledExceptionHandlerTests.spec.ts index ae01897d..e6e962f3 100644 --- a/packages/react-native/tests/unhandledExceptionHandlerTests.spec.ts +++ b/packages/react-native/tests/unhandledExceptionHandlerTests.spec.ts @@ -21,11 +21,19 @@ jest.mock('../src/common/hermesHelper', () => ({ // eslint-disable-next-line @typescript-eslint/no-var-requires const rejectionTracking = require('promise/setimmediate/rejection-tracking'); +import { AppState } from 'react-native'; import { CrashReporter } from '../src/crashReporter/CrashReporter'; import { UnhandledExceptionHandler } from '../src/handlers/UnhandledExceptionHandler'; const markFatalErrorMock = CrashReporter.markFatalError as jest.Mock; +const flush = () => new Promise((resolve) => setImmediate(resolve)); +const flushMicrotasks = async () => { + for (let i = 0; i < 10; i++) { + await Promise.resolve(); + } +}; + describe('UnhandledExceptionHandler labeling', () => { let sendMock: jest.Mock; let client: BacktraceClient; @@ -80,8 +88,11 @@ describe('UnhandledExceptionHandler managed errors', () => { let registeredHandler: (error: Error, fatal?: boolean) => void; let previousGlobalHandler: jest.Mock; let originalErrorUtils: unknown; + let originalDev: PropertyDescriptor | undefined; beforeEach(() => { + originalDev = Object.getOwnPropertyDescriptor(globalThis, '__DEV__'); + Object.defineProperty(globalThis, '__DEV__', { configurable: true, value: false }); markFatalErrorMock.mockClear(); sendMock = jest.fn(); client = { send: sendMock } as unknown as BacktraceClient; @@ -101,18 +112,199 @@ describe('UnhandledExceptionHandler managed errors', () => { afterEach(() => { (global as unknown as { ErrorUtils: unknown }).ErrorUtils = originalErrorUtils; + if (originalDev) { + Object.defineProperty(globalThis, '__DEV__', originalDev); + } else { + Reflect.deleteProperty(globalThis, '__DEV__'); + } + jest.restoreAllMocks(); }); - it('Should mark a fatal unhandled error so the native reporter skips the duplicate, then chain the previous handler', () => { + it('Should forward a fatal unhandled error only after the report settles, marking it for the native reporter first', async () => { + let resolveSend!: () => void; + sendMock.mockReturnValue( + new Promise((resolve) => { + resolveSend = resolve; + }), + ); const error = new Error('boom'); + registeredHandler(error, true); expect(sendMock).toHaveBeenCalledWith(error, { 'error.type': 'Unhandled exception', fatal: true }); + expect(markFatalErrorMock).not.toHaveBeenCalled(); + expect(previousGlobalHandler).not.toHaveBeenCalled(); + + resolveSend(); + await flush(); + expect(markFatalErrorMock).toHaveBeenCalledTimes(1); expect(previousGlobalHandler).toHaveBeenCalledWith(error, true); + expect(markFatalErrorMock.mock.invocationCallOrder[0]).toBeLessThan( + previousGlobalHandler.mock.invocationCallOrder[0], + ); + }); + + it('Should forward a fatal unhandled error when the report fails', async () => { + sendMock.mockRejectedValue(new Error('offline')); + const error = new Error('boom'); + + registeredHandler(error, true); + await flush(); + + expect(markFatalErrorMock).toHaveBeenCalledTimes(1); + expect(previousGlobalHandler).toHaveBeenCalledTimes(1); + expect(previousGlobalHandler).toHaveBeenCalledWith(error, true); + }); + + it('Should forward a fatal unhandled error after 5 seconds when the report never settles', async () => { + jest.useFakeTimers(); + try { + sendMock.mockReturnValue(new Promise(() => undefined)); + const error = new Error('boom'); + + registeredHandler(error, true); + await flushMicrotasks(); + expect(previousGlobalHandler).not.toHaveBeenCalled(); + + jest.advanceTimersByTime(4999); + await flushMicrotasks(); + expect(previousGlobalHandler).not.toHaveBeenCalled(); + + jest.advanceTimersByTime(1); + await flushMicrotasks(); + expect(markFatalErrorMock).toHaveBeenCalledTimes(1); + expect(previousGlobalHandler).toHaveBeenCalledWith(error, true); + } finally { + jest.useRealTimers(); + } + }); + + it('Should forward a fatal error at once and mark it when the app is in the background', () => { + const appState = AppState as unknown as { currentState: unknown }; + const original = appState.currentState; + appState.currentState = 'background'; + try { + sendMock.mockReturnValue(new Promise(() => undefined)); + const error = new Error('boom'); + + registeredHandler(error, true); + + expect(sendMock).toHaveBeenCalledTimes(1); + expect(markFatalErrorMock).toHaveBeenCalledTimes(1); + expect(previousGlobalHandler).toHaveBeenCalledWith(error, true); + } finally { + appState.currentState = original; + } + }); + + it('Should forward a pending fatal error once the app goes to the background', async () => { + let onChange!: (state: string) => void; + jest.spyOn(AppState, 'addEventListener').mockImplementation((( + _type: string, + listener: (state: string) => void, + ) => { + onChange = listener; + return { remove: jest.fn() }; + }) as unknown as typeof AppState.addEventListener); + sendMock.mockReturnValue(new Promise(() => undefined)); + const error = new Error('boom'); + + registeredHandler(error, true); + await flush(); + expect(previousGlobalHandler).not.toHaveBeenCalled(); + + onChange('inactive'); + await flush(); + expect(previousGlobalHandler).not.toHaveBeenCalled(); + + onChange('background'); + await flush(); + + expect(markFatalErrorMock).toHaveBeenCalledTimes(1); + expect(previousGlobalHandler).toHaveBeenCalledWith(error, true); + expect(markFatalErrorMock.mock.invocationCallOrder[0]).toBeLessThan( + previousGlobalHandler.mock.invocationCallOrder[0], + ); + }); + + it('Should stop listening for the background once the report settles', async () => { + const remove = jest.fn(); + jest.spyOn(AppState, 'addEventListener').mockReturnValue({ + remove, + } as unknown as ReturnType); + sendMock.mockResolvedValue(undefined); + + registeredHandler(new Error('boom'), true); + await flush(); + + expect(previousGlobalHandler).toHaveBeenCalledTimes(1); + expect(remove).toHaveBeenCalled(); + }); + + it('Should forward a fatal error at once without marking it in a debug build', () => { + Object.defineProperty(globalThis, '__DEV__', { configurable: true, value: true }); + sendMock.mockReturnValue(new Promise(() => undefined)); + const error = new Error('boom'); + + registeredHandler(error, true); + + expect(sendMock).toHaveBeenCalledWith(error, { 'error.type': 'Unhandled exception', fatal: true }); + expect(markFatalErrorMock).not.toHaveBeenCalled(); + expect(previousGlobalHandler).toHaveBeenCalledWith(error, true); + }); + + it('Should report a second fatal error while the first is pending and forward both in order', async () => { + let resolveSend!: () => void; + sendMock.mockReturnValue( + new Promise((resolve) => { + resolveSend = resolve; + }), + ); + const first = new Error('first'); + const second = new Error('second'); + + registeredHandler(first, true); + registeredHandler(second, true); + + expect(sendMock).toHaveBeenCalledTimes(2); + expect(previousGlobalHandler).not.toHaveBeenCalled(); + + resolveSend(); + await flush(); + + expect(previousGlobalHandler.mock.calls).toEqual([ + [first, true], + [second, true], + ]); + }); + + it('Should report fatal errors again once the pending one is forwarded', async () => { + sendMock.mockResolvedValue(undefined); + + registeredHandler(new Error('first'), true); + await flush(); + registeredHandler(new Error('later'), true); + await flush(); + + expect(sendMock).toHaveBeenCalledTimes(2); + expect(previousGlobalHandler).toHaveBeenCalledTimes(2); + }); + + it('Should keep reporting non-fatal errors while a fatal error is pending', async () => { + sendMock.mockReturnValue(new Promise(() => undefined)); + const fatal = new Error('fatal'); + const nonFatal = new Error('warning'); + + registeredHandler(fatal, true); + registeredHandler(nonFatal, false); + + expect(sendMock).toHaveBeenCalledTimes(2); + expect(previousGlobalHandler).toHaveBeenCalledTimes(1); + expect(previousGlobalHandler).toHaveBeenCalledWith(nonFatal, false); }); - it('Should NOT mark a non-fatal unhandled error', () => { + it('Should forward a non-fatal unhandled error synchronously without marking it', () => { const error = new Error('boom'); registeredHandler(error, false);