Skip to content

fix: add 30s timeout to GoldSky subscriber fetch#575

Merged
realfishsam merged 1 commit into
mainfrom
fix/512-goldsky-timeout
May 24, 2026
Merged

fix: add 30s timeout to GoldSky subscriber fetch#575
realfishsam merged 1 commit into
mainfrom
fix/512-goldsky-timeout

Conversation

@realfishsam
Copy link
Copy Markdown
Contributor

Fixes #512

@realfishsam
Copy link
Copy Markdown
Contributor Author

PR Review: PASS (NOT VERIFIED)

What This Does

Adds a 30-second timeout to GoldSky subscriber GraphQL fetch calls using AbortSignal.timeout(), combined with the existing caller-provided abort signal via a shared AbortController. Prevents indefinite hangs when the GoldSky subgraph endpoint is unresponsive.

Blast Radius

  • Core: core/src/subscriber/external/goldsky.ts only
  • No OpenAPI, SDK, or type changes

Findings

  1. Signal composition is correct: The pattern of creating a combinedController and wiring both the caller's signal and AbortSignal.timeout(30_000) to abort it is the standard approach. The finally block properly removes both event listeners to avoid leaks.
  2. Node.js compatibility: AbortSignal.timeout() requires Node 17.3+. The project requires Node >=18.0.0, so this is safe. This is the first use of AbortSignal.timeout() in the codebase.
  3. TimeoutError handling: The catch block now also suppresses TimeoutError (in addition to AbortError), returning null silently. This is consistent with the existing behavior of treating fetch failures as non-fatal for polling subscribers. However, a logger.debug for timeouts specifically might help debugging slow endpoints in production. Non-blocking.
  4. Hardcoded 30s timeout: The timeout is not configurable. This matches the existing DEFAULT_WATCH_TIMEOUT_MS constant used elsewhere, but a config option would be more flexible. Non-blocking.

PMXT Pipeline Check

  • Field propagation: N/A
  • OpenAPI sync: N/A
  • Type safety: OK

Semver Impact

patch -- internal resilience fix, no API or behavioral change

Risk

Not verified against a live GoldSky endpoint. The 30s timeout is reasonable but may be too aggressive if the subgraph legitimately takes longer under load. Monitor after deploy.

@realfishsam realfishsam merged commit a5f7ce9 into main May 24, 2026
10 of 11 checks passed
@realfishsam realfishsam deleted the fix/512-goldsky-timeout branch May 24, 2026 17:04
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.

Missing timeout: GoldSky subscriber — fetch() has AbortSignal but no time-based abort

1 participant