Replace Error.isError with instanceof Error in createErrorDetails

[mjuarros]

Description

createErrorDetails narrowed unknown values with Error.isError, which is Baseline limited availability and is not polyfilled here. On a browser without the method, the call threw TypeError: Error.isError is not a function from inside the function whose job is to describe an error, so the error panel failed instead of rendering the message.

Both call sites now use instanceof Error, which is sufficient here:

• The four earlier branches (ServerConnectionError, NetworkError, ZodError, QueryValueError) all match Error subclasses and are evaluated first, so their output is unchanged.
• DOMException subclasses Error, so the IndexedDB failures that reach the persistence and migration paths still narrow correctly.
• The only behavioural difference is cross-realm errors, which do not route through this function.

No polyfill was added, which matters because #2180 removes core-js entirely.

How to read

1. createErrorDetails.ts — the core change: two guards, one in createErrorDetails and one in serializeCause
2. createErrorDetails.test.ts — three tests covering the absent-method path, plus the withoutErrorIsError helper. It deletes the property reflectively rather than referencing Error.isError as a typed property, so the test still compiles once Stop using Iterator helpers so core-js can be removed #2180 narrows lib to ES2024
3. docs/agents/testing.md — supporting: records that absent-global test pattern as a convention

Validation

• The three new tests were confirmed failing before the fix, with exactly TypeError: Error.isError is not a function at createErrorDetails.ts:52, and pass after it.
• pnpm checks passes. pnpm test passes at 2675 tests across 220 files. The branch is rebased onto current main, so this includes the Vitest 5 upgrade from Update Vitest to version 5 #2166.
• Verified the #2180 unblock rather than assuming it: setting lib to ["ES2024", "DOM", "DOM.Iterable"] and running pnpm check:types reports no remaining errors attributable to this call site. That tsconfig.json edit was reverted and is not part of this PR.
• No user-visible output changes on browsers that do have the method: only the guard conditions changed, never the branch bodies.

Related Issues

• Fixes Error.isError is not available in all supported browsers #2181
• Related to Stop using Iterator helpers so core-js can be removed #2180, which this unblocks
• Related to core-js is a devDependency but is used in production code #2130 and Move babel-plugin-react-compiler to devDependencies #2157, the same class of using a runtime feature newer than the browser floor we build for

Check List

• I confirm that my contribution is made under the terms of the Apache 2.0 license.
• I have verified pnpm checks passes with no errors.
• I have verified pnpm test passes with no failures.
• I have covered new added functionality with unit tests if necessary.
• I have updated documentation if necessary.

[mjuarros]

Nothing here enforces the floor, and nothing in this PR can. I've filed #2237 with the investigation.
Two things worth knowing up front: oxlint 1.82 can load ESLint plugins via jsPlugins, so this is reachable without adding ESLint, and eslint-plugin-baseline-js pinned to available: 2022 flags Error.isError and Web APIs. Running the candidate stack over src also turned up two violations already shipping on main Array.prototype.toSorted in displayTypeConfigs.ts:109 and displayAttribute.ts:16, which Firefox 114 lacks. So the class is live right now, not just historical.
I've also commented on #2180, because its planned lib: ES2024 permits Promise.withResolvers and Object.groupBy, both Safari 17.4; ES2023 closes those for 5 more type errors.

On the tests: they are a regression guard for this function, not a guard for the class. Reverting line 52 back to Error.isError fails all three of them, so they hold that one function in place. Each covers something distinct: serializeCause is only reached
when a cause is present, so the cause-chain case is the only one exercising the second call site at line 66, and the DOMException case is the issue's reproduction.
I did try replacing the reflective deletion with a vi.spyOn mock, which would have been shorter. Two things ruled it out. @vitest/expect calls Error.isError internally, so a test-scoped spy is still installed while assertions run — mocking it to throw fails from inside toStrictEqual, not from the code under test. And vi.spyOn(Error, "isError") stops compiling under the narrowed lib that #2180 introduces, which would mean shipping a cast into the branch whose purpose is to unblock that narrowing. The current helper avoids both by scoping the deletion to the single call and keeping the property name out of the type system.

[kmcginnes]

Verified this works. I reproduced the crash on the pre-fix commit in Chrome with Error.isError deleted, and it's gone here. With the method present, output is identical on both commits.

Approving. Three non-blocking notes below.

You convinced me on the test block

I was going to ask you to drop describe("browsers without Error.isError") and the helper, on the grounds that nothing in createErrorDetails reads Error.isError anymore so the deletion is inert. Your reasoning is better than mine. Two points I'd under-weighted: within that block the cause case is the only one reaching the second call site at line 66, since serializeCause needs a cause present. And you checked the shorter vi.spyOn version and found concrete reasons it fails, including that @vitest/expect calls Error.isError internally so the mock breaks toStrictEqual from the inside. Documenting the alternative you rejected is what settled it. Keeping them.

Two small things in that block, neither blocking:

• includes the nested cause chain as data builds a single-level cause, so it never reaches the recursion at createErrorDetails.ts:75. It also nearly duplicates the name of :67, which does build a chain. Rename it, or add a mid level.
• withoutErrorIsError ignores the Reflect.deleteProperty result. On a runtime without Error.isError the delete is a no-op and all three tests pass having exercised nothing. expect(descriptor).toBeDefined() before the delete covers that.

Worth folding into the description

The impact is worse than Error.isError is not available in all supported browsers #2181 describes, and its repro steps don't actually work:

• An unreachable endpoint throws TypeError, which the ServerConnectionError branch catches at line 17, before the guard. Reaching it needs a DOMException, for example a listener that accepts and never responds against a fetch timeout.
• On the old code the whole route was lost to the app error boundary, not just the error panel. And on the write queue path a QuotaExceededError escaped as Uncaught (in promise), so the "Changes not saved" indicator never appeared at all. A user with full storage got no warning that nothing was being saved.

Wording

"No user-visible output changes on browsers that do have the method" isn't quite absolute. Object.create(Error.prototype) gives Unknown Error / {} before and Error / "" after, since Error.isError tests the internal slot while instanceof only walks the prototype chain. Nothing in the app produces such a value, so no code change needed, but worth narrowing since this becomes the squash commit message.

Before merge

Needs a rebase. The branch is 10 commits behind main, and the red install-and-test check is the useSchemaSync flake fixed by Stop flaky assertions in proxy server and schema sync tests #2220, which landed after this branch's base. A re-run at the current head would still use the pre-fix base.

Thanks for Enforce the baseline-widely-available browser floor #2237. That's the right home for the enforcement question, and it's out of scope here.

[mjuarros]

Fair on both counts, and the second one turned out to be bigger than it looked.

On enforcement: you're right that nothing here enforces the floor, and nothing in this PR can. I've filed #2237 with the investigation. Two things worth knowing up front: oxlint 1.82 can load ESLint plugins via , so this is reachable without adding ESLint, and pinned to flags and Web APIs. Running the candidate stack over also turned up two violations already shipping on — in and , which Firefox 114 lacks. So the class is live right now, not just historical. I've also commented on #2180, because its planned permits and , both Safari 17.4; closes those for 5 more type errors.

On the tests: they are a regression guard for this function, not a guard for the class. Reverting line 52 back to fails all three of them, so they hold that one function in place. Each covers something distinct: is only reached when a cause is present, so the cause-chain case is the only one exercising the second call site at line 66, and the case is the issue's reproduction.

I did try replacing the reflective deletion with a mock, which would have been shorter. Two things ruled it out. calls internally, so a test-scoped spy is still installed while assertions run — mocking it to throw fails from inside , not from the code under test. And stops compiling under the narrowed that #2180 introduces, which would mean shipping a cast into the branch whose purpose is to unblock that narrowing. The current helper avoids both by scoping the deletion to the single call and keeping the property name out of the type system.

Happy to trim to fewer cases if you still think three is too many, but I'd keep at least the cause-chain one since it's the only coverage of the second guard.