Skip to content

fix: prefer native fetch over node-fetch in Node.js - #9101

Open
HelloWNW wants to merge 2 commits into
googleapis:mainfrom
HelloWNW:fix-gaxios-prefer-native-fetch-in-node
Open

fix: prefer native fetch over node-fetch in Node.js#9101
HelloWNW wants to merge 2 commits into
googleapis:mainfrom
HelloWNW:fix-gaxios-prefer-native-fetch-in-node

Conversation

@HelloWNW

@HelloWNW HelloWNW commented Aug 6, 2026

Copy link
Copy Markdown

Problem

Gaxios.#getFetch() only checks for a browser window before falling back to
node-fetch, so Node.js always uses node-fetch even though native fetch has
been available since Node 18 — this package's minimum supported version.

node-fetch intermittently rejects with Premature close on some networks. This
reproduces as a consistent clasp login failure, whose OAuth2 token exchange runs
through google-auth-library and gaxios:

Invalid response body while trying to fetch https://oauth2.googleapis.com/token: Premature close

The same POST succeeds via curl and via Node's native fetch on the same machine,
so the failure is specific to node-fetch.

Change

Prefer native fetch in Node.js, keeping node-fetch as a fallback when no global
fetch exists. Browser behavior is unchanged.

Two implementation differences are normalized to preserve existing behavior:

  • responseType: 'stream' converts native fetch's ReadableStream back to a
    stream.Readable. A caller-supplied fetchImplementation still returns its own
    body type, as covered by the existing test.
  • An AbortSignal reason that is not an Error is propagated rather than replaced
    with a generic message. The existing test for this case carried a comment
    anticipating the change, which is now applied.

Testing

npm test — 129 passing.

node-fetch can intermittently reject requests with `Premature close`,
notably POSTs to the OAuth2 token endpoint. Native fetch is unaffected
and has been stable since Node 18, this package's minimum supported
version. node-fetch remains a fallback when no global fetch exists.

Two differences between the implementations are normalized so the
default path behaves as before:

- responseType: 'stream' converts the ReadableStream back to a
  stream.Readable. A caller-provided fetchImplementation is untouched.
- A non-Error AbortSignal reason is propagated instead of being
  replaced with a generic message.
@HelloWNW
HelloWNW requested a review from a team as a code owner August 6, 2026 12:40
@google-cla

google-cla Bot commented Aug 6, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@gemini-code-assist gemini-code-assist Bot left a comment

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.

Code Review

This pull request updates Gaxios to prefer the native globalThis.fetch over node-fetch in environments where it is available (Node 18+), updating error handling and tests to align with native fetch's behavior. To maintain the stream.Readable contract when using native fetch, Readable.fromWeb is used to convert the response body. The review feedback highlights a potential runtime crash in browser environments where Readable might be undefined, causing instanceof Readable to throw a TypeError. It is recommended to safely check if Readable and Readable.fromWeb are defined before performing the check and conversion.

Comment thread core/packages/gaxios/src/gaxios.ts Outdated
Comment on lines +287 to +294
if (usingInternalFetch && res.body && !(res.body instanceof Readable)) {
// Native `fetch` resolves a `ReadableStream`, so convert it to retain
// the `stream.Readable` contract. A caller-provided
// `fetchImplementation` keeps its own body type.
return Readable.fromWeb(
res.body as unknown as import('stream/web').ReadableStream,
);
}

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.

high

In browser environments or environments where the stream module is stubbed or not polyfilled, Readable may be undefined. Referencing instanceof Readable when Readable is undefined will throw a TypeError: Right-hand side of 'instanceof' is not an object.

To prevent runtime crashes in browser environments, we should safely check if Readable is defined and is a function, and whether Readable.fromWeb is available before using them.

        if (
          usingInternalFetch &&
          res.body &&
          typeof Readable === 'function' &&
          typeof Readable.fromWeb === 'function' &&
          !(res.body instanceof Readable)
        ) {
          // Native fetch resolves a ReadableStream, so convert it to retain
          // the stream.Readable contract. A caller-provided
          // fetchImplementation keeps its own body type.
          return Readable.fromWeb(
            res.body as unknown as import('stream/web').ReadableStream,
          );
        }

Readable.fromWeb is unavailable in browser bundles, where stream is
stubbed or polyfilled by stream-browserify, and Readable itself may be
undefined. Detect a web stream by its getReader method and confirm
fromWeb exists before converting, so browsers keep resolving a
ReadableStream as they did previously.
@HelloWNW

HelloWNW commented Aug 6, 2026

Copy link
Copy Markdown
Author

Addressed in e8ec31e — the conversion now checks that fromWeb is available before using it, and no longer references instanceofReadable.

This is my first contribution to a project of this size, so apologies for any review overhead. Thanks for taking a look.

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