Skip to content

fix: honour HTTP_PROXY on the customFetch path#305

Merged
scottlovegrove merged 2 commits intomainfrom
scottl/fix-proxy
May 1, 2026
Merged

fix: honour HTTP_PROXY on the customFetch path#305
scottlovegrove merged 2 commits intomainfrom
scottl/fix-proxy

Conversation

@scottlovegrove
Copy link
Copy Markdown
Collaborator

Summary

  • Bumps @doist/todoist-sdk to 10.1.0 (which now publicly exports getDefaultDispatcher).
  • In createTrackedFetch and fetchTodoist, attach the SDK's shared EnvHttpProxyAgent dispatcher whenever the underlying fetch is the real native one — restoring HTTP_PROXY / HTTPS_PROXY / NO_PROXY support on the customFetch path.
  • Test stubs continue to skip the dispatcher so their captured options stay clean.

Fixes #304.

Why

#302 (1.60.0) routed every Todoist-bound request through a customFetch so we could attach the usage-tracking headers. The SDK's transport/fetch-with-retry.ts only attaches the proxy dispatcher on its native-fetch branch — once a customFetch is supplied, that branch is skipped, so the proxy env vars were silently dropped. This regressed every corporate-proxy user from 1.60.0 onwards.

The SDK fix landed in Doist/todoist-sdk-typescript#594, shipped as 10.1.0. This PR consumes that and wires it into our two outbound paths.

Test plan

  • npm run type-check clean
  • npm run check clean (lint + format)
  • npm test — 1577 passing (4 new dispatcher cases added)
  • Manual proxy check: HTTPS_PROXY=http://localhost:8888 td today transits the proxy
  • Manual NO_PROXY check: HTTPS_PROXY=http://localhost:8888 NO_PROXY=api.todoist.com td today bypasses the proxy
  • Manual OAuth check: HTTPS_PROXY=http://localhost:8888 td auth login token exchange transits the proxy

🤖 Generated with Claude Code

PR #302 routed every Todoist-bound request through a `customFetch` to
attach usage-tracking headers, which silently bypassed the SDK's
native-fetch branch — the only place `EnvHttpProxyAgent` was wired up.
Result: `HTTP_PROXY` / `HTTPS_PROXY` / `NO_PROXY` were ignored from
1.60.0 onwards.

Attach the SDK's shared dispatcher (re-exported in @doist/todoist-sdk
10.1.0) inside `createTrackedFetch` and `fetchTodoist` whenever the
underlying fetch is the real native one. Test stubs continue to skip
the dispatcher to keep their captured options clean.

Fixes #304.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@scottlovegrove scottlovegrove self-assigned this May 1, 2026
@scottlovegrove scottlovegrove requested a review from gnapse May 1, 2026 16:08
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.

This PR effectively restores proxy support for corporate users by integrating the newly exposed getDefaultDispatcher from the SDK into the outbound customFetch paths. It is a crucial fix that cleanly bridges the proxy regression while keeping test stubs properly isolated. A few refinements are noted to ensure explicitly provided dispatchers aren't inadvertently overwritten, to extract the duplicated proxy-attachment logic into a shared helper, and to streamline both the test mock setups and TypeScript assertions for better maintainability.

Share FeedbackReview Logs

Comment thread src/lib/usage-tracking.ts
Comment thread src/lib/usage-tracking.test.ts Outdated
Comment thread src/lib/usage-tracking.ts Outdated
Comment thread src/lib/usage-tracking.ts
- Fold the native-fetch check into the helper as
  `attachDispatcherIfNative(fetchImpl, options)`, so both call sites stop
  duplicating the gate.
- Skip attachment if the caller already set `dispatcher` on the options
  — defensive against future composition; no current caller does this.
- Replace the test boilerplate that swapped `globalThis.fetch` in/out
  via `try/finally` with `vi.spyOn(globalThis, 'fetch')`, restored
  through `vi.restoreAllMocks()` in `afterEach`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@scottlovegrove scottlovegrove added the 👀 Show PR PR must be reviewed before or after merging label May 1, 2026
@scottlovegrove scottlovegrove merged commit e1fc71a into main May 1, 2026
4 checks passed
@scottlovegrove scottlovegrove deleted the scottl/fix-proxy branch May 1, 2026 16:41
doist-release-bot Bot added a commit that referenced this pull request May 1, 2026
## [1.60.1](v1.60.0...v1.60.1) (2026-05-01)

### Bug Fixes

* honour HTTP_PROXY on the customFetch path ([#305](#305)) ([e1fc71a](e1fc71a))
@doist-release-bot
Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 1.60.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

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

HTTP_PROXY/HTTPS_PROXY no longer honored after 1.60.0

2 participants