fetch() without a timeout is undetected — including the analyzer's own 10 un-timed call sites

[serge-ivo]

Rewritten 2026-08-11 against main @ d1c5314. The original body proposed a reliability/operations category with six checks. Verifying them against the code: only the first — "fetch/HTTP client calls require timeout or abort support" — is statically detectable without configuration that does not exist yet. The other five all reduce to "does this project follow its own convention?", and VCQA has no way for a project to declare that convention (see #30). A category with one implementable member is not a category — so this issue is now scoped to that one rule, added to an existing check. The rest are preserved under Blocked on per-project convention config.

Problem

A fetch() with no timeout does not fail — it hangs. In a Worker it burns the CPU/wall budget until the request is killed; in a Node service it holds a connection and a promise forever; in a browser it leaves a spinner up permanently. It is one of the few production-readiness defects that is (a) genuinely common, (b) genuinely damaging, and (c) visible from the call site alone with no project context.

VCQA does not detect it anywhere today.

We ship the bug ourselves. cli/src/ has 11 production fetch() call sites; exactly one passes an abort signal:

// cli/src/cli.ts:585 — the only one that does it right
const res = await fetch("https://registry.npmjs.org/@vibecodeqa/cli/latest", { signal: AbortSignal.timeout(3000) });

The other ten have no signal and no timeout, including every Pro-check network call, which runs inside a scan:

• cli/src/runners/dead-patterns.ts:322
• cli/src/runners/file-cohesion.ts:204
• cli/src/runners/design-consistency.ts:115
• cli/src/runners/test-audit.ts:298
• cli/src/ai-fix.ts:194, :229
• cli/src/cli.ts:475
• cli/src/pr-comment.ts:170, :183, :191

A hung Pro endpoint hangs the scan. VCQA scanning VCQA should report ten findings and currently reports zero.

Where the gap is

The nearest existing rule is in error-handling, and it is a different rule. cli/src/runners/error-handling.ts:79-93 matches fetch only to detect a floating promise:

/^\s*(?:fetch|axios|new Promise)\s*\(/.test(line) &&
!line.includes("await") &&
...
!line.includes(".catch") &&

That fires when the result is discarded. A correctly-awaited, correctly-caught fetch with no timeout passes it cleanly — which is exactly the shape of all ten call sites above. Nothing in cli/src/runners/ mentions AbortSignal, AbortController, retry, backoff, or idempotency; the only timeout hits are exec.ts:111/:153 (the analyzer's own execSync wrapper) and a magic-number heuristic at standards.ts:48. Verified by grep across all runners.

What to do

1. Add the rule to error-handling — do not create a new check or category.
error-handling already owns "async call that will misbehave at runtime", already walks every source file line-by-line, and is stack-blind with weight 3. Adding a rule there costs one CHECK_META edit (none) and zero downstream taxonomy churn. Proposed rule id: fetch-no-timeout, severity: "warning".

Detection, conservative on purpose:

• Match a fetch( call whose second argument is an object literal that contains no signal key, or which has no second argument at all.
• Treat as satisfied: signal: anywhere in the options object (AbortSignal.timeout(...), controller.signal, a variable, a spread ...opts — a spread means we cannot see it, so do not flag).
• Do not flag calls where the options object is a bare identifier (fetch(url, opts)) — the signal may be inside it.
• Do not attempt this for axios, got, ky, undici.request or node-fetch in the first pass. Each has its own timeout convention and some are configured globally at client construction; that is the config-dependent case this issue is deliberately not doing.

2. If step 1 lands clean, consider one follow-up rule in the same check. while/for loops containing an await on a network call and no bounded counter — a genuinely unbounded retry. Only after fetch-no-timeout has run against real repos without a false-positive complaint.

Alternatives considered and rejected

• A new reliability check with its own CHECK_META entry. Rejected: a check with one rule cannot be scored meaningfully (a project with one un-timed fetch and one file would score 0), and it costs a weight rebalance (cli/src/check-meta.test.ts:57 asserts the sum is 100) plus five hardcoded-taxonomy edits. Revisit only if the blocked rules below become implementable.
• A new Reliability category as the original body proposed. Rejected outright: cli/src/check-meta.test.ts:51 hardcodes the seven valid category names as an allowlist, and cli/src/report/html.ts:32, app/src/components/ReportViewer.tsx:54, cli/CLAUDE.md:163 each hardcode their own copy. One rule does not justify a fifth taxonomy edit. See CHECK_META is not the only taxonomy: four hardcoded copies leave 12 checks on no category page #61.
• Putting it in performance. Rejected: a missing timeout is a correctness/liveness bug, not a throughput one, and performance is already a mixed bag (barrel imports, heavy deps, dead code).
• Full dataflow analysis to resolve opts variables. Rejected for this scope: no runner does dataflow today, and the conservative "skip what you cannot see" rule above already catches the ten real call sites in our own tree.
• Configurable HTTP-client list up front. Rejected: that is Analyzer settings schema: pass per-analyzer config through scans and record effective settings #30's surface and it does not exist. Ship the zero-config fetch rule now.

Acceptance criteria

• fetch(url) and fetch(url, { method: "POST" }) each produce one fetch-no-timeout warning.
• fetch(url, { signal: AbortSignal.timeout(3000) }), fetch(url, { signal: controller.signal }), fetch(url, { ...base, signal }) and fetch(url, opts) produce no finding.
• A fetch inside a test file produces no finding (tests are judged by their own standards — the runner already consumes FileInventory; use its classification rather than a path regex).
• Running the CLI against its own repo reports the ten call sites listed above and not cli/src/cli.ts:585. This is the acceptance test — a fixture is not enough, the dogfood run is the proof.
• Existing error-handling fixtures still pass, in particular the floating-promise cases at cli/src/runners/error-handling.test.ts — one un-awaited fetch with no timeout must not double-report as both floating-promise and fetch-no-timeout. Pick one; recommend suppressing fetch-no-timeout when floating-promise already fired on the same line (#13 set the precedent for de-duplicating findings).
• Fix the ten call sites in this repo in the same change, or open a follow-up issue for them. Shipping a rule we fail is worse than not shipping it.

Blocked on per-project convention config

The remaining five proposals from the original body are recorded here. Each depends on knowing a convention the project has never told us. #30 (per-analyzer settings through AnalyzerContext, recorded in the report) is the prerequisite for all of them; .vcqa.json currently supports only enabled and ignore per check.

Proposal	Why it is not decidable today	Config it would need
Retry loops use bounded retries and backoff	"Retry loop" is not syntactically distinct from any other loop, and most real retries come from a library (p-retry, undici's built-in, @aws-sdk middleware) where the bound is a config value, not a loop.	Declared retry helper(s) and their bound/backoff argument names.
Mutating external calls have idempotency or dedupe markers	Requires knowing which calls mutate and what this project's idempotency marker is called (Idempotency-Key, a request id, a dedupe column).	Declared idempotency header/field name and the list of mutating client modules.
Queue/job handlers have retry/dead-letter or failure handling	Handler registration is framework- and platform-specific (Cloudflare Queues, BullMQ, SQS consumers, cron). Nothing marks a function as a job handler.	Declared queue library and handler entry points.
Request handlers propagate correlation/request IDs	The original body already concedes this only applies "where project conventions exist". There is no convention to read.	Declared logger helper and correlation field name.
Health/readiness endpoints exist when deploy metadata is detected	Detectable that a Dockerfile or wrangler config exists (container-health already does), but not whether a given route is the health endpoint, nor whether the platform requires one.	Declared health route path, or platform metadata that names it.

If #30 lands and a real project supplies these settings, re-open the category question — with the settings in hand, three or four of these become tractable and a reliability check would have enough members to be worth a CHECK_META entry.

Constraints the implementer should know

• Stack-gating rule (cli/CLAUDE.md:128-137): error-handling is stack-blind. A stack.framework === ... branch added to it is a rejected diff. The fetch rule must hold for React, Node, Workers and plain JS alike — which it does.
• Trunk-based (cli/CLAUDE.md:21-45): commit to main, no PR.
• Tests are a separate class. Do not flag fetch in test files; use the FileInventory classification the runner already receives, not a bespoke path check (#70 made this the rule).
• Fixtures must not name downstream repos. cli/src/downstream-fixture-guard.test.ts:23-40 fails the build on downstream repo-specific strings in production scanner code. This issue originated from a PAGS observation; keep PAGS paths out of fixtures.
• #73 (closed) still governs: do not close this until the rule is published to npm and validated through the published CLI.
• Severity discipline: #12 established that only build-breaking findings are error. A missing timeout is a warning.

Downstream — what else must move

Scoped as written (a rule inside error-handling), this changes no check count and needs no doc updates beyond the check's own description in schema/src/check-meta.ts:97-109 and the error-handling section of vibecodeqa/docs/docs/checks.md.

If the maintainer overrules the scoping and creates a separate check, the registry goes 38 → 39 and CHECK_META 37 → 38, and the following must move with it: cli/CLAUDE.md:155 and :163-170; vibecodeqa/docs/docs/checks.md (inventory table and the "verified against 0.54.4" callout at line 9); the 11 marketing files still claiming 34 (vibecodeqa/index.html, scan.html, skills.html, tools.html, stacks/flutter.html, compare/*.html, docs/zensical.toml); app/src/App.tsx:122; mcp/README.md:58; ops/ARCHITECTURE.md:140. The full count-drift inventory and its fix live in #61.

Open questions for the maintainer

1. Is error-handling the right home, or is this the seed of a real reliability check? This issue argues for error-handling on cost grounds. If a reliability check is wanted anyway as a placeholder for post-Analyzer settings schema: pass per-analyzer config through scans and record effective settings #30 work, say so and the scoping changes.
2. Should the ten in-repo call sites be fixed in this change or tracked separately? They are real bugs in the analyzer (a hung Pro endpoint hangs a scan), independent of whether the rule ships.

Related

• Analyzer settings schema: pass per-analyzer config through scans and record effective settings #30 — per-analyzer settings; blocks all five deferred proposals.
• CHECK_META is not the only taxonomy: four hardcoded copies leave 12 checks on no category page #61 — taxonomy; owns the category question and the check-count drift.
• Severity inflation: style/lint rules emitted as error (only 1 of 79 'errors' was build-breaking) #12 (closed) — severity inflation; sets the warning-vs-error bar.
• Duplicate findings: circular dependencies double-counted across 'architecture' and 'context' checks #13 (closed) — duplicate findings across checks; sets the de-duplication precedent.
• Introduce FileInventory: classify scan files once and require runners to consume it #70 (closed) — FileInventory; the correct way to classify test files.