[breken-ai]

What does this PR do?

The builtin node-fetch shim's error classes do not match node-fetch's:

• new FetchError(...) has name "Error", so String(err) is "Error: ..." and the toStringTag is [object Error].
• errno and erroredSysCall are never set.
• An own code: undefined is always defined, even with no system error.
• new AbortError(...) has no type.

node-fetch names each error after its class (errors/base.js). It copies code, errno and erroredSysCall from the system error only when one is given (errors/fetch-error.js). Its AbortError is a FetchBaseError with type "aborted" (errors/abort-error.js). Libraries and tests that build these errors and check err.name === "FetchError", err.errno or err.type === "aborted" behaved differently under Bun.

Changes to src/js/thirdparty/node-fetch.ts:

• FetchBaseError gets name and Symbol.toStringTag getters that return this.constructor.name. Release builtins are bundled with --keep-names, so this gives FetchError and AbortError.
• FetchError sets code/errno/erroredSysCall only when systemError is given. The fields are declared, so no own undefined properties are created.
• AbortError now extends FetchBaseError, not DOMException, with type = "aborted" by default, as node-fetch does. Aborted fetch() calls still reject with the native DOMException AbortError. This class is only the exported constructor.

import { FetchError, AbortError } from "node-fetch";
const e = new FetchError("request to http://example.test/ failed", "system", { code: "ECONNRESET", syscall: "read" });
console.log(String(e), Object.prototype.toString.call(e), e.code, e.errno, e.erroredSysCall);
console.log("code" in new FetchError("invalid json", "invalid-json"), new AbortError("aborted").type);
// node-fetch 3.3.2 on Node 22:
//   FetchError: request to http://example.test/ failed [object FetchError] ECONNRESET ECONNRESET read
//   false aborted
// Bun 1.4.2:
//   Error: request to http://example.test/ failed [object Error] ECONNRESET undefined undefined
//   true undefined

How did you verify your code works?

New test in test/js/node/http/node-fetch.test.js: "node-fetch FetchError and AbortError match node-fetch's error classes". It asserts the name, string form, toStringTag, type, code, errno and erroredSysCall. It also checks that code is absent without a system error, and the AbortError name and type.

Red on Bun 1.4.2:

Expected: "FetchError"
Received: "Error"
(fail) node-fetch FetchError and AbortError match node-fetch's error classes

I did not build Bun locally (no bun bd on this machine), so the green half was checked by loading the patched module into Bun 1.4.2:

• Red on the release build. USE_SYSTEM_BUN=1 bun test test/js/node/http/node-fetch.test.js -t "<new test>" with Bun 1.4.2 (744846f), whose builtin node-fetch is the same as main at 29d9638 for these lines: fails with the output below.
• Green on the patched source. A small script turns src/js/thirdparty/node-fetch.ts into a user module and changes only the builtin-only syntax ($cpp(...) → the global Response/Request/Blob/Headers/FormData/File, .$call → .call, $isJSArray → Array.isArray, new $Buffer → Buffer.from, Promise.$resolve → Promise.resolve, and a Readable.fromWeb stand-in for _ReadableFromWeb). A --preload then mock.module("node-fetch", ...)s it, and the same test file runs against it. The new test fails on the transformed main source and passes on the transformed patched source. The rest of the file gives the same results before and after the patch. The 12 body-stream tests that fail under the stand-in fail the same way in both runs, because the stand-in is not _ReadableFromWeb.
• The debug build in CI is the real green check.
• Reference: node-fetch 3.3.2 under Node 22.14.0, output below.
• prettier --check and oxlint --config=oxlint.json pass on the changed files. tsc --noEmit -p src/js gives no new errors in node-fetch.ts.

🤖 Generated with Claude Code

[claude]

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: oven-sh/bun/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: dad0fef1-02e7-4de6-adef-7897ea490b65

📥 Commits

Reviewing files that changed from the base of the PR and between 29d9638 and 72e3aae.

📒 Files selected for processing (2)

• src/js/thirdparty/node-fetch.ts
• test/js/node/http/node-fetch.test.js

Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.

---

Walkthrough

Updates node-fetch error classes with class-based names and tags, optional system-error properties, and revised AbortError inheritance. Adds tests for these error properties and behaviors.

Changes

node-fetch error properties

Layer / File(s)	Summary
Error class metadata and tests src/js/thirdparty/node-fetch.ts, test/js/node/http/node-fetch.test.js	FetchBaseError exposes a name and Symbol.toStringTag based on its concrete class. FetchError sets optional system-error properties when given system error details. AbortError extends FetchBaseError and defaults its type to "aborted". Tests cover these properties and error identity and formatting.

Suggested reviewers: robobun

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 72e3a

The change updates node-fetch error metadata without changing native fetch abort behavior. No actionable user-facing risk remains, so it is ready for normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 72e3a

The change may affect applications that classify exported errors, but no new security capability or change to the fetch cancellation path was identified. Compatibility effects in external applications remain uncertain.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — Applications can observe the revised exported error objects. The available dependency evidence does not bound all external consumers or establish a new privileged downstream path.

Trust Boundaries and Controls

• observed — The request path continues to delegate URL and options handling to Bun.fetch; the modified exported constructors are not used in that delegation.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: aligning FetchError and AbortError naming with node-fetch.
Description check	✅ Passed	The description includes both required sections, explains the implementation and compatibility goals, and documents verification results and limitations.
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.

---

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

[Hotragn]

I checked the part this rests on: FetchBaseError's name getter returns this.constructor.name, so it only works if release builds keep class names. They do. src/codegen/bundle-modules.ts:217 bundles builtins with --minify-syntax --keep-names and doesn't minify identifiers. On 1.4.2, FetchError.name and AbortError.name already come out right. So the getter gives the same answer in debug and release.

I also compared against real node-fetch 3.3.2 on Node 24, and found a gap next to this PR that I think matters more. This PR fixes the errors you build with new FetchError(...). But most code never builds one. It catches the error fetch() rejects with, and fetch() in the shim does await nativeFetch.$call(undefined, url, init) and passes native errors through unchanged:

fetch() fails because…	node-fetch 3.3.2 on Node 24	Bun 1.4.2
the signal aborted	AbortError, type: "aborted", instanceof AbortError true	DOMException AbortError, no type, instanceof AbortError false
connection refused	FetchError, type: "system", code/errno: "ECONNREFUSED", erroredSysCall: "connect"	TypeError, code: "ConnectionRefused", instanceof FetchError false
DNS lookup failed	FetchError, code: "ENOTFOUND", erroredSysCall: "getaddrinfo"	TypeError, code: "ENOTFOUND", instanceof FetchError false

The usual node-fetch retry and timeout code (if (err instanceof AbortError), if (err instanceof FetchError && err.code === "ECONNREFUSED"), err.type === "aborted") never matches on Bun. That's true before this PR and after it. This PR doesn't cause it. I'm raising it here because this PR's classes are what a fix would build on: catch around the nativeFetch call and rethrow new AbortError("The operation was aborted.") or new FetchError(\request to ${url} failed, reason: ${e.message}`, "system", { code, syscall })`.

Bun's ConnectionRefused would need mapping to ECONNREFUSED. And some code may already check instanceof DOMException on Bun, so the rethrow is a behavior change a maintainer should sign off on. That's why I'd keep it out of this PR. I've opened it as its own issue so it doesn't get lost.

Script to reproduce: install node-fetch@3.3.2, then run it with node and with bun:

import fetch, { AbortError, FetchError } from "node-fetch";
import http from "node:http";
const srv = http.createServer((q, s) => setTimeout(() => s.end("x"), 2000)).listen(0);
await new Promise(r => srv.once("listening", r));
const ac = new AbortController();
const p = fetch(`http://127.0.0.1:${srv.address().port}/`, { signal: ac.signal });
setTimeout(() => ac.abort(), 20);
try { await p; } catch (e) { console.log(e.constructor.name, e instanceof AbortError, e.type); }
try { await fetch("http://127.0.0.1:1/"); } catch (e) { console.log(e.constructor.name, e instanceof FetchError, e.code); }
srv.closeAllConnections(); srv.close();

The changes in the diff look right to me. code/errno/erroredSysCall are set only when there's a system error, like errors/fetch-error.js. The aborted fetch() path can't regress: it rejected with a native DOMException before and still does, so instanceof AbortError was already false.