From ace728ce0e4e8473d2f3189cfe31f48834bcc30e Mon Sep 17 00:00:00 2001 From: nuno <59452877+komen205@users.noreply.github.com> Date: Thu, 21 May 2026 19:47:32 +0100 Subject: [PATCH 1/2] Fail open on stalled Docker route checks --- .../docker/docker-interception-services.ts | 76 +++++++++++++++---- .../unit/docker-interception-services.spec.ts | 43 +++++++++++ 2 files changed, 105 insertions(+), 14 deletions(-) create mode 100644 test/unit/docker-interception-services.spec.ts diff --git a/src/interceptors/docker/docker-interception-services.ts b/src/interceptors/docker/docker-interception-services.ts index 62b3d5e82..76acc5f0a 100644 --- a/src/interceptors/docker/docker-interception-services.ts +++ b/src/interceptors/docker/docker-interception-services.ts @@ -21,14 +21,22 @@ import { stopDockerTunnel, } from './docker-tunnel-proxy'; import { ensureDockerInjectionVolumeExists } from './docker-data-injection'; +import { TimeoutError, withTimeout } from '../../util/promise'; let dockerAvailableCache: Promise | undefined; +const DOCKER_AVAILABILITY_TIMEOUT_MS = 3_000; +const DOCKER_PROXY_SETTING_TIMEOUT_MS = 250; + +type DockerNetworkRouteMonitor = { + dockerRoutedAliases: ReadonlySet +}; + export const isDockerAvailable = (options: { logError?: boolean } = {}) => { if (dockerAvailableCache) return dockerAvailableCache; else { dockerAvailableCache = (async () => { // Catch sync & async setup errors - return new Docker().info(); + return withTimeout(DOCKER_AVAILABILITY_TIMEOUT_MS, new Docker().info()); })() .then((info: { OSType?: 'windows' | 'linux' }) => { if (info.OSType === 'windows') { @@ -40,7 +48,12 @@ export const isDockerAvailable = (options: { logError?: boolean } = {}) => { } }) .catch((error) => { - if (options.logError) console.warn('Docker not available:', error.message); + if (options.logError) { + const errorMessage = error instanceof TimeoutError + ? `timed out after ${DOCKER_AVAILABILITY_TIMEOUT_MS}ms` + : error.message; + console.warn('Docker not available:', errorMessage); + } return false; }); @@ -53,6 +66,48 @@ export const isDockerAvailable = (options: { logError?: boolean } = {}) => { const IPv4_IPv6_PREFIX = "::ffff:"; +// This proxy setting is included in the default passthrough rules, so it can run for +// browser/Android/etc traffic too. It must fail open quickly if Docker is unavailable. +export function getDockerTunnelProxySetting( + proxyPort: number, + networkMonitor: Promise, + options: { timeoutMs?: number } = {} +): ProxySettingCallback { + const timeoutMs = options.timeoutMs ?? DOCKER_PROXY_SETTING_TIMEOUT_MS; + let hasLoggedTimeout = false; + let hasLoggedFailure = false; + + return async ({ hostname }: { hostname: any }) => { + hostname = hostname.startsWith(IPv4_IPv6_PREFIX) + ? hostname.slice(IPv4_IPv6_PREFIX.length) + : hostname; + + const monitor = await withTimeout(timeoutMs, networkMonitor).catch((error) => { + if (error instanceof TimeoutError) { + if (!hasLoggedTimeout) { + hasLoggedTimeout = true; + console.warn( + `Docker route lookup timed out after ${timeoutMs}ms; skipping Docker tunnel routing` + ); + } + } else { + if (!hasLoggedFailure) { + hasLoggedFailure = true; + logError(error); + } + } + + return undefined; + }); + + if (monitor?.dockerRoutedAliases.has(hostname)) { + return { + proxyUrl: `socks5://127.0.0.1:${await getDockerTunnelPort(proxyPort)}` + }; + } + }; +} + // On shutdown, clean up every container & image that we created, disappearing // into the mist as if we were never here... // (Those images/containers are unusable without us, so leaving them breaks things). @@ -91,17 +146,10 @@ export async function startDockerInterceptionServices( const networkMonitor = monitorDockerNetworkAliases(proxyPort); - ruleParameters[`docker-tunnel-proxy-${proxyPort}`] = async ({ hostname }: { hostname: any }) => { - hostname = hostname.startsWith(IPv4_IPv6_PREFIX) - ? hostname.slice(IPv4_IPv6_PREFIX.length) - : hostname; - - if ((await networkMonitor)?.dockerRoutedAliases.has(hostname)) { - return { - proxyUrl: `socks5://127.0.0.1:${await getDockerTunnelPort(proxyPort)}` - }; - } - }; + ruleParameters[`docker-tunnel-proxy-${proxyPort}`] = getDockerTunnelProxySetting( + proxyPort, + networkMonitor + ); console.log(`Created docker-tunnel-proxy-${proxyPort}`); await Promise.all([ @@ -203,4 +251,4 @@ export async function deleteAllInterceptedDockerData(proxyPort: number | 'all'): delete pendingDeactivations[proxyPort]; })() ]) as Promise as Promise; -} \ No newline at end of file +} diff --git a/test/unit/docker-interception-services.spec.ts b/test/unit/docker-interception-services.spec.ts new file mode 100644 index 000000000..d1bdb262c --- /dev/null +++ b/test/unit/docker-interception-services.spec.ts @@ -0,0 +1,43 @@ +import { expect } from 'chai'; + +import { + getDockerTunnelProxySetting +} from '../../src/interceptors/docker/docker-interception-services'; + +describe("Docker interception services", () => { + + it("should fail open if Docker route lookup does not resolve quickly", async () => { + const originalWarn = console.warn; + console.warn = () => {}; + + try { + const proxySetting = getDockerTunnelProxySetting( + 8000, + new Promise(() => {}), + { timeoutMs: 10 } + ); + + const startTime = Date.now(); + const result = await proxySetting({ hostname: 'example.com' } as any); + + expect(result).to.equal(undefined); + expect(Date.now() - startTime).to.be.lessThan(200); + } finally { + console.warn = originalWarn; + } + }); + + it("should not use the Docker tunnel for non-Docker hostnames", async () => { + const proxySetting = getDockerTunnelProxySetting( + 8000, + Promise.resolve({ + dockerRoutedAliases: new Set(['service-a']) + }) + ); + + const result = await proxySetting({ hostname: 'example.com' } as any); + + expect(result).to.equal(undefined); + }); + +}); From 9782f91631945d12da05ca8f1c11d0d33bc865c1 Mon Sep 17 00:00:00 2001 From: nuno <59452877+komen205@users.noreply.github.com> Date: Thu, 28 May 2026 18:38:49 +0100 Subject: [PATCH 2/2] Address review feedback: simplify to bounded availability check Per review on #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 ms"; isDockerAvailable's catch returns to its original one-line form. Co-Authored-By: Claude Opus 4.8 --- .../docker/docker-interception-services.ts | 71 ++++--------------- src/util/promise.ts | 9 ++- .../unit/docker-interception-services.spec.ts | 43 ----------- 3 files changed, 19 insertions(+), 104 deletions(-) delete mode 100644 test/unit/docker-interception-services.spec.ts diff --git a/src/interceptors/docker/docker-interception-services.ts b/src/interceptors/docker/docker-interception-services.ts index 76acc5f0a..a1fa4bd17 100644 --- a/src/interceptors/docker/docker-interception-services.ts +++ b/src/interceptors/docker/docker-interception-services.ts @@ -21,16 +21,11 @@ import { stopDockerTunnel, } from './docker-tunnel-proxy'; import { ensureDockerInjectionVolumeExists } from './docker-data-injection'; -import { TimeoutError, withTimeout } from '../../util/promise'; +import { withTimeout } from '../../util/promise'; let dockerAvailableCache: Promise | undefined; const DOCKER_AVAILABILITY_TIMEOUT_MS = 3_000; -const DOCKER_PROXY_SETTING_TIMEOUT_MS = 250; - -type DockerNetworkRouteMonitor = { - dockerRoutedAliases: ReadonlySet -}; export const isDockerAvailable = (options: { logError?: boolean } = {}) => { if (dockerAvailableCache) return dockerAvailableCache; @@ -48,12 +43,7 @@ export const isDockerAvailable = (options: { logError?: boolean } = {}) => { } }) .catch((error) => { - if (options.logError) { - const errorMessage = error instanceof TimeoutError - ? `timed out after ${DOCKER_AVAILABILITY_TIMEOUT_MS}ms` - : error.message; - console.warn('Docker not available:', errorMessage); - } + if (options.logError) console.warn('Docker not available:', error.message); return false; }); @@ -66,48 +56,6 @@ export const isDockerAvailable = (options: { logError?: boolean } = {}) => { const IPv4_IPv6_PREFIX = "::ffff:"; -// This proxy setting is included in the default passthrough rules, so it can run for -// browser/Android/etc traffic too. It must fail open quickly if Docker is unavailable. -export function getDockerTunnelProxySetting( - proxyPort: number, - networkMonitor: Promise, - options: { timeoutMs?: number } = {} -): ProxySettingCallback { - const timeoutMs = options.timeoutMs ?? DOCKER_PROXY_SETTING_TIMEOUT_MS; - let hasLoggedTimeout = false; - let hasLoggedFailure = false; - - return async ({ hostname }: { hostname: any }) => { - hostname = hostname.startsWith(IPv4_IPv6_PREFIX) - ? hostname.slice(IPv4_IPv6_PREFIX.length) - : hostname; - - const monitor = await withTimeout(timeoutMs, networkMonitor).catch((error) => { - if (error instanceof TimeoutError) { - if (!hasLoggedTimeout) { - hasLoggedTimeout = true; - console.warn( - `Docker route lookup timed out after ${timeoutMs}ms; skipping Docker tunnel routing` - ); - } - } else { - if (!hasLoggedFailure) { - hasLoggedFailure = true; - logError(error); - } - } - - return undefined; - }); - - if (monitor?.dockerRoutedAliases.has(hostname)) { - return { - proxyUrl: `socks5://127.0.0.1:${await getDockerTunnelPort(proxyPort)}` - }; - } - }; -} - // On shutdown, clean up every container & image that we created, disappearing // into the mist as if we were never here... // (Those images/containers are unusable without us, so leaving them breaks things). @@ -146,10 +94,17 @@ export async function startDockerInterceptionServices( const networkMonitor = monitorDockerNetworkAliases(proxyPort); - ruleParameters[`docker-tunnel-proxy-${proxyPort}`] = getDockerTunnelProxySetting( - proxyPort, - networkMonitor - ); + ruleParameters[`docker-tunnel-proxy-${proxyPort}`] = async ({ hostname }: { hostname: any }) => { + hostname = hostname.startsWith(IPv4_IPv6_PREFIX) + ? hostname.slice(IPv4_IPv6_PREFIX.length) + : hostname; + + if ((await networkMonitor)?.dockerRoutedAliases.has(hostname)) { + return { + proxyUrl: `socks5://127.0.0.1:${await getDockerTunnelPort(proxyPort)}` + }; + } + }; console.log(`Created docker-tunnel-proxy-${proxyPort}`); await Promise.all([ diff --git a/src/util/promise.ts b/src/util/promise.ts index 99ec6f9ba..cf4554f7d 100644 --- a/src/util/promise.ts +++ b/src/util/promise.ts @@ -23,8 +23,11 @@ export async function waitUntil( } export class TimeoutError extends CustomError { - constructor() { - super('Timeout', { code: 'timeout' }); + constructor(timeoutMs?: number) { + super( + timeoutMs === undefined ? 'Timeout' : `Timeout after ${timeoutMs}ms`, + { code: 'timeout' } + ); } } @@ -35,6 +38,6 @@ export async function withTimeout( return Promise.race([ promise, delay(timeoutMs, { unref: true }) - .then(() => { throw new TimeoutError(); }) + .then(() => { throw new TimeoutError(timeoutMs); }) ]); } \ No newline at end of file diff --git a/test/unit/docker-interception-services.spec.ts b/test/unit/docker-interception-services.spec.ts deleted file mode 100644 index d1bdb262c..000000000 --- a/test/unit/docker-interception-services.spec.ts +++ /dev/null @@ -1,43 +0,0 @@ -import { expect } from 'chai'; - -import { - getDockerTunnelProxySetting -} from '../../src/interceptors/docker/docker-interception-services'; - -describe("Docker interception services", () => { - - it("should fail open if Docker route lookup does not resolve quickly", async () => { - const originalWarn = console.warn; - console.warn = () => {}; - - try { - const proxySetting = getDockerTunnelProxySetting( - 8000, - new Promise(() => {}), - { timeoutMs: 10 } - ); - - const startTime = Date.now(); - const result = await proxySetting({ hostname: 'example.com' } as any); - - expect(result).to.equal(undefined); - expect(Date.now() - startTime).to.be.lessThan(200); - } finally { - console.warn = originalWarn; - } - }); - - it("should not use the Docker tunnel for non-Docker hostnames", async () => { - const proxySetting = getDockerTunnelProxySetting( - 8000, - Promise.resolve({ - dockerRoutedAliases: new Set(['service-a']) - }) - ); - - const result = await proxySetting({ hostname: 'example.com' } as any); - - expect(result).to.equal(undefined); - }); - -});