Skip to content

fix: fallback to undiciFetch when globalThis.fetch is unavailable#840

Open
deep-innovator wants to merge 1 commit intojackwener:mainfrom
deep-innovator:patch-1
Open

fix: fallback to undiciFetch when globalThis.fetch is unavailable#840
deep-innovator wants to merge 1 commit intojackwener:mainfrom
deep-innovator:patch-1

Conversation

@deep-innovator
Copy link
Copy Markdown

Problem

On Windows with older Node.js versions, globalThis.fetch is not available at module load time, causing the original nativeFetch definition to fail immediately.

Solution

  • Keep nativeFetch as a direct request function, preserving the original signature (input, init) => Promise<Response>.
  • Fall back to undiciFetch synchronously when globalThis.fetch is unavailable.
  • This maintains the existing no-proxy path behavior while fixing the startup error on Windows.

Verification

  • No breaking changes to the call site return nativeFetch(input, init).
  • Works both with native fetch and the undiciFetch fallback.

Resolves compatibility issue on Windows with older Node.js versions where `globalThis.fetch` is not available at module load time.

This change keeps the original `nativeFetch` function signature unchanged, preserving the no-proxy path behavior while adding a synchronous fallback to `undiciFetch`.
@Astro-Han
Copy link
Copy Markdown
Contributor

I reproduced the current failure by deleting globalThis.fetch before importing src/node-network.ts. On main, that throws at the eager bind, so this PR is fixing the right spot.

One thing still missing is a regression test for the no-fetch module-load path. The current tests cover proxy decisions after import, but not the startup case this change is meant to fix.

Also worth confirming the support target here. package.json still says node >=20, so this is either a best-effort compatibility fix for unsupported runtimes or a sign that the engine floor should be revisited.

@deep-innovator
Copy link
Copy Markdown
Author

deep-innovator commented Apr 7, 2026 via email

const directDispatcher = new Agent();
const proxyDispatcherCache = new Map<string, Dispatcher>();
const nativeFetch = globalThis.fetch.bind(globalThis);
const nativeFetch = globalThis.fetch
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would it be simpler to make nativeFetch a tiny wrapper instead of choosing once at module load? As written, if fetch is missing during import but gets installed later, the direct path is still pinned to undiciFetch for the rest of the process.

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.

2 participants