Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

fix: decompress gzipped responses on Node 24+ (undici 7)#63

Merged
scottlovegrove merged 2 commits intomainDoist/outline-cli:mainfrom
scottl/node-26-undici-gzip-decompressionDoist/outline-cli:scottl/node-26-undici-gzip-decompressionCopy head branch name to clipboard
May 9, 2026
Merged

fix: decompress gzipped responses on Node 24+ (undici 7)#63
scottlovegrove merged 2 commits intomainDoist/outline-cli:mainfrom
scottl/node-26-undici-gzip-decompressionDoist/outline-cli:scottl/node-26-undici-gzip-decompressionCopy head branch name to clipboard

Conversation

@scottlovegrove
Copy link
Copy Markdown
Contributor

Summary

  • Compose undici's built-in interceptors.decompress() onto the dispatcher so gzip/deflate/br/zstd response bodies are decoded before the CLI parses them.
  • Suppress undici's one-time ExperimentalWarning for the decompress interceptor so it does not leak to every ol invocation's stderr.

Problem

On Node 24+, attaching any custom dispatcher to global fetch strips the response's content-encoding header but does not actually decompress the body. ol auth status (and the OAuth login flow's auth.info step) fail with Unexpected token '...' is not valid JSON because res.json() is parsing raw gzipped bytes.

Fix

undici 7 ships a built-in decompress interceptor (public API: undici.interceptors.decompress) that handles gzip/deflate/br/zstd, strips content-encoding/content-length from upstream headers (so Node's fetch layer doesn't try to decompress again), and is composed via the standard Dispatcher.prototype.compose(...) API. ~1 line of dispatcher change; preserves wire compression (no Accept-Encoding: identity workaround, no bandwidth regression).

The undici interceptor is currently flagged ExperimentalWarning but is fully implemented and stable for HTTPS gzipped-JSON. The change includes a small suppressExperimentalWarningsSync helper (exported for direct unit testing) that suppresses every ExperimentalWarning during the synchronous dispatcher-init critical section. The synchronous contract is what makes the global process.emitWarning override safe — no unrelated code can interleave during the microsecond window.

Supersedes #62

#62 sidesteps the bug by sending Accept-Encoding: identity so the server skips compression entirely. That works but trades wire bandwidth for simplicity (~70-80% inflation on JSON payloads). Composing the interceptor is the proper fix and preserves compression. Recommend closing #62 once this lands.

This is the same fix already shipped in @doist/todoist-sdk@10.1.3 (Doist/todoist-cli#318) and @doist/twist-sdk@2.5.1. outline-cli has its own transport layer rather than going through a shared SDK, so the fix lives directly in the CLI.

Test plan

  • npm test — 129/129 passing, including 4 new dispatcher tests:
    • existing Agent / EnvHttpProxyAgent instanceof + cache + reset assertions still pass (compose preserves the prototype chain)
    • decompresses gzip-encoded response bodies — spins up a node:http server, returns Content-Encoding: gzip with raw gzipped JSON, verifies the dispatcher decodes it
    • suppressExperimentalWarningsSync unit tests — directly verify the helper swallows both emitWarning signatures, restores the original on throw, and returns the callback result
  • npm run lint:check — clean
  • npm run format:check — clean
  • npm run type-check — clean

🤖 Generated with Claude Code

Compose undici's response-decompression interceptor onto the dispatcher
so gzip/deflate/br/zstd response bodies are decoded before the CLI
parses them.

On Node 24+, attaching any custom dispatcher to global `fetch` strips
`content-encoding` from the response headers but does not actually
decompress the body, so `res.json()` chokes on raw gzipped bytes with
"Unexpected token ... is not valid JSON" — and `ol auth status` (and
the OAuth login flow's `auth.info` step) fail.

Suppress undici's one-time `ExperimentalWarning` for the decompress
interceptor so it does not leak to every CLI invocation's stderr on
the first request. The interceptor is fully implemented and stable for
our gzipped-JSON-over-HTTPS use case.

Same fix already shipped in @doist/todoist-sdk 10.1.3 (Doist/todoist-cli#318)
and @doist/twist-sdk 2.5.1.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@scottlovegrove scottlovegrove self-assigned this May 9, 2026
@scottlovegrove scottlovegrove added the 👀 Show PR PR must be reviewed before or after merging label May 9, 2026
Copy link
Copy Markdown
Member

@doistbot doistbot left a comment

Choose a reason for hiding this comment

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

Thank you for putting together this fix for Node 24+ decompression; composing the native Undici interceptor is a great approach that preserves bandwidth while resolving the JSON parsing errors. The implementation is clean and effective, though there are a few noted areas for refinement around the experimental warning suppression helper. The feedback focuses on enforcing strict synchronous execution in the helper's type signature, avoiding forced type assertions to better align with repository guidelines, and adding an integration test to verify that the dispatcher successfully swallows the warning in practice.

Share FeedbackReview Logs

Comment thread src/transport/http-dispatcher.ts Outdated
Comment thread src/__tests__/http-dispatcher.test.ts
Comment thread src/transport/http-dispatcher.ts Outdated
- Constrain `suppressExperimentalWarningsSync` to synchronous callbacks
  at the type level via `SyncOnly<T>` (`T extends PromiseLike<unknown>
  ? never : T`). Async usage is now a compile error. Add a runtime
  thenable guard for defence-in-depth.
- Drop the `(originalEmit as (...args: unknown[]) => void)` cast.
  Forward args via `Reflect.apply` with a `Parameters<typeof
  process.emitWarning>` tuple. One cast remains at the assignment
  boundary because TS overloads can't be matched cleanly otherwise.
- Add an integration test that mocks `undici.interceptors.decompress`
  to emit an `ExperimentalWarning` synchronously at compose time and
  verifies `getDefaultDispatcher()` does not forward it. Catches the
  regression case the unit tests can't (helper not wrapping decompress,
  warning emitted before suppression installed).
- Add a runtime test for the thenable guard.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@scottlovegrove scottlovegrove requested review from a team and nvignola and removed request for a team May 9, 2026 11:01
@scottlovegrove scottlovegrove merged commit c44fbea into main May 9, 2026
4 checks passed
@scottlovegrove scottlovegrove deleted the scottl/node-26-undici-gzip-decompression branch May 9, 2026 11:01
doist-release-bot Bot added a commit that referenced this pull request May 9, 2026
## [1.5.2](v1.5.1...v1.5.2) (2026-05-09)

### Bug Fixes

* decompress gzipped responses on Node 24+ (undici 7) ([#63](#63)) ([c44fbea](c44fbea))
@doist-release-bot
Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 1.5.2 🎉

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

released 👀 Show PR PR must be reviewed before or after merging

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.