Skip to content

feat: web-compliant error handling (unhandled rejections, error events, interop.escapeException)#409

Open
edusperoni wants to merge 4 commits into
feat/update-libffifrom
feat/error-handling
Open

feat: web-compliant error handling (unhandled rejections, error events, interop.escapeException)#409
edusperoni wants to merge 4 commits into
feat/update-libffifrom
feat/error-handling

Conversation

@edusperoni

@edusperoni edusperoni commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Stacked on #408 (libffi 3.7.1 update) — only the last 3 commits are new here. The escapeException feature relies on the updated libffi's ObjC exception unwinding through closure trampolines (validated on arm64 simulator against the shipped slices).

What this does

Adopts the WHATWG error model at the runtime level so @nativescript/core no longer needs bespoke error hooks, closes the silently-swallowed-rejections hole, and adds an explicit escape hatch for forwarding real native exceptions to native callers.

1. Unhandled promise rejection tracking

The runtime never registered SetPromiseRejectCallback, so unhandled rejections vanished with no log, no callback, no crash. Now:

  • Rejections without handlers are tracked per-isolate and drained once per runloop turn (kCFRunLoopBeforeWaiting observer).
  • Unhandled ones report through the same machinery as uncaught sync exceptions (__onUncaughtError/__onDiscardedError, honoring discardUncaughtJsExceptions), prefixed Unhandled promise rejection:.
  • A handler attached before the drain cancels the report. Worker rejections flow through the existing worker error channel (worker-global onerror → main isolate's worker.onerror).

2. WHATWG error events on globalThis

Spec-shaped Event, EventTarget, ErrorEvent, PromiseRejectionEvent constructors, addEventListener/removeEventListener/dispatchEvent on globalThis, and global reportError() — same surface as workers/Deno, no window needed.

  • Uncaught errors dispatch a cancelable error ErrorEvent; unhandled rejections a cancelable unhandledrejection PromiseRejectionEvent. preventDefault() = fully handled (no shim, no fatal log, no error modal).
  • Late-attached handlers fire rejectionhandled (as a task, per spec, carrying the original reason; already-reported promises held phantom-weak so GC'd ones drop out).
  • Native code dispatches through closures captured at init, so events keep firing even if app code overwrites globalThis.dispatchEvent.
  • Back-compat: unprevented errors behave byte-for-byte as before — __onUncaughtError/__onDiscardedError keep working, so current core needs zero changes (they're now deprecated shims; a follow-up core PR can migrate application-common.ts to globalThis.addEventListener).

3. interop.escapeException + boundary hardening

  • Native calls that throw now preserve the original NSException as error.nativeException (name/message from the exception) — re-enables the long-dormant Throw_ObjC_exceptions_to_JavaScript test, now on simulator too.
  • throw interop.escapeException(x) from a JS override/block converts to a real @throw into the native caller at the boundary — a parent native @try/@catch sees the original NSException (or one synthesized from the JS error, with the JS stack in userInfo). Throws happen strictly after every V8 scope has destructed; ObjC exceptions never unwind through live V8 frames.
  • The tns::Assert process-aborts on JS throws in property getters/setters and the Array/Dictionary/FastEnumeration adapters are gone — those boundaries now report through the uncaught path and return a defined default.
  • New opt-in crashOnUncaughtJsExceptions config (default off): unprevented fatal errors schedule an uncaught @throw on the runtime loop from a clean frame, so crash reporters capture a real native crash with the JS stack attached.

Before / after examples

Unhandled rejections

// BEFORE: this vanished — no log, no hook, no crash. The error object was
// simply garbage-collected.
Promise.reject(new Error("boom"));

// AFTER: logged as "Unhandled promise rejection:" through the same pipeline
// as uncaught errors (Application.uncaughtErrorEvent still fires via the
// __onUncaughtError shim), and observable the standard web way:
globalThis.addEventListener("unhandledrejection", (e) => {
  console.log(e.reason.message); // "boom"
  console.log(e.promise);        // the rejected promise
  e.preventDefault();            // handled: no shim, no fatal log, no modal
});

// Fires when a handler is attached after the rejection was already reported,
// carrying the original reason:
globalThis.addEventListener("rejectionhandled", (e) => {
  console.log("late catch for", e.reason);
});

Error events and reportError

// AFTER (new surface — before, the only hooks were the private
// __onUncaughtError/__onDiscardedError globals that core had to install):
globalThis.addEventListener("error", (e) => {
  crashReporter.record(e.error);  // any uncaught JS exception
  e.preventDefault();             // fully handles it
});

// Route a caught-but-fatal error through the exact same pipeline:
reportError(new Error("something unrecoverable"));

Catching native exceptions in JS

try {
  NSArray.alloc().init().objectAtIndex(3);
} catch (e) {
  // BEFORE: e was a generic Error holding only [exception description];
  // the original NSException object was lost.
  // AFTER:
  e.nativeException instanceof NSException; // true
  e.nativeException.name;                   // "NSRangeException"
  e.nativeException.reason;                 // "... index 3 beyond bounds ..."
}

Forwarding exceptions to a native caller (interop.escapeException)

// BEFORE: a throw inside a JS override was swallowed — the native caller
// resumed with a zeroed return value and no way to observe the failure.
// AFTER: brand the throw and the boundary converts it to a real @throw that
// a native @try/@catch above the caller can handle (or that crashes with a
// real native report if nothing catches it):
const Delegate = NSObject.extend({
  someNativeMethod() {
    throw interop.escapeException(new Error("propagate me natively"));
  },
}, { protocols: [SomeProtocol] });

// Rethrowing a caught native exception preserves the ORIGINAL NSException
// object, so a parent @catch (NSRangeException*) still matches:
try {
  riskyNativeCall();
} catch (e) {
  throw interop.escapeException(e);
}

// Plain (unbranded) throws keep today's semantics: reported through the
// uncaught path, native caller gets a defined default value.

Boundary hardening

// BEFORE: a JS throw inside a property getter/setter override or a
// Dictionary/Array adapter callback hit tns::Assert and ABORTED THE PROCESS.
// AFTER: the error is reported (error event + __onUncaughtError) and the
// native caller receives a defined default (nil/0/NO) — no abort.
const Obj = NSObject.extend({
  get someProperty() { throw new Error("oops"); }, // no longer kills the app
});

Opt-in crash on fatal errors

// app/package.json (same place as discardUncaughtJsExceptions), default off:
{ "crashOnUncaughtJsExceptions": true }

When enabled, an error that no listener preventDefault()s crashes the app with a real NSException (JS stack in the reason and userInfo["JavaScriptStack"]), thrown from a clean frame on the runtime loop so crash reporters capture it properly.

Behavior changes (default config)

  • Unhandled rejections are now reported (previously silent). Everything else is opt-in (preventDefault(), the new config flag) or identical.
  • JS throws in property accessors/adapters no longer abort the process.

Testing

  • Full suite: 900 tests, 0 failures (was 869) — 31 new specs: unhandled rejections (5), WHATWG events (17), escapeException/boundary hardening (8+), plus the re-enabled ObjC-exception test.
  • Guards kept green: ApiTests NSError shape (code/domain/nativeException), ExceptionHandlingTests, Promise thread-affinity, TNS Workers.
  • New ObjC fixtures (TNSTestNativeCallbacks) exercise catch-in-native round trips.
  • Outstanding: a device-hardware pass before release; crashOnUncaughtJsExceptions manual smoke (documented in EscapeExceptionTests.js).

Register SetPromiseRejectCallback and track rejected promises without
handlers in a per-isolate PromiseRejectionTracker (owned by Caches).
A kCFRunLoopBeforeWaiting observer drains the pending list once per
runloop turn and reports each rejection through the same
__onUncaughtError/__onDiscardedError machinery as uncaught sync
exceptions (log-prefixed 'Unhandled promise rejection:'). A handler
attached before the drain cancels the report. Worker isolates give the
worker-global onerror a chance first, then forward to the main
isolate's worker.onerror like uncaught worker errors.

OnUncaughtError's reporting tail is refactored into the shared
ReportToJsHandlersAndLog helper (behavior preserved); the observer
polls an atomic pending count since rejections can arrive from any
thread holding the v8::Locker, and the drain catches NSExceptions so
they cannot unwind through the observer's live V8 scopes.
Add a JS bootstrap (InitErrorEvents, evaluated for main and worker
isolates right after PromiseProxy::Init) providing spec-shaped Event,
EventTarget, ErrorEvent and PromiseRejectionEvent constructors, the
EventTarget methods on globalThis, and a global reportError().

Uncaught exceptions now dispatch a cancelable 'error' ErrorEvent and
unhandled rejections a cancelable 'unhandledrejection'
PromiseRejectionEvent before reporting; preventDefault() fully handles
the error (no __on* shim, no fatal log, no modal). Unprevented errors
keep the existing behavior byte-for-byte, so NativeScript core's
__onUncaughtError/__onDiscardedError hooks continue to work unchanged.
A handler attached after reporting fires 'rejectionhandled' as a task
on the next drain turn; already-reported promises are held phantom-weak
so GC'd promises drop out.

Native code dispatches through closures returned by the bootstrap IIFE
(stashed as Persistents in Caches), so the events keep firing even if
app code overwrites globalThis.dispatchEvent. Listener-thrown errors
route to the fatal tail without recursive dispatch.
Preserve the original NSException when a native call throws: the JS
error now carries name/message from the exception and the wrapped
original as error.nativeException (re-enables the long-dormant
Throw_ObjC_exceptions_to_JavaScript test everywhere - libffi 3.7.1
fixed simulator unwinding).

Add interop.escapeException(x): returns a JS Error branded with an
isolate-private symbol carrying either the original NSException or
synthesis info. When a branded throw reaches a JS-to-native boundary
(overridden methods and JS-backed blocks via ArgConverter's
MethodCallback, property accessors, adapters), the boundary converts it
to a real @throw into the native caller - always after every V8 scope
has destructed (pendingThrow captured before the scopes, thrown after
the inner block). Unbranded throws keep today's semantics: ReThrow to
the message listener in MethodCallback, and report-once-plus-defined-
default at the former tns::Assert abort sites (property getters/
setters, Array/Dictionary/FastEnumeration adapters no longer abort the
process on a JS throw).

Add opt-in crashOnUncaughtJsExceptions (default off): unprevented fatal
errors schedule an uncaught @throw on the runtime loop from a clean,
V8-scope-free frame so crash reporters capture a real native crash with
the JS stack in userInfo. Branded escapes reaching the fatal tail (e.g.
thrown from an error-event listener) always take that path.
@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 41.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main additions: error events, unhandled rejections, and interop.escapeException.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@NativeScript/runtime/ArgConverter.h`:
- Around line 68-85: Update GetEscapeExceptionBrand to validate the cache with
cache->IsValid() before calling v8::Private::New or creating any persistent
state. Preserve the documented empty-handle return for both null and invalid
caches, including the dummy cache returned after removal.

In `@NativeScript/runtime/MetadataBuilder.mm`:
- Around line 1166-1170: Update the fallback wrapper creation in the setter path
around self_ to retain the newly created ObjCDataWrapper and invoke
tns::DeleteWrapperIfUnused(...) after creating the JavaScript wrapper, mirroring
the getter cleanup. Preserve the cached-instance path and ensure cleanup applies
only when thiz is not already cached.

In `@NativeScript/runtime/NativeScriptException.h`:
- Around line 66-71: Update the rejection tracking state around
DispatchRejectionHandledEvent and reportedOutstanding_ so each reported promise
retains its original rejection reason alongside the promise. When moving entries
into pendingRejectionHandled_, carry that stored reason through and dispatch
rejectionhandled with it instead of allowing reason to become undefined.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: f59b87c7-4598-4217-955d-2ba7d862ab38

📥 Commits

Reviewing files that changed from the base of the PR and between f41d7e5 and b36ab05.

📒 Files selected for processing (25)
  • NativeScript/runtime/ArgConverter.h
  • NativeScript/runtime/ArgConverter.mm
  • NativeScript/runtime/ArrayAdapter.mm
  • NativeScript/runtime/Caches.cpp
  • NativeScript/runtime/Caches.h
  • NativeScript/runtime/ClassBuilder.mm
  • NativeScript/runtime/DataWrapper.h
  • NativeScript/runtime/DictionaryAdapter.mm
  • NativeScript/runtime/FastEnumerationAdapter.mm
  • NativeScript/runtime/Interop.h
  • NativeScript/runtime/Interop.mm
  • NativeScript/runtime/InteropTypes.mm
  • NativeScript/runtime/MetadataBuilder.mm
  • NativeScript/runtime/NativeScriptException.h
  • NativeScript/runtime/NativeScriptException.mm
  • NativeScript/runtime/Runtime.h
  • NativeScript/runtime/Runtime.mm
  • NativeScript/runtime/WorkerWrapper.mm
  • TestFixtures/TNSTestNativeCallbacks.h
  • TestFixtures/TNSTestNativeCallbacks.m
  • TestRunner/app/tests/ErrorEventsTests.js
  • TestRunner/app/tests/EscapeExceptionTests.js
  • TestRunner/app/tests/MethodCallsTests.js
  • TestRunner/app/tests/Promises.js
  • TestRunner/app/tests/index.js

Comment thread NativeScript/runtime/ArgConverter.h
Comment thread NativeScript/runtime/MetadataBuilder.mm Outdated
Comment thread NativeScript/runtime/NativeScriptException.h
…jectionhandled reason)

- GetEscapeExceptionBrand now checks Caches::IsValid() — Caches::Get
  returns a dummy cache (never null) after isolate disposal, so the
  nullptr check could not honor the documented empty-handle contract.
- The property setter's fallback ObjCDataWrapper is now released via
  DeleteWrapperIfUnused, mirroring the getter (pre-existing asymmetry).
- rejectionhandled now carries the original rejection reason:
  reportedOutstanding_/pendingRejectionHandled_ retain {promise, reason}
  pairs (promise phantom-weak, so a GC'd promise still drops the entry
  and its reason), per PromiseRejectionEvent semantics.
@edusperoni

Copy link
Copy Markdown
Collaborator Author

Addressed all three review findings in 0d1e372 (full suite re-run: 900/900 green):

  1. GetEscapeExceptionBrand liveness — now checks cache->IsValid(); Caches::Get returns a dummy cache (never null) after isolate disposal, so the previous nullptr check was dead. Invalid caches return the documented empty handle, which every caller already handles.
  2. Setter wrapper leak (MetadataBuilder.mm) — the fallback ObjCDataWrapper in the property setter is now released via DeleteWrapperIfUnused, mirroring the getter. Note this asymmetry pre-dates this PR (it existed before the boundary rework), but it lives in code this PR touches so it's fixed here.
  3. rejectionhandled reason — the tracker now retains {promise, reason} pairs; the promise handle stays phantom-weak so a GC'd promise still drops the entry (reason released with it), and a late handler gets the original reason on the event per PromiseRejectionEvent semantics. The test now asserts e.reason identity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant