Skip to content

test(audit-fixes): de-flake TimeoutError test (closes #120)#122

Merged
productdevbook merged 1 commit into
mainfrom
fix/120-audit-fixes-timeout-flake
May 3, 2026
Merged

test(audit-fixes): de-flake TimeoutError test (closes #120)#122
productdevbook merged 1 commit into
mainfrom
fix/120-audit-fixes-timeout-flake

Conversation

@productdevbook
Copy link
Copy Markdown
Owner

Summary

Fixes the long-standing flake in test/audit-fixes.test.ts > 'TimeoutError carries the configured timeout' > 'error.timeout reflects the option, not 0' (issue #120, observed across cycles 5/6/7/8).

Root cause

The test wired a driver whose request returned a Promise that only rejected once req.signal aborted. The signal aborted via the misina-built AbortSignal.timeout(25). On a saturated runner (full suite, threads pool, ~118 parallel test files) the 25 ms native timer can drift well past the inner test's vitest budget, plus the per-attempt overhead of waiting for the abort listener to fire and propagate. Locally the test took 27-29 ms — only ~3 ms of headroom over the 25 ms timer, so any scheduling jitter on CI under load tipped it past whatever inner deadline Vitest was using and produced the intermittent failure.

Fix approach

The contract under test is purely the shape of the resulting TimeoutError (err.timeout === 25, err.message includes "25ms") — not the wall-clock behavior of AbortSignal.timeout. The mapTransportError path in src/misina.ts triggers the configured-timeout branch whenever cause.name === "TimeoutError", so the test now drives that path synchronously by having the driver reject with new DOMException("...", "TimeoutError"). No real timer fires, no wall-clock dependency, runtime drops from ~28 ms to ~2 ms.

Rationale vs alternatives:

  • vi.useFakeTimers() — would not help here. AbortSignal.timeout is implemented by the host (Node), not via the timer functions Vitest replaces, so faking timers does not advance it.
  • Bumping to timeout: 100 + 5 s test budget — works but keeps the wall-clock dependency and inflates test duration without reason.
  • it.retry(2) — masks the symptom, explicitly discouraged by the issue.

Verification

  • Ran pnpm exec vitest run --pool=threads --maxConcurrency=20 5 consecutive times: 5/5 clean, 932/932 tests passing each run.
  • Ran the full pnpm test (lint + typecheck + vitest): green.
  • The test still hits the same mapTransportError branch and asserts the same observable contract.

Closes #120.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 3, 2026

Bench results

⚠️ 1 misina row(s) >5% slower than base.

benchmark base avg head avg Δ
createMisina cold start :: createMisina({ use: [bearer] }) 520.4 ns 548.5 ns 🔴 +5.4%
createMisina cold start :: createMisina() 1.30 µs 1.36 µs · +4.6%
POST JSON body :: misina 488.29 µs 491.52 µs · +0.7%
retry on 503 → 200 :: misina — retry 1× then 200 1.74 ms 1.74 ms · +0.2%
plugin overhead (no plugins / 1 hook plugin / 3 plugins inc. wrapping) :: misina — bearer 271.65 µs 272.29 µs · +0.2%
steady-state GET (200 OK, JSON parse) :: axios 302.78 µs 302.55 µs · -0.1%
POST JSON body :: ofetch 324.01 µs 322.51 µs · -0.5%
hooks overhead (no hooks vs 5 hooks) :: misina — 5 hooks 278.71 µs 277.05 µs · -0.6%
plugin overhead (no plugins / 1 hook plugin / 3 plugins inc. wrapping) :: misina — no plugins 267.50 µs 265.83 µs · -0.6%
POST JSON body :: axios 311.24 µs 309.14 µs · -0.7%
plugin overhead (no plugins / 1 hook plugin / 3 plugins inc. wrapping) :: misina — bearer + tracing + breaker 301.37 µs 298.16 µs · -1.1%
hooks overhead (no hooks vs 5 hooks) :: misina — no hooks 279.12 µs 276.12 µs · -1.1%
POST JSON body :: native fetch 342.68 µs 338.77 µs · -1.1%
steady-state GET (200 OK, JSON parse) :: misina 294.43 µs 290.39 µs · -1.4%
steady-state GET (200 OK, JSON parse) :: ofetch 266.84 µs 262.12 µs · -1.8%
POST JSON body :: ky 450.24 µs 442.21 µs · -1.8%
createMisina cold start :: createMisina({ use: [bearer, tracing, breaker] }) 2.02 µs 1.96 µs · -2.6%
steady-state GET (200 OK, JSON parse) :: ky 295.40 µs 284.76 µs · -3.6%
steady-state GET (200 OK, JSON parse) :: native fetch 332.57 µs 320.11 µs · -3.7%

Local Node HTTP server, Apple-class GitHub runner. Numbers fluctuate ±2-3% from runner heat alone — only sustained >5% deltas are signal.

@productdevbook
Copy link
Copy Markdown
Owner Author

Verified: synthetic DOMException("...", "TimeoutError") hits the same mapTransportError branch (cause.name === "TimeoutError" at src/misina.ts:966) that native AbortSignal.timeout triggers — test still exercises the production TimeoutError-mapping path and asserts the same observable contract (timeout=25, message contains 25ms). 5/5 isolated runs green, full suite 932/932. Note: test/abort-reason.test.ts > our own timeout fires TimeoutError shows the same wall-clock flake pattern under load — pre-existing (not in this diff), worth a follow-up with the same approach. Merging.

@productdevbook productdevbook merged commit 34a8cc2 into main May 3, 2026
5 checks passed
@productdevbook productdevbook deleted the fix/120-audit-fixes-timeout-flake branch May 3, 2026 09:44
productdevbook added a commit that referenced this pull request May 3, 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.

Flaky test: audit-fixes 'TimeoutError carries the configured timeout' fails ~1/N runs

1 participant