Use createError in JSError and forward it in RuntimeDecorator by mrousavy · Pull Request #2203 · meta/hermes

mrousavy · 2026-09-25T23:08:09Z

Summary

JSError(rt, message) still creates its value by looking up globalThis.Error and calling it, which predates IRuntime::createError. It now uses rt.createError(...) like the JSError::createXError factories, with the same try/catch fallback. The default Runtime::createError makes the same global call, so runtimes without a native implementation behave as before. On Hermes every JSError thrown from native code skips a global lookup and a JS call, and message, stack and what() come out the same.

RuntimeDecorator and WithRuntimeDecorator didn't forward the createError family, so decorated runtimes (the thread-safe runtime, TracingRuntime) quietly fell back to the global constructor calls. They forward to plain() now, through Around for WithRuntimeDecorator, and they're public like in jsi::Runtime.

One visible difference on Hermes, same as the other createXError APIs: if user code replaced or deleted globalThis.Error, JSError(rt, message) no longer calls the replacement and creates a real Error anyway.

Test Plan

Two new tests in testlib.cpp. JSErrorMessageTest checks the value is an Error with the right message and a string stack, and that a counting globalThis.Error replacement gets called exactly as often as rt.createError calls it, so it holds for native and default implementations. DecoratorCreateErrorTest checks that all seven createXError calls, and JSError, reach the wrapped runtime once through both decorators, and that Around runs once.

JSErrorDoesNotInfinitelyRecurse and CreateJSErrorPropagatesOOMExceptionTest run through decorators that use the default Runtime::createError. This keeps the missing-constructor fallback and OOM propagation covered, since both tests depend on changing globalThis.Error.

APITests (ASan + Debug, macOS arm64, HERMESVM_EXCEPTION_ON_OOM=ON): 362/362 pass, including SynthTrace and all three OOM tests. Built with -O1 and HERMES_ENABLE_WERROR=ON; the existing GoogleTest uninitialized-const-pointer warning is demoted for AppleClang 21.

cmake --build cmake-build-asan-oom --target APITests -j 8
cmake-build-asan-oom/unittests/API/APITests

Before the OOM test adaptation, CreateJSErrorPropagatesOOMExceptionTest fails because it throws JSError instead of JSOutOfMemoryError. It passes with the decorator. The earlier checks with the jsi.cpp and decorator.h changes reverted also made the new JSI tests fail.

JSError(rt, message) built its value by looking up and calling the global Error function, which predates IRuntime::createError. Use rt.createError instead, as the JSError::createXError factories already do. Runtimes without a native implementation behave as before, since the default Runtime::createError makes the same global call. Runtimes with a native implementation skip the global lookup and JS call. RuntimeDecorator and WithRuntimeDecorator did not override the createError family, so decorated runtimes (e.g. the thread-safe runtime and TracingRuntime) fell back to the default global calls instead of the wrapped runtime's implementation. Forward them. JSErrorDoesNotInfinitelyRecurse now runs through a decorator that keeps the default createError, so it still covers the fallback when creating the Error fails.

lavenzg · 2026-10-07T19:31:17Z

+  // implementation, which calls the global Error constructor, so that removing
+  // it below makes creating the Error fail even if the runtime has a native
+  // createError.
+  class RD : public RuntimeDecorator<Runtime, Runtime> {


We need to do the same for test CreateJSErrorPropagatesOOMExceptionTest.

…eact#58936) Summary: `IRuntime::length(const String&)` came in with "Add `String::length` API" (590396bd4 on static_h, D96030262), and that commit didn't touch `decorator.h`. Neither `RuntimeDecorator` nor `WithRuntimeDecorator` overrides it, so on a decorated runtime `length()` falls back to the default `utf16(str).size()` and never reaches the wrapped runtime. It looks like an oversight. The same stack added `push`, `tryGetMutableBuffer`, `detached` and the TypedArray methods, and each of those commits added decorator overrides, including one for this method's overload `length(const TypedArray&)`. The `createError*` commit (meta/hermes#2203) and this one didn't. For Hermes-backed decorators the value is already right, because Hermes' `utf16()` is exact, but every call copies the whole string instead of reading the length. Runtimes with a native `length()` and the default `utf16()` get wrong answers. React Native's `JSCRuntime` is one: built against the system JavaScriptCore and wrapped in a `RuntimeDecorator`, `'a\uD800b'` has length 1 instead of 3 and `'x\uDC00yz'` has 1 instead of 4. With this change both match the undecorated runtime. That setup lives outside this repo, so it isn't a test here. `WithRuntimeDecorator` needs the override too. Otherwise it would inherit the plain forward and `length()` would skip `Around`, which it currently goes through via `utf16()`. `TracingRuntime` no longer records an incidental `Utf16Record` when `length()` is called. That record only came from the `utf16()` fallback. I didn't add a record for `length()`: strings are immutable, and `TracingRuntime` already leaves pure queries like `size(Array)` untraced. X-link: meta/hermes#2205 Test Plan: Added `JSITest.DecoratorForwardsStringLength`. An inner decorator returns a sentinel from `length()` and counts `length()` and `utf16()` calls. The test checks that a `RuntimeDecorator` and a `WithRuntimeDecorator` over it both reach that `length()` without calling `utf16()`, and that `Around` runs once. It fails without the change (it gets 5 from the `utf16()` fallback) and passes with it. The full `APITests` binary passes (357 tests) in a Debug + ASan build, including the SynthTrace and TraceInterpreter tests. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Reviewed By: avp Differential Revision: D123943189 Pulled By: lavenzg

Run CreateJSErrorPropagatesOOMExceptionTest through a decorator that calls Runtime::createError, so its replacement global Error constructor still triggers OOM after JSError starts using the native createError API. Verified that the test fails before this change and passes afterward with HERMESVM_EXCEPTION_ON_OOM=ON. All 362 API tests pass in ASan+Debug.

…2205) Summary: X-link: react/react-native#58936 `IRuntime::length(const String&)` came in with "Add `String::length` API" (590396b on static_h, D96030262), and that commit didn't touch `decorator.h`. Neither `RuntimeDecorator` nor `WithRuntimeDecorator` overrides it, so on a decorated runtime `length()` falls back to the default `utf16(str).size()` and never reaches the wrapped runtime. It looks like an oversight. The same stack added `push`, `tryGetMutableBuffer`, `detached` and the TypedArray methods, and each of those commits added decorator overrides, including one for this method's overload `length(const TypedArray&)`. The `createError*` commit (#2203) and this one didn't. For Hermes-backed decorators the value is already right, because Hermes' `utf16()` is exact, but every call copies the whole string instead of reading the length. Runtimes with a native `length()` and the default `utf16()` get wrong answers. React Native's `JSCRuntime` is one: built against the system JavaScriptCore and wrapped in a `RuntimeDecorator`, `'a\uD800b'` has length 1 instead of 3 and `'x\uDC00yz'` has 1 instead of 4. With this change both match the undecorated runtime. That setup lives outside this repo, so it isn't a test here. `WithRuntimeDecorator` needs the override too. Otherwise it would inherit the plain forward and `length()` would skip `Around`, which it currently goes through via `utf16()`. `TracingRuntime` no longer records an incidental `Utf16Record` when `length()` is called. That record only came from the `utf16()` fallback. I didn't add a record for `length()`: strings are immutable, and `TracingRuntime` already leaves pure queries like `size(Array)` untraced. Pull Request resolved: #2205 Test Plan: Added `JSITest.DecoratorForwardsStringLength`. An inner decorator returns a sentinel from `length()` and counts `length()` and `utf16()` calls. The test checks that a `RuntimeDecorator` and a `WithRuntimeDecorator` over it both reach that `length()` without calling `utf16()`, and that `Around` runs once. It fails without the change (it gets 5 from the `utf16()` fallback) and passes with it. The full `APITests` binary passes (357 tests) in a Debug + ASan build, including the SynthTrace and TraceInterpreter tests. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Reviewed By: avp Differential Revision: D123943189 Pulled By: lavenzg fbshipit-source-id: 519fdde9f2ad46ce906567bfd1d94d787015915c

…58936) Summary: Pull Request resolved: #58936 `IRuntime::length(const String&)` came in with "Add `String::length` API" (590396bd4 on static_h, D96030262), and that commit didn't touch `decorator.h`. Neither `RuntimeDecorator` nor `WithRuntimeDecorator` overrides it, so on a decorated runtime `length()` falls back to the default `utf16(str).size()` and never reaches the wrapped runtime. It looks like an oversight. The same stack added `push`, `tryGetMutableBuffer`, `detached` and the TypedArray methods, and each of those commits added decorator overrides, including one for this method's overload `length(const TypedArray&)`. The `createError*` commit (meta/hermes#2203) and this one didn't. For Hermes-backed decorators the value is already right, because Hermes' `utf16()` is exact, but every call copies the whole string instead of reading the length. Runtimes with a native `length()` and the default `utf16()` get wrong answers. React Native's `JSCRuntime` is one: built against the system JavaScriptCore and wrapped in a `RuntimeDecorator`, `'a\uD800b'` has length 1 instead of 3 and `'x\uDC00yz'` has 1 instead of 4. With this change both match the undecorated runtime. That setup lives outside this repo, so it isn't a test here. `WithRuntimeDecorator` needs the override too. Otherwise it would inherit the plain forward and `length()` would skip `Around`, which it currently goes through via `utf16()`. `TracingRuntime` no longer records an incidental `Utf16Record` when `length()` is called. That record only came from the `utf16()` fallback. I didn't add a record for `length()`: strings are immutable, and `TracingRuntime` already leaves pure queries like `size(Array)` untraced. X-link: meta/hermes#2205 Test Plan: Added `JSITest.DecoratorForwardsStringLength`. An inner decorator returns a sentinel from `length()` and counts `length()` and `utf16()` calls. The test checks that a `RuntimeDecorator` and a `WithRuntimeDecorator` over it both reach that `length()` without calling `utf16()`, and that `Around` runs once. It fails without the change (it gets 5 from the `utf16()` fallback) and passes with it. The full `APITests` binary passes (357 tests) in a Debug + ASan build, including the SynthTrace and TraceInterpreter tests. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Reviewed By: avp Differential Revision: D123943189 Pulled By: lavenzg fbshipit-source-id: 519fdde9f2ad46ce906567bfd1d94d787015915c

…2205) Summary: X-link: react/react-native#58936 `IRuntime::length(const String&)` came in with "Add `String::length` API" (590396b on static_h, D96030262), and that commit didn't touch `decorator.h`. Neither `RuntimeDecorator` nor `WithRuntimeDecorator` overrides it, so on a decorated runtime `length()` falls back to the default `utf16(str).size()` and never reaches the wrapped runtime. It looks like an oversight. The same stack added `push`, `tryGetMutableBuffer`, `detached` and the TypedArray methods, and each of those commits added decorator overrides, including one for this method's overload `length(const TypedArray&)`. The `createError*` commit (#2203) and this one didn't. For Hermes-backed decorators the value is already right, because Hermes' `utf16()` is exact, but every call copies the whole string instead of reading the length. Runtimes with a native `length()` and the default `utf16()` get wrong answers. React Native's `JSCRuntime` is one: built against the system JavaScriptCore and wrapped in a `RuntimeDecorator`, `'a\uD800b'` has length 1 instead of 3 and `'x\uDC00yz'` has 1 instead of 4. With this change both match the undecorated runtime. That setup lives outside this repo, so it isn't a test here. `WithRuntimeDecorator` needs the override too. Otherwise it would inherit the plain forward and `length()` would skip `Around`, which it currently goes through via `utf16()`. `TracingRuntime` no longer records an incidental `Utf16Record` when `length()` is called. That record only came from the `utf16()` fallback. I didn't add a record for `length()`: strings are immutable, and `TracingRuntime` already leaves pure queries like `size(Array)` untraced. Pull Request resolved: #2205 Test Plan: Added `JSITest.DecoratorForwardsStringLength`. An inner decorator returns a sentinel from `length()` and counts `length()` and `utf16()` calls. The test checks that a `RuntimeDecorator` and a `WithRuntimeDecorator` over it both reach that `length()` without calling `utf16()`, and that `Around` runs once. It fails without the change (it gets 5 from the `utf16()` fallback) and passes with it. The full `APITests` binary passes (357 tests) in a Debug + ASan build, including the SynthTrace and TraceInterpreter tests. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Reviewed By: avp Differential Revision: D123943189 Pulled By: lavenzg fbshipit-source-id: 519fdde9f2ad46ce906567bfd1d94d787015915c

meta-codesync · 2026-10-08T20:33:28Z

@lavenzg has imported this pull request. If you are a Meta employee, you can view this in D124178378.

…58961) Summary: `JSError(rt, message)` still creates its value by looking up `globalThis.Error` and calling it, which predates `IRuntime::createError`. It now uses `rt.createError(...)` like the `JSError::createXError` factories, with the same try/catch fallback. The default `Runtime::createError` makes the same global call, so runtimes without a native implementation behave as before. On Hermes every `JSError` thrown from native code skips a global lookup and a JS call, and `message`, `stack` and `what()` come out the same. `RuntimeDecorator` and `WithRuntimeDecorator` didn't forward the `createError` family, so decorated runtimes (the thread-safe runtime, `TracingRuntime`) quietly fell back to the global constructor calls. They forward to `plain()` now, through `Around` for `WithRuntimeDecorator`, and they're public like in `jsi::Runtime`. One visible difference on Hermes, same as the other `createXError` APIs: if user code replaced or deleted `globalThis.Error`, `JSError(rt, message)` no longer calls the replacement and creates a real `Error` anyway. X-link: meta/hermes#2203 Test Plan: Two new tests in testlib.cpp. `JSErrorMessageTest` checks the value is an `Error` with the right message and a string stack, and that a counting `globalThis.Error` replacement gets called exactly as often as `rt.createError` calls it, so it holds for native and default implementations. `DecoratorCreateErrorTest` checks that all seven `createXError` calls, and `JSError`, reach the wrapped runtime once through both decorators, and that `Around` runs once. `JSErrorDoesNotInfinitelyRecurse` and `CreateJSErrorPropagatesOOMExceptionTest` run through decorators that use the default `Runtime::createError`. This keeps the missing-constructor fallback and OOM propagation covered, since both tests depend on changing `globalThis.Error`. `APITests` (ASan + Debug, macOS arm64, `HERMESVM_EXCEPTION_ON_OOM=ON`): 362/362 pass, including SynthTrace and all three OOM tests. Built with `-O1` and `HERMES_ENABLE_WERROR=ON`; the existing GoogleTest `uninitialized-const-pointer` warning is demoted for AppleClang 21. ```sh cmake --build cmake-build-asan-oom --target APITests -j 8 cmake-build-asan-oom/unittests/API/APITests ``` Before the OOM test adaptation, `CreateJSErrorPropagatesOOMExceptionTest` fails because it throws `JSError` instead of `JSOutOfMemoryError`. It passes with the decorator. The earlier checks with the `jsi.cpp` and `decorator.h` changes reverted also made the new JSI tests fail. Reviewed By: javache Differential Revision: D124178378 Pulled By: lavenzg

…58961) Summary: Pull Request resolved: react#58961 `JSError(rt, message)` still creates its value by looking up `globalThis.Error` and calling it, which predates `IRuntime::createError`. It now uses `rt.createError(...)` like the `JSError::createXError` factories, with the same try/catch fallback. The default `Runtime::createError` makes the same global call, so runtimes without a native implementation behave as before. On Hermes every `JSError` thrown from native code skips a global lookup and a JS call, and `message`, `stack` and `what()` come out the same. `RuntimeDecorator` and `WithRuntimeDecorator` didn't forward the `createError` family, so decorated runtimes (the thread-safe runtime, `TracingRuntime`) quietly fell back to the global constructor calls. They forward to `plain()` now, through `Around` for `WithRuntimeDecorator`, and they're public like in `jsi::Runtime`. One visible difference on Hermes, same as the other `createXError` APIs: if user code replaced or deleted `globalThis.Error`, `JSError(rt, message)` no longer calls the replacement and creates a real `Error` anyway. X-link: meta/hermes#2203 Test Plan: Two new tests in testlib.cpp. `JSErrorMessageTest` checks the value is an `Error` with the right message and a string stack, and that a counting `globalThis.Error` replacement gets called exactly as often as `rt.createError` calls it, so it holds for native and default implementations. `DecoratorCreateErrorTest` checks that all seven `createXError` calls, and `JSError`, reach the wrapped runtime once through both decorators, and that `Around` runs once. `JSErrorDoesNotInfinitelyRecurse` and `CreateJSErrorPropagatesOOMExceptionTest` run through decorators that use the default `Runtime::createError`. This keeps the missing-constructor fallback and OOM propagation covered, since both tests depend on changing `globalThis.Error`. `APITests` (ASan + Debug, macOS arm64, `HERMESVM_EXCEPTION_ON_OOM=ON`): 362/362 pass, including SynthTrace and all three OOM tests. Built with `-O1` and `HERMES_ENABLE_WERROR=ON`; the existing GoogleTest `uninitialized-const-pointer` warning is demoted for AppleClang 21. ```sh cmake --build cmake-build-asan-oom --target APITests -j 8 cmake-build-asan-oom/unittests/API/APITests ``` Before the OOM test adaptation, `CreateJSErrorPropagatesOOMExceptionTest` fails because it throws `JSError` instead of `JSOutOfMemoryError`. It passes with the decorator. The earlier checks with the `jsi.cpp` and `decorator.h` changes reverted also made the new JSI tests fail. Reviewed By: javache Differential Revision: D124178378 Pulled By: lavenzg

Summary: X-link: react/react-native#58961 `JSError(rt, message)` still creates its value by looking up `globalThis.Error` and calling it, which predates `IRuntime::createError`. It now uses `rt.createError(...)` like the `JSError::createXError` factories, with the same try/catch fallback. The default `Runtime::createError` makes the same global call, so runtimes without a native implementation behave as before. On Hermes every `JSError` thrown from native code skips a global lookup and a JS call, and `message`, `stack` and `what()` come out the same. `RuntimeDecorator` and `WithRuntimeDecorator` didn't forward the `createError` family, so decorated runtimes (the thread-safe runtime, `TracingRuntime`) quietly fell back to the global constructor calls. They forward to `plain()` now, through `Around` for `WithRuntimeDecorator`, and they're public like in `jsi::Runtime`. One visible difference on Hermes, same as the other `createXError` APIs: if user code replaced or deleted `globalThis.Error`, `JSError(rt, message)` no longer calls the replacement and creates a real `Error` anyway. Pull Request resolved: #2203 Test Plan: Two new tests in testlib.cpp. `JSErrorMessageTest` checks the value is an `Error` with the right message and a string stack, and that a counting `globalThis.Error` replacement gets called exactly as often as `rt.createError` calls it, so it holds for native and default implementations. `DecoratorCreateErrorTest` checks that all seven `createXError` calls, and `JSError`, reach the wrapped runtime once through both decorators, and that `Around` runs once. `JSErrorDoesNotInfinitelyRecurse` and `CreateJSErrorPropagatesOOMExceptionTest` run through decorators that use the default `Runtime::createError`. This keeps the missing-constructor fallback and OOM propagation covered, since both tests depend on changing `globalThis.Error`. `APITests` (ASan + Debug, macOS arm64, `HERMESVM_EXCEPTION_ON_OOM=ON`): 362/362 pass, including SynthTrace and all three OOM tests. Built with `-O1` and `HERMES_ENABLE_WERROR=ON`; the existing GoogleTest `uninitialized-const-pointer` warning is demoted for AppleClang 21. ```sh cmake --build cmake-build-asan-oom --target APITests -j 8 cmake-build-asan-oom/unittests/API/APITests ``` Before the OOM test adaptation, `CreateJSErrorPropagatesOOMExceptionTest` fails because it throws `JSError` instead of `JSOutOfMemoryError`. It passes with the decorator. The earlier checks with the `jsi.cpp` and `decorator.h` changes reverted also made the new JSI tests fail. Reviewed By: javache Differential Revision: D124178378 Pulled By: lavenzg fbshipit-source-id: 33fa348abc52e21626f68d6509093313ab438d87

meta-codesync · 2026-10-13T18:24:45Z

@lavenzg merged this pull request in 99a7d9b.

Summary: Pull Request resolved: #58961 `JSError(rt, message)` still creates its value by looking up `globalThis.Error` and calling it, which predates `IRuntime::createError`. It now uses `rt.createError(...)` like the `JSError::createXError` factories, with the same try/catch fallback. The default `Runtime::createError` makes the same global call, so runtimes without a native implementation behave as before. On Hermes every `JSError` thrown from native code skips a global lookup and a JS call, and `message`, `stack` and `what()` come out the same. `RuntimeDecorator` and `WithRuntimeDecorator` didn't forward the `createError` family, so decorated runtimes (the thread-safe runtime, `TracingRuntime`) quietly fell back to the global constructor calls. They forward to `plain()` now, through `Around` for `WithRuntimeDecorator`, and they're public like in `jsi::Runtime`. One visible difference on Hermes, same as the other `createXError` APIs: if user code replaced or deleted `globalThis.Error`, `JSError(rt, message)` no longer calls the replacement and creates a real `Error` anyway. X-link: meta/hermes#2203 Test Plan: Two new tests in testlib.cpp. `JSErrorMessageTest` checks the value is an `Error` with the right message and a string stack, and that a counting `globalThis.Error` replacement gets called exactly as often as `rt.createError` calls it, so it holds for native and default implementations. `DecoratorCreateErrorTest` checks that all seven `createXError` calls, and `JSError`, reach the wrapped runtime once through both decorators, and that `Around` runs once. `JSErrorDoesNotInfinitelyRecurse` and `CreateJSErrorPropagatesOOMExceptionTest` run through decorators that use the default `Runtime::createError`. This keeps the missing-constructor fallback and OOM propagation covered, since both tests depend on changing `globalThis.Error`. `APITests` (ASan + Debug, macOS arm64, `HERMESVM_EXCEPTION_ON_OOM=ON`): 362/362 pass, including SynthTrace and all three OOM tests. Built with `-O1` and `HERMES_ENABLE_WERROR=ON`; the existing GoogleTest `uninitialized-const-pointer` warning is demoted for AppleClang 21. ```sh cmake --build cmake-build-asan-oom --target APITests -j 8 cmake-build-asan-oom/unittests/API/APITests ``` Before the OOM test adaptation, `CreateJSErrorPropagatesOOMExceptionTest` fails because it throws `JSError` instead of `JSOutOfMemoryError`. It passes with the decorator. The earlier checks with the `jsi.cpp` and `decorator.h` changes reverted also made the new JSI tests fail. Reviewed By: javache Differential Revision: D124178378 Pulled By: lavenzg fbshipit-source-id: 33fa348abc52e21626f68d6509093313ab438d87

mrousavy · 2026-10-13T19:38:59Z

Awesome thanks for merging!

Summary: X-link: react/react-native#58960 `RuntimeDecorator` doesn't forward `createValueFromJsonUtf8` to the plain runtime, so decorated runtimes use the `JSON.parse`-based default from jsi.cpp instead of the plain runtime's parser. For Hermes that means `makeThreadSafeHermesRuntime` and anything built on `WithRuntimeDecorator` never reach the native parser from a3f615c. I think this was missed rather than chosen. `WithRuntimeDecorator` already overrides it (added in c2a0da2 as `Around` plus `RD::createValueFromJsonUtf8`), which only makes sense if `RuntimeDecorator` forwards it. Today that `RD::` call lands in the default, so it takes one `Around` plus several nested ones for `global`, `getProperty`, `createStringFromUtf8` and `call`. It's also observable: on a thread-safe runtime, after `JSON.parse = () => 'hijacked'`, `Value::createFromJsonUtf8` returns `'hijacked'`, and after `delete globalThis.JSON` it throws. A plain `HermesRuntime` parses normally in both cases. `TracingRuntime` relies on the current behavior. JSON parsing only shows up in traces because the default splits into traced `global`/`getProperty`/`call` calls. Forwarding alone would hand back a value from the plain runtime with no def, and replay would hit the "has no def" assert in `useObjectID`. So `TracingRuntime` now overrides `createValueFromJsonUtf8` and calls `jsi::Runtime::createValueFromJsonUtf8` explicitly. That runs the same code as before, so traces are unchanged and there's no new record type. If you'd prefer a dedicated SynthTrace record I can do that instead. A side effect: decorators that watched JSON parsing through `global`/`getProperty`/`call` now need to override `createValueFromJsonUtf8`, like every other forwarded method. For example, `TimedRuntime` no longer counts JSON parsing under its `call` timer. The other methods decorators don't forward yet are the `createError*` family (#2203) and `String::length` (#2205). React Native's `decorator.h` has the same gap. Pull Request resolved: #2206 Test Plan: New tests: - `JSITest.CreateValueFromJsonUtf8Test` checks that parsing through a `RuntimeDecorator` reaches the wrapped runtime, and keeps coverage of the jsi.cpp default through a decorator that overrides back to it. It calls `outer.createValueFromJsonUtf8` directly to check that the override remains public. - `HermesRuntimeTest.CreateValueFromJsonUtf8IgnoresGlobalJSON` replaces `JSON.parse` and checks that parsing still works, on the plain and thread-safe runtimes. - `SynthTraceReplayTest.CreateValueFromJsonUtf8Replay` parses on the tracing runtime, uses the result from native code, and replays with verification. Without the change the first two fail (the second only on the thread-safe runtime). With only the `decorator.h` part, the SynthTrace test aborts in `useObjectID`. With the full change all pass, and so does the full `APITests` binary (360 tests) in a Debug + ASan build. `APILeanTests` also passes. The direct call fails to compile with the previous header because `createValueFromJsonUtf8` is protected. With this revision, `cmake --build cmake-build-asan --target check-hermes -j 12` passes in Debug + ASan: 4,126 expected passes, 6 expected failures, and 197 unsupported tests. Node-API checks: 41 passes and 23 skips. Reviewed By: javache Differential Revision: D124177263 Pulled By: lavenzg fbshipit-source-id: 973dc8b3c69002a81e634bb6c992f5f167e13bf2

Summary: Pull Request resolved: #58960 `RuntimeDecorator` doesn't forward `createValueFromJsonUtf8` to the plain runtime, so decorated runtimes use the `JSON.parse`-based default from jsi.cpp instead of the plain runtime's parser. For Hermes that means `makeThreadSafeHermesRuntime` and anything built on `WithRuntimeDecorator` never reach the native parser from a3f615cec. I think this was missed rather than chosen. `WithRuntimeDecorator` already overrides it (added in c2a0da2ae as `Around` plus `RD::createValueFromJsonUtf8`), which only makes sense if `RuntimeDecorator` forwards it. Today that `RD::` call lands in the default, so it takes one `Around` plus several nested ones for `global`, `getProperty`, `createStringFromUtf8` and `call`. It's also observable: on a thread-safe runtime, after `JSON.parse = () => 'hijacked'`, `Value::createFromJsonUtf8` returns `'hijacked'`, and after `delete globalThis.JSON` it throws. A plain `HermesRuntime` parses normally in both cases. `TracingRuntime` relies on the current behavior. JSON parsing only shows up in traces because the default splits into traced `global`/`getProperty`/`call` calls. Forwarding alone would hand back a value from the plain runtime with no def, and replay would hit the "has no def" assert in `useObjectID`. So `TracingRuntime` now overrides `createValueFromJsonUtf8` and calls `jsi::Runtime::createValueFromJsonUtf8` explicitly. That runs the same code as before, so traces are unchanged and there's no new record type. If you'd prefer a dedicated SynthTrace record I can do that instead. A side effect: decorators that watched JSON parsing through `global`/`getProperty`/`call` now need to override `createValueFromJsonUtf8`, like every other forwarded method. For example, `TimedRuntime` no longer counts JSON parsing under its `call` timer. The other methods decorators don't forward yet are the `createError*` family (meta/hermes#2203) and `String::length` (meta/hermes#2205). React Native's `decorator.h` has the same gap. X-link: meta/hermes#2206 Test Plan: New tests: - `JSITest.CreateValueFromJsonUtf8Test` checks that parsing through a `RuntimeDecorator` reaches the wrapped runtime, and keeps coverage of the jsi.cpp default through a decorator that overrides back to it. It calls `outer.createValueFromJsonUtf8` directly to check that the override remains public. - `HermesRuntimeTest.CreateValueFromJsonUtf8IgnoresGlobalJSON` replaces `JSON.parse` and checks that parsing still works, on the plain and thread-safe runtimes. - `SynthTraceReplayTest.CreateValueFromJsonUtf8Replay` parses on the tracing runtime, uses the result from native code, and replays with verification. Without the change the first two fail (the second only on the thread-safe runtime). With only the `decorator.h` part, the SynthTrace test aborts in `useObjectID`. With the full change all pass, and so does the full `APITests` binary (360 tests) in a Debug + ASan build. `APILeanTests` also passes. The direct call fails to compile with the previous header because `createValueFromJsonUtf8` is protected. With this revision, `cmake --build cmake-build-asan --target check-hermes -j 12` passes in Debug + ASan: 4,126 expected passes, 6 expected failures, and 197 unsupported tests. Node-API checks: 41 passes and 23 skips. Reviewed By: javache Differential Revision: D124177263 Pulled By: lavenzg fbshipit-source-id: 973dc8b3c69002a81e634bb6c992f5f167e13bf2

…58936) Summary: Pull Request resolved: #58936 `IRuntime::length(const String&)` came in with "Add `String::length` API" (590396bd4 on static_h, D96030262), and that commit didn't touch `decorator.h`. Neither `RuntimeDecorator` nor `WithRuntimeDecorator` overrides it, so on a decorated runtime `length()` falls back to the default `utf16(str).size()` and never reaches the wrapped runtime. It looks like an oversight. The same stack added `push`, `tryGetMutableBuffer`, `detached` and the TypedArray methods, and each of those commits added decorator overrides, including one for this method's overload `length(const TypedArray&)`. The `createError*` commit (meta/hermes#2203) and this one didn't. For Hermes-backed decorators the value is already right, because Hermes' `utf16()` is exact, but every call copies the whole string instead of reading the length. Runtimes with a native `length()` and the default `utf16()` get wrong answers. React Native's `JSCRuntime` is one: built against the system JavaScriptCore and wrapped in a `RuntimeDecorator`, `'a\uD800b'` has length 1 instead of 3 and `'x\uDC00yz'` has 1 instead of 4. With this change both match the undecorated runtime. That setup lives outside this repo, so it isn't a test here. `WithRuntimeDecorator` needs the override too. Otherwise it would inherit the plain forward and `length()` would skip `Around`, which it currently goes through via `utf16()`. `TracingRuntime` no longer records an incidental `Utf16Record` when `length()` is called. That record only came from the `utf16()` fallback. I didn't add a record for `length()`: strings are immutable, and `TracingRuntime` already leaves pure queries like `size(Array)` untraced. X-link: meta/hermes#2205 Test Plan: Added `JSITest.DecoratorForwardsStringLength`. An inner decorator returns a sentinel from `length()` and counts `length()` and `utf16()` calls. The test checks that a `RuntimeDecorator` and a `WithRuntimeDecorator` over it both reach that `length()` without calling `utf16()`, and that `Around` runs once. It fails without the change (it gets 5 from the `utf16()` fallback) and passes with it. The full `APITests` binary passes (357 tests) in a Debug + ASan build, including the SynthTrace and TraceInterpreter tests. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Reviewed By: avp Differential Revision: D123943189 Pulled By: lavenzg fbshipit-source-id: 519fdde9f2ad46ce906567bfd1d94d787015915c (cherry picked from commit be567d2)

Summary: Pull Request resolved: #58961 `JSError(rt, message)` still creates its value by looking up `globalThis.Error` and calling it, which predates `IRuntime::createError`. It now uses `rt.createError(...)` like the `JSError::createXError` factories, with the same try/catch fallback. The default `Runtime::createError` makes the same global call, so runtimes without a native implementation behave as before. On Hermes every `JSError` thrown from native code skips a global lookup and a JS call, and `message`, `stack` and `what()` come out the same. `RuntimeDecorator` and `WithRuntimeDecorator` didn't forward the `createError` family, so decorated runtimes (the thread-safe runtime, `TracingRuntime`) quietly fell back to the global constructor calls. They forward to `plain()` now, through `Around` for `WithRuntimeDecorator`, and they're public like in `jsi::Runtime`. One visible difference on Hermes, same as the other `createXError` APIs: if user code replaced or deleted `globalThis.Error`, `JSError(rt, message)` no longer calls the replacement and creates a real `Error` anyway. X-link: meta/hermes#2203 Test Plan: Two new tests in testlib.cpp. `JSErrorMessageTest` checks the value is an `Error` with the right message and a string stack, and that a counting `globalThis.Error` replacement gets called exactly as often as `rt.createError` calls it, so it holds for native and default implementations. `DecoratorCreateErrorTest` checks that all seven `createXError` calls, and `JSError`, reach the wrapped runtime once through both decorators, and that `Around` runs once. `JSErrorDoesNotInfinitelyRecurse` and `CreateJSErrorPropagatesOOMExceptionTest` run through decorators that use the default `Runtime::createError`. This keeps the missing-constructor fallback and OOM propagation covered, since both tests depend on changing `globalThis.Error`. `APITests` (ASan + Debug, macOS arm64, `HERMESVM_EXCEPTION_ON_OOM=ON`): 362/362 pass, including SynthTrace and all three OOM tests. Built with `-O1` and `HERMES_ENABLE_WERROR=ON`; the existing GoogleTest `uninitialized-const-pointer` warning is demoted for AppleClang 21. ```sh cmake --build cmake-build-asan-oom --target APITests -j 8 cmake-build-asan-oom/unittests/API/APITests ``` Before the OOM test adaptation, `CreateJSErrorPropagatesOOMExceptionTest` fails because it throws `JSError` instead of `JSOutOfMemoryError`. It passes with the decorator. The earlier checks with the `jsi.cpp` and `decorator.h` changes reverted also made the new JSI tests fail. Reviewed By: javache Differential Revision: D124178378 Pulled By: lavenzg fbshipit-source-id: 33fa348abc52e21626f68d6509093313ab438d87 (cherry picked from commit 0cca3a1)

Summary: Pull Request resolved: #58960 `RuntimeDecorator` doesn't forward `createValueFromJsonUtf8` to the plain runtime, so decorated runtimes use the `JSON.parse`-based default from jsi.cpp instead of the plain runtime's parser. For Hermes that means `makeThreadSafeHermesRuntime` and anything built on `WithRuntimeDecorator` never reach the native parser from a3f615cec. I think this was missed rather than chosen. `WithRuntimeDecorator` already overrides it (added in c2a0da2ae as `Around` plus `RD::createValueFromJsonUtf8`), which only makes sense if `RuntimeDecorator` forwards it. Today that `RD::` call lands in the default, so it takes one `Around` plus several nested ones for `global`, `getProperty`, `createStringFromUtf8` and `call`. It's also observable: on a thread-safe runtime, after `JSON.parse = () => 'hijacked'`, `Value::createFromJsonUtf8` returns `'hijacked'`, and after `delete globalThis.JSON` it throws. A plain `HermesRuntime` parses normally in both cases. `TracingRuntime` relies on the current behavior. JSON parsing only shows up in traces because the default splits into traced `global`/`getProperty`/`call` calls. Forwarding alone would hand back a value from the plain runtime with no def, and replay would hit the "has no def" assert in `useObjectID`. So `TracingRuntime` now overrides `createValueFromJsonUtf8` and calls `jsi::Runtime::createValueFromJsonUtf8` explicitly. That runs the same code as before, so traces are unchanged and there's no new record type. If you'd prefer a dedicated SynthTrace record I can do that instead. A side effect: decorators that watched JSON parsing through `global`/`getProperty`/`call` now need to override `createValueFromJsonUtf8`, like every other forwarded method. For example, `TimedRuntime` no longer counts JSON parsing under its `call` timer. The other methods decorators don't forward yet are the `createError*` family (meta/hermes#2203) and `String::length` (meta/hermes#2205). React Native's `decorator.h` has the same gap. X-link: meta/hermes#2206 Test Plan: New tests: - `JSITest.CreateValueFromJsonUtf8Test` checks that parsing through a `RuntimeDecorator` reaches the wrapped runtime, and keeps coverage of the jsi.cpp default through a decorator that overrides back to it. It calls `outer.createValueFromJsonUtf8` directly to check that the override remains public. - `HermesRuntimeTest.CreateValueFromJsonUtf8IgnoresGlobalJSON` replaces `JSON.parse` and checks that parsing still works, on the plain and thread-safe runtimes. - `SynthTraceReplayTest.CreateValueFromJsonUtf8Replay` parses on the tracing runtime, uses the result from native code, and replays with verification. Without the change the first two fail (the second only on the thread-safe runtime). With only the `decorator.h` part, the SynthTrace test aborts in `useObjectID`. With the full change all pass, and so does the full `APITests` binary (360 tests) in a Debug + ASan build. `APILeanTests` also passes. The direct call fails to compile with the previous header because `createValueFromJsonUtf8` is protected. With this revision, `cmake --build cmake-build-asan --target check-hermes -j 12` passes in Debug + ASan: 4,126 expected passes, 6 expected failures, and 197 unsupported tests. Node-API checks: 41 passes and 23 skips. Reviewed By: javache Differential Revision: D124177263 Pulled By: lavenzg fbshipit-source-id: 973dc8b3c69002a81e634bb6c992f5f167e13bf2 (cherry picked from commit b5eec4e)

…eact#58936) Summary: Pull Request resolved: react#58936 `IRuntime::length(const String&)` came in with "Add `String::length` API" (590396bd4 on static_h, D96030262), and that commit didn't touch `decorator.h`. Neither `RuntimeDecorator` nor `WithRuntimeDecorator` overrides it, so on a decorated runtime `length()` falls back to the default `utf16(str).size()` and never reaches the wrapped runtime. It looks like an oversight. The same stack added `push`, `tryGetMutableBuffer`, `detached` and the TypedArray methods, and each of those commits added decorator overrides, including one for this method's overload `length(const TypedArray&)`. The `createError*` commit (meta/hermes#2203) and this one didn't. For Hermes-backed decorators the value is already right, because Hermes' `utf16()` is exact, but every call copies the whole string instead of reading the length. Runtimes with a native `length()` and the default `utf16()` get wrong answers. React Native's `JSCRuntime` is one: built against the system JavaScriptCore and wrapped in a `RuntimeDecorator`, `'a\uD800b'` has length 1 instead of 3 and `'x\uDC00yz'` has 1 instead of 4. With this change both match the undecorated runtime. That setup lives outside this repo, so it isn't a test here. `WithRuntimeDecorator` needs the override too. Otherwise it would inherit the plain forward and `length()` would skip `Around`, which it currently goes through via `utf16()`. `TracingRuntime` no longer records an incidental `Utf16Record` when `length()` is called. That record only came from the `utf16()` fallback. I didn't add a record for `length()`: strings are immutable, and `TracingRuntime` already leaves pure queries like `size(Array)` untraced. X-link: meta/hermes#2205 Test Plan: Added `JSITest.DecoratorForwardsStringLength`. An inner decorator returns a sentinel from `length()` and counts `length()` and `utf16()` calls. The test checks that a `RuntimeDecorator` and a `WithRuntimeDecorator` over it both reach that `length()` without calling `utf16()`, and that `Around` runs once. It fails without the change (it gets 5 from the `utf16()` fallback) and passes with it. The full `APITests` binary passes (357 tests) in a Debug + ASan build, including the SynthTrace and TraceInterpreter tests. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Reviewed By: avp Differential Revision: D123943189 Pulled By: lavenzg fbshipit-source-id: 519fdde9f2ad46ce906567bfd1d94d787015915c (cherry picked from commit be567d2)

…58961) Summary: Pull Request resolved: react#58961 `JSError(rt, message)` still creates its value by looking up `globalThis.Error` and calling it, which predates `IRuntime::createError`. It now uses `rt.createError(...)` like the `JSError::createXError` factories, with the same try/catch fallback. The default `Runtime::createError` makes the same global call, so runtimes without a native implementation behave as before. On Hermes every `JSError` thrown from native code skips a global lookup and a JS call, and `message`, `stack` and `what()` come out the same. `RuntimeDecorator` and `WithRuntimeDecorator` didn't forward the `createError` family, so decorated runtimes (the thread-safe runtime, `TracingRuntime`) quietly fell back to the global constructor calls. They forward to `plain()` now, through `Around` for `WithRuntimeDecorator`, and they're public like in `jsi::Runtime`. One visible difference on Hermes, same as the other `createXError` APIs: if user code replaced or deleted `globalThis.Error`, `JSError(rt, message)` no longer calls the replacement and creates a real `Error` anyway. X-link: meta/hermes#2203 Test Plan: Two new tests in testlib.cpp. `JSErrorMessageTest` checks the value is an `Error` with the right message and a string stack, and that a counting `globalThis.Error` replacement gets called exactly as often as `rt.createError` calls it, so it holds for native and default implementations. `DecoratorCreateErrorTest` checks that all seven `createXError` calls, and `JSError`, reach the wrapped runtime once through both decorators, and that `Around` runs once. `JSErrorDoesNotInfinitelyRecurse` and `CreateJSErrorPropagatesOOMExceptionTest` run through decorators that use the default `Runtime::createError`. This keeps the missing-constructor fallback and OOM propagation covered, since both tests depend on changing `globalThis.Error`. `APITests` (ASan + Debug, macOS arm64, `HERMESVM_EXCEPTION_ON_OOM=ON`): 362/362 pass, including SynthTrace and all three OOM tests. Built with `-O1` and `HERMES_ENABLE_WERROR=ON`; the existing GoogleTest `uninitialized-const-pointer` warning is demoted for AppleClang 21. ```sh cmake --build cmake-build-asan-oom --target APITests -j 8 cmake-build-asan-oom/unittests/API/APITests ``` Before the OOM test adaptation, `CreateJSErrorPropagatesOOMExceptionTest` fails because it throws `JSError` instead of `JSOutOfMemoryError`. It passes with the decorator. The earlier checks with the `jsi.cpp` and `decorator.h` changes reverted also made the new JSI tests fail. Reviewed By: javache Differential Revision: D124178378 Pulled By: lavenzg fbshipit-source-id: 33fa348abc52e21626f68d6509093313ab438d87 (cherry picked from commit 0cca3a1)

Summary: Pull Request resolved: react#58960 `RuntimeDecorator` doesn't forward `createValueFromJsonUtf8` to the plain runtime, so decorated runtimes use the `JSON.parse`-based default from jsi.cpp instead of the plain runtime's parser. For Hermes that means `makeThreadSafeHermesRuntime` and anything built on `WithRuntimeDecorator` never reach the native parser from a3f615cec. I think this was missed rather than chosen. `WithRuntimeDecorator` already overrides it (added in c2a0da2ae as `Around` plus `RD::createValueFromJsonUtf8`), which only makes sense if `RuntimeDecorator` forwards it. Today that `RD::` call lands in the default, so it takes one `Around` plus several nested ones for `global`, `getProperty`, `createStringFromUtf8` and `call`. It's also observable: on a thread-safe runtime, after `JSON.parse = () => 'hijacked'`, `Value::createFromJsonUtf8` returns `'hijacked'`, and after `delete globalThis.JSON` it throws. A plain `HermesRuntime` parses normally in both cases. `TracingRuntime` relies on the current behavior. JSON parsing only shows up in traces because the default splits into traced `global`/`getProperty`/`call` calls. Forwarding alone would hand back a value from the plain runtime with no def, and replay would hit the "has no def" assert in `useObjectID`. So `TracingRuntime` now overrides `createValueFromJsonUtf8` and calls `jsi::Runtime::createValueFromJsonUtf8` explicitly. That runs the same code as before, so traces are unchanged and there's no new record type. If you'd prefer a dedicated SynthTrace record I can do that instead. A side effect: decorators that watched JSON parsing through `global`/`getProperty`/`call` now need to override `createValueFromJsonUtf8`, like every other forwarded method. For example, `TimedRuntime` no longer counts JSON parsing under its `call` timer. The other methods decorators don't forward yet are the `createError*` family (meta/hermes#2203) and `String::length` (meta/hermes#2205). React Native's `decorator.h` has the same gap. X-link: meta/hermes#2206 Test Plan: New tests: - `JSITest.CreateValueFromJsonUtf8Test` checks that parsing through a `RuntimeDecorator` reaches the wrapped runtime, and keeps coverage of the jsi.cpp default through a decorator that overrides back to it. It calls `outer.createValueFromJsonUtf8` directly to check that the override remains public. - `HermesRuntimeTest.CreateValueFromJsonUtf8IgnoresGlobalJSON` replaces `JSON.parse` and checks that parsing still works, on the plain and thread-safe runtimes. - `SynthTraceReplayTest.CreateValueFromJsonUtf8Replay` parses on the tracing runtime, uses the result from native code, and replays with verification. Without the change the first two fail (the second only on the thread-safe runtime). With only the `decorator.h` part, the SynthTrace test aborts in `useObjectID`. With the full change all pass, and so does the full `APITests` binary (360 tests) in a Debug + ASan build. `APILeanTests` also passes. The direct call fails to compile with the previous header because `createValueFromJsonUtf8` is protected. With this revision, `cmake --build cmake-build-asan --target check-hermes -j 12` passes in Debug + ASan: 4,126 expected passes, 6 expected failures, and 197 unsupported tests. Node-API checks: 41 passes and 23 skips. Reviewed By: javache Differential Revision: D124177263 Pulled By: lavenzg fbshipit-source-id: 973dc8b3c69002a81e634bb6c992f5f167e13bf2 (cherry picked from commit b5eec4e)

meta-cla Bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Sep 25, 2026

mrousavy temporarily deployed to npm-publish September 25, 2026 23:32 — with GitHub Actions Inactive

This was referenced Sep 25, 2026

Forward String::length in RuntimeDecorator and WithRuntimeDecorator #2205

Closed

Forward createValueFromJsonUtf8 in RuntimeDecorator #2206

Closed

mrousavy force-pushed the jsi-jserror-create-error branch from e050793 to f3a9eab Compare September 26, 2026 13:16

mrousavy temporarily deployed to npm-publish September 26, 2026 13:41 — with GitHub Actions Inactive

lavenzg requested changes Oct 7, 2026

View reviewed changes

lavenzg mentioned this pull request Oct 7, 2026

Forward String::length in RuntimeDecorator and WithRuntimeDecorator (#58936) react/react-native#58936

Closed

mrousavy temporarily deployed to npm-publish October 7, 2026 22:15 — with GitHub Actions Inactive

This was referenced Oct 8, 2026

Forward createValueFromJsonUtf8 in RuntimeDecorator react/react-native#58960

Closed

Use createError in JSError and forward it in RuntimeDecorator (#58961) react/react-native#58961

Closed

meta-codesync Bot closed this in 99a7d9b Oct 13, 2026

meta-codesync Bot added the Merged label Oct 13, 2026

Navigation Menu

Repository navigation

Use createError in JSError and forward it in RuntimeDecorator - #2203

Use createError in JSError and forward it in RuntimeDecorator#2203
mrousavy wants to merge 2 commits into
meta:static_hfrom
mrousavy:jsi-jserror-create-error

mrousavy commented Sep 25, 2026 •

edited

Loading

Uh oh!

lavenzg Oct 7, 2026

Uh oh!

meta-codesync Bot commented Oct 8, 2026

Uh oh!

meta-codesync Bot commented Oct 13, 2026

Uh oh!

mrousavy commented Oct 13, 2026

Uh oh!

This branch was previously deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

mrousavy commented Sep 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test Plan

Uh oh!

lavenzg Oct 7, 2026

Choose a reason for hiding this comment

Uh oh!

meta-codesync Bot commented Oct 8, 2026

Uh oh!

meta-codesync Bot commented Oct 13, 2026

Uh oh!

mrousavy commented Oct 13, 2026

Uh oh!

This branch was previously deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mrousavy commented Sep 25, 2026 •

edited

Loading