Skip to content

fix: use undici's own fetch to avoid version mismatch on Node 26#44

Merged
scottlovegrove merged 2 commits into
mainfrom
fix/undici-fetch-version-mismatch
Jul 2, 2026
Merged

fix: use undici's own fetch to avoid version mismatch on Node 26#44
scottlovegrove merged 2 commits into
mainfrom
fix/undici-fetch-version-mismatch

Conversation

@scottlovegrove

Copy link
Copy Markdown
Collaborator

Problem

On Node 26, every authenticated Comms request fails with CommsRequestError: terminated. Reproduced against the real CLI on Node 26 (tdc doctorStored credentials failed validation: terminated); Node 22/24 are unaffected.

Root cause

The transport builds its dispatcher (EnvHttpProxyAgent + interceptors.decompress()) from the npm undici package (7.28.0), then hands it to Node's global fetch, which is backed by whatever undici ships inside the Node release:

Node built-in undici
22 6.24.1
24 7.28.0
26 8.5.0

Driving a v7 dispatcher/interceptor with a v8 fetch core makes the decompress interceptor terminate gzip responses mid-stream. Bumping the npm undici is not a fix — it just moves the mismatch (npm undici 8 on Node 24 hangs).

Global fetch cannot satisfy both response framings on Node 26 either:

  • Chunked gzip responses need the decompress interceptor (see Doist/todoist-cli#318).
  • The Comms API's Content-Length gzip responses break with it.

Only pairing the dispatcher with undici's own fetch — one undici version end to end — decodes both framings correctly.

Fix

  • Source fetch from the same import('undici') that builds the dispatcher, and use it on the Node path; fall back to the global fetch in the browser/edge path (no dispatcher).
  • Server-side gzip is kept — no bandwidth regression.

Tests

MSW intercepts the global fetch, not the separate undici instance, so a suite-wide seam in msw-setup.ts routes MSW tests through the global fetch; the transport tests opt out and exercise the real undici path. Added coverage that undici's paired fetch is preferred, with global-fetch fallback.

Verified

  • Full suite: 166/166 pass; tsc + oxlint + oxfmt clean.
  • Real CLI doctor + inbox on Node 22 / 24 / 26: all pass (was terminated on 26).
  • Compression retained (gzip still on the wire).

The same three-line pattern applies to twist-sdk and todoist-sdk (near-identical transport, minus the allowH2 line) — follow-up once this is confirmed.

🤖 Generated with Claude Code

@scottlovegrove scottlovegrove self-assigned this Jul 2, 2026

@doistbot doistbot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This PR pairs undici's own fetch with its dispatcher to eliminate the version mismatch that caused gzip responses to fail on Node 26, with a clean fallback to global fetch for browser/edge paths.

Few things worth tightening:

  • Atomicity of dispatcher/fetch pairing: dispatcher and fetchImpl are read from separate mutable singletons, so a concurrent closeDefaultDispatcher() can clear defaultFetch while the old dispatcher is still in use — recreating the exact mismatch this PR fixes. Consider returning { dispatcher, fetch } from a single accessor or caching them together so callers can never observe a mixed transport.
  • Missing test for the core wiring: The assignment defaultFetch = undiciFetch (after getDefaultDispatcher() resolves on the Node path) is untested — removing it would silently revert the fix with no test failure. A simple assertion alongside the existing "returns a dispatcher in Node" test (e.g., expect(getDefaultFetch()).toEqual(undiciFetch)) would cover the bridge.
  • Unnecessary optional chaining: getDefaultFetch is always a defined named import (and every mock supplies vi.fn()), so ?.() is a pointless truthiness check on every request. Use getDefaultFetch() ?? fetch instead.

I also included a few optional follow-up notes in the details below.

Optional follow-up note (1)
  • P3 src/transport/fetch-with-retry.ts:97: getDefaultFetch is always a defined function export — it can never be null or undefined at the call site (it's a named import, and every test mock also supplies it as vi.fn()). The ?.() optional chaining adds a pointless truthiness check on every request. Use getDefaultFetch() ?? fetch instead.

Share FeedbackReview Logs

Comment thread src/transport/fetch-with-retry.ts Outdated
Comment thread src/transport/http-dispatcher.test.ts
The transport builds its dispatcher (EnvHttpProxyAgent + decompress
interceptor) from the npm `undici` package, then hands it to Node's global
`fetch`, which is backed by whatever undici ships inside the Node release
(6.x on Node 22 … 8.x on Node 26). Driving a dispatcher from one undici
version with a `fetch` core from another makes the decompress interceptor
terminate gzip responses mid-stream with `terminated` — every authenticated
Comms request fails on Node 26.

Global `fetch` cannot satisfy both response framings on Node 26: chunked
responses need the decompress interceptor, whereas the Comms API's
Content-Length responses break with it. Only pairing the dispatcher with
undici's own `fetch` — one version end to end — decodes both correctly.

Source `fetch` from the same `import('undici')` that builds the dispatcher
and use it on the Node path; fall back to the global `fetch` in the
browser/edge path where there is no dispatcher. Server-side compression is
kept.

Tests intercept the global `fetch` via MSW, which does not see the separate
undici instance, so a suite-wide seam routes MSW tests through the global
`fetch`; the transport tests opt out and exercise the real undici path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@scottlovegrove scottlovegrove force-pushed the fix/undici-fetch-version-mismatch branch from e6abe3e to 6908bd3 Compare July 2, 2026 16:18
Address review feedback:

- Cache the dispatcher and its paired `fetch` as one `DefaultTransport`
  object read via `getDefaultTransport()`, so a concurrent
  `closeDefaultDispatcher()` can no longer clear the fetch between the two
  reads and leave an old dispatcher paired with the global `fetch`.
- Add a test asserting `getDefaultFetch()` returns undici's own `fetch`
  after the dispatcher resolves, covering the wiring that fixes the mismatch.
- Drop the pointless optional chaining on the `getDefaultFetch` import.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@scottlovegrove scottlovegrove requested review from a team and Bloomca and removed request for a team July 2, 2026 16:27
@scottlovegrove scottlovegrove merged commit 59f3677 into main Jul 2, 2026
4 checks passed
@scottlovegrove scottlovegrove deleted the fix/undici-fetch-version-mismatch branch July 2, 2026 16:27
doist-release-bot Bot added a commit that referenced this pull request Jul 2, 2026
## [0.7.1](v0.7.0...v0.7.1) (2026-07-02)

### Bug Fixes

* use undici's own fetch to avoid version mismatch on Node 26 ([#44](#44)) ([59f3677](59f3677))
@doist-release-bot

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 0.7.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants