Skip to content

fix: bound the startup zone check so a hung API cannot freeze the server - #176

Open
karaposu wants to merge 1 commit into
brightdata:mainfrom
karaposu:fix/zone-check-timeout
Open

fix: bound the startup zone check so a hung API cannot freeze the server#176
karaposu wants to merge 1 commit into
brightdata:mainfrom
karaposu:fix/zone-check-timeout

Conversation

@karaposu

Copy link
Copy Markdown

Problem

The very first thing the process does — before the FastMCP server object exists — is
await ensure_required_zones(): a network round-trip to list (and possibly create) the account's
zones, using raw axios with no timeout on any of its three calls. A slow or hung Bright Data
API therefore freezes startup indefinitely: the MCP client spawns the subprocess, the handshake never
happens, and the user sees a generic "connection failed" with the real cause invisible.

The failure path was already non-fatal (the catch logs a warning and the server boots) — only the
hang was unbounded.

Fix

Add the missing ceiling:

const zone_check_timeout = base_timeout || 10*1000;

applied as timeout: on all three zone calls. BASE_TIMEOUT is honored when configured; otherwise a
10 s default applies — including when BASE_TIMEOUT=0, since "no timeout" is never an acceptable
setting for startup (tool calls keep their unbounded-by-choice semantics). The expression is NaN-safe
on current main (BASE_TIMEOUT=abc → falls through to 10000). A timeout now lands in the existing
catch as Error checking/creating zones: timeout of 10000ms exceeded followed by a normal boot — the
same degradation a 401 or 500 already gets.

Behaviour change

Startup no longer blocks indefinitely on a slow/hung Bright Data API; success and failure paths are
unchanged.

Tests

test/zone-check-timeout.test.js: a blackhole proxy (accepts connections, never responds) routed via
HTTPS_PROXY into the spawned server, proving the server still completes the MCP handshake and
serves tools/list within the bound. Verified red: the unfixed server never handshakes and the test
fails bounded. NO_PROXY is blanked so a developer shell's proxy rules can't bypass the blackhole;
the stdio pipe between test and server is proxy-immune.

The very first thing the process does -- before the FastMCP server object
exists -- is await ensure_required_zones(): a network round-trip to list
(and possibly create) the account's zones, using raw axios with no timeout
on any of its three calls. A slow or hung Bright Data API therefore froze
startup indefinitely: the MCP client spawns the subprocess, the handshake
never happens, and the user sees a generic "connection failed" with the
real cause invisible.

The failure path was already non-fatal (the catch logs a warning and the
server boots); only the hang was unbounded. Add the missing ceiling:

    const zone_check_timeout = base_timeout || 10*1000;

applied as timeout: on all three zone calls. BASE_TIMEOUT is honored when
configured; otherwise a 10s default applies -- including when
BASE_TIMEOUT=0, since "no timeout" is never an acceptable setting for
startup (tool calls keep their unbounded-by-choice semantics). The
expression is also NaN-safe on this base (BASE_TIMEOUT=abc -> NaN ->
falls through to 10000). A timeout now lands in the existing catch as
"Error checking/creating zones: timeout of 10000ms exceeded" followed by a
normal boot -- the same degradation a 401 or 500 already gets.

Adds test/zone-check-timeout.test.js: a blackhole proxy (accepts
connections, never responds) routed via HTTPS_PROXY into the spawned
server, proving the server still completes the MCP handshake and serves
tools/list within the bound (verified red: the unfixed server never
handshakes and the test fails bounded at 10s). NO_PROXY is blanked so a
developer shell's proxy rules cannot bypass the blackhole; the stdio pipe
between test and server is proxy-immune.
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.

1 participant