Skip to content

Fail on stalled Docker route checks#220

Open
komen205 wants to merge 2 commits into
httptoolkit:mainfrom
komen205:codex/docker-route-fail-open
Open

Fail on stalled Docker route checks#220
komen205 wants to merge 2 commits into
httptoolkit:mainfrom
komen205:codex/docker-route-fail-open

Conversation

@komen205
Copy link
Copy Markdown

Summary

This makes Docker routing fail open when Docker is unavailable or stalled, so the Docker tunnel proxy setting cannot block unrelated passthrough traffic.

The docker-tunnel-proxy-* setting is included in the default passthrough rules, which means it can run for browser, Android/ADB, and other non-Docker traffic too. If Docker is present but the daemon is wedged, monitorDockerNetworkAliases() can remain pending and every request that awaits the rule parameter can hang before falling through to a direct connection.

Changes

  • Bound Docker availability checks with a 3s timeout.
  • Extracted Docker tunnel proxy setting creation into a small helper.
  • Added a 250ms fail-open timeout around Docker route monitor lookup.
  • Log Docker route lookup timeout/failure once per proxy setting instead of once per request.
  • Added unit coverage for fail-open behavior when Docker route lookup never resolves.

Validation

  • source ~/.nvm/nvm.sh && nvm use 24.15.0 && npm_config_cache=/tmp/npm-cache-codex npm run test:unit
  • source ~/.nvm/nvm.sh && nvm use 24.15.0 && npm_config_cache=/tmp/npm-cache-codex npm run build:src:ts

I also tried the focused Docker compose integration path locally, but this machine's Docker Desktop setup times out on the fixture's container-local localhost/127.0.0.1 self-calls while cross-container and external calls succeed, so I did not treat that as validation for this patch.

@komen205 komen205 marked this pull request as ready for review May 21, 2026 18:49
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented May 21, 2026

CLA assistant check
All committers have signed the CLA.

Copy link
Copy Markdown
Member

@pimterry pimterry left a comment

Choose a reason for hiding this comment

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

Thanks for looking at this @komen205! I wasn't aware of this issue, but I can imagine how that could happen. I've left a couple of comments to simplify and tweak this but I'm on board with the general concept 👍

Comment thread src/interceptors/docker/docker-interception-services.ts Outdated
Comment thread src/interceptors/docker/docker-interception-services.ts Outdated
Per review on httptoolkit#220:
- Revert the docker-tunnel-proxy rule callback to its original inline form
  and drop the per-request DOCKER_PROXY_SETTING_TIMEOUT_MS timeout. The 3s
  Docker-availability timeout already bounds the network monitor (which
  awaits isDockerAvailable), so this still fails open: only requests in the
  first few seconds of a wedged-Docker startup wait, then everything works.
- Move the timeout duration into TimeoutError/withTimeout so every timeout
  reports "Timeout after <n>ms"; isDockerAvailable's catch returns to its
  original one-line form.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@komen205 komen205 force-pushed the codex/docker-route-fail-open branch from f7a3e23 to 9782f91 Compare May 28, 2026 18:44
@komen205 komen205 requested a review from pimterry May 28, 2026 19:34
@komen205 komen205 changed the title [codex] Fail open on stalled Docker route checks Fail on stalled Docker route checks May 28, 2026
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.

3 participants