Skip to content

Commit 8fece22

Browse files
critesjoshclaude
andcommitted
fix(version-self-check): clear timer in finally + unref to avoid event-loop pin
Codex review feedback on PR #21. The previous implementation only cleared the abort timer on the success path — when `fetchImpl` rejected (network error, malformed body), the catch block returned null but the timer was left running until it fired. In a long-lived MCP server that's invisible (the timer just expires harmlessly seconds later), but in short-lived processes / tests it would prevent the event loop from exiting cleanly. Two-line fix: - `timer.unref()` (Node-only, optional-chained for portability) so the timer alone never keeps the loop alive. - `clearTimeout(timer)` moved into a `finally` block so it runs on both success and exception paths. Belt-and-braces by design: either fix alone is sufficient, but together they leave no path for a stray timer. 19 version-self-check tests still pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent fabe48e commit 8fece22

1 file changed

Lines changed: 14 additions & 3 deletions

File tree

src/utils/version-self-check.ts

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,18 +49,29 @@ export async function fetchLatestNpmVersion(
4949
timeoutMs: number = 2000,
5050
fetchImpl: typeof fetch = fetch
5151
): Promise<string | null> {
52+
const ctl = new AbortController();
53+
const timer = setTimeout(() => ctl.abort(), timeoutMs);
54+
// `unref` (Node-only) prevents this timer from keeping the event
55+
// loop alive on its own. Critical for short-lived processes and
56+
// tests where a forgotten timer would block exit. Optional-chained
57+
// because `setTimeout` in browser-shaped environments returns a
58+
// primitive number with no `unref` — the optional call is safe.
59+
(timer as unknown as { unref?: () => void }).unref?.();
5260
try {
53-
const ctl = new AbortController();
54-
const timer = setTimeout(() => ctl.abort(), timeoutMs);
5561
const resp = await fetchImpl(NPM_REGISTRY_URL, { signal: ctl.signal });
56-
clearTimeout(timer);
5762
if (!resp.ok) return null;
5863
const data = await resp.json();
5964
if (!data || typeof data !== "object") return null;
6065
const v = (data as Record<string, unknown>).version;
6166
return typeof v === "string" ? v : null;
6267
} catch {
6368
return null;
69+
} finally {
70+
// Always clear: the previous implementation only cleared on the
71+
// success path, leaking the timer when `fetchImpl` rejected
72+
// (network error, CORS, malformed body) before the timeout
73+
// fired. Combined with `unref` above this is belt-and-braces.
74+
clearTimeout(timer);
6475
}
6576
}
6677

0 commit comments

Comments
 (0)