diff --git a/packages/cli/src/ui/utils/updateCheck.test.ts b/packages/cli/src/ui/utils/updateCheck.test.ts index 4ae6070eeae..a2b1d0d8589 100644 --- a/packages/cli/src/ui/utils/updateCheck.test.ts +++ b/packages/cli/src/ui/utils/updateCheck.test.ts @@ -5,7 +5,12 @@ */ import { vi, describe, it, expect, beforeEach } from 'vitest'; -import { checkForUpdates, checkForUpdatesDetailed } from './updateCheck.js'; +import { + checkForUpdates, + checkForUpdatesDetailed, + FETCH_TIMEOUT_MS, + UpdateCheckTimeoutError, +} from './updateCheck.js'; const getPackageJson = vi.hoisted(() => vi.fn()); vi.mock('../../utils/package.js', () => ({ @@ -254,4 +259,114 @@ describe('checkForUpdates', () => { expect(result?.update.latest).toBe('1.2.3-nightly.2'); }); }); + + describe('fetchInfo timeout (#6857)', () => { + it('returns a detailed error when fetchInfo does not resolve within FETCH_TIMEOUT_MS', async () => { + // update-notifier's fetchInfo() takes no timeout option, so an + // unreachable registry (proxy, offline, corporate mirror without + // scoped .npmrc auth) would hang the check. We race it against a + // bounded timer instead — this asserts the timer actually fires and + // surfaces a real error rather than silently reporting "up to date". + getPackageJson.mockResolvedValue({ + name: 'test-package', + version: '1.0.0', + }); + updateNotifier.mockReturnValue({ + // never resolves + fetchInfo: vi.fn().mockReturnValue(new Promise(() => {})), + }); + + const resultPromise = checkForUpdatesDetailed(); + await vi.advanceTimersByTimeAsync(FETCH_TIMEOUT_MS + 1); + const result = await resultPromise; + + expect(result.status).toBe('error'); + if (result.status === 'error') { + expect(result.error).toBeInstanceOf(UpdateCheckTimeoutError); + expect(result.error.message).toContain(`${FETCH_TIMEOUT_MS}ms`); + // Non-nightly path only queries the `latest` dist-tag; the message + // must name it so oncall can tell which registry endpoint stalled. + expect(result.error.message).toContain('for latest'); + expect(result.currentVersion).toBe('1.0.0'); + } + }); + + it('still resolves the update path when fetchInfo returns before the timeout', async () => { + // Guards against the timer accidentally firing on a healthy fast fetch — + // if it did, every /update call would silently drop back to error. + getPackageJson.mockResolvedValue({ + name: 'test-package', + version: '1.0.0', + }); + updateNotifier.mockReturnValue({ + fetchInfo: vi + .fn() + .mockResolvedValue({ current: '1.0.0', latest: '1.1.0' }), + }); + + const result = await checkForUpdatesDetailed(); + + expect(result.status).toBe('update'); + if (result.status === 'update') { + expect(result.info.update.latest).toBe('1.1.0'); + } + }); + + it('surfaces a timeout when only the nightly dist-tag stalls', async () => { + // The nightly path fires `latest` and `nightly` fetches concurrently via + // Promise.all — if the timer wiring is wrong (e.g. only the outer race + // has one, or the reject reaches Promise.all and Promise.all doesn't + // propagate), a single stalled fetch would let /update silently degrade. + // Assert Promise.all propagates the timeout AND names the exact dist-tag + // that stalled so oncall reading logs can point at the endpoint. + getPackageJson.mockResolvedValue({ + name: 'test-package', + version: '1.0.0-nightly.1', + }); + updateNotifier.mockImplementation(({ distTag }) => ({ + fetchInfo: () => + distTag === 'nightly' + ? new Promise(() => {}) // never resolves + : Promise.resolve({ + current: '1.0.0-nightly.1', + latest: '1.0.0', + }), + })); + + const resultPromise = checkForUpdatesDetailed(); + await vi.advanceTimersByTimeAsync(FETCH_TIMEOUT_MS + 1); + const result = await resultPromise; + + expect(result.status).toBe('error'); + if (result.status === 'error') { + expect(result.error).toBeInstanceOf(UpdateCheckTimeoutError); + expect(result.error.message).toContain('for nightly'); + expect(result.currentVersion).toBe('1.0.0-nightly.1'); + } + }); + + it('surfaces a timeout when both nightly dist-tags stall', async () => { + // Full outage / offline network — both fetches hang, both timers fire. + // The first rejection Promise.all sees wins; assert only that we get a + // typed UpdateCheckTimeoutError for one of the two dist-tags (either is + // a valid symptom of the same failure). + getPackageJson.mockResolvedValue({ + name: 'test-package', + version: '1.0.0-nightly.1', + }); + updateNotifier.mockImplementation(() => ({ + fetchInfo: () => new Promise(() => {}), + })); + + const resultPromise = checkForUpdatesDetailed(); + await vi.advanceTimersByTimeAsync(FETCH_TIMEOUT_MS + 1); + const result = await resultPromise; + + expect(result.status).toBe('error'); + if (result.status === 'error') { + expect(result.error).toBeInstanceOf(UpdateCheckTimeoutError); + expect(result.error.message).toMatch(/for (nightly|latest)/); + } + }); + }); }); diff --git a/packages/cli/src/ui/utils/updateCheck.ts b/packages/cli/src/ui/utils/updateCheck.ts index 748e9fdb22d..3cea93cc731 100644 --- a/packages/cli/src/ui/utils/updateCheck.ts +++ b/packages/cli/src/ui/utils/updateCheck.ts @@ -15,6 +15,50 @@ const debugLogger = createDebugLogger('UPDATE_CHECK'); export const FETCH_TIMEOUT_MS = 2000; +/** + * Sentinel error thrown when `fetchInfo()` does not resolve within + * `FETCH_TIMEOUT_MS`. `update-notifier`'s `fetchInfo()` does not accept a + * timeout option, so slow / unreachable registries (corporate proxies, offline + * networks, DNS failures) would otherwise hang the check indefinitely or fall + * through to a stale configstore cache. Race the call against a bounded timer + * and surface a real error so `/update` can report "check failed" instead of + * silently returning "up to date". The `distTag` is carried on the message so + * an oncall reading logs can tell which registry endpoint stalled — the + * nightly path fires two concurrent fetches, and only one of them may be + * blocked (e.g. a corporate proxy that lets `nightly` through but not + * `latest`). Related: #6857. + */ +export class UpdateCheckTimeoutError extends Error { + readonly distTag?: string; + constructor(timeoutMs: number, distTag?: string) { + const suffix = distTag ? ` for ${distTag}` : ''; + super(`update-notifier fetchInfo timed out after ${timeoutMs}ms${suffix}`); + this.name = 'UpdateCheckTimeoutError'; + this.distTag = distTag; + } +} + +async function fetchInfoWithTimeout( + notifier: { fetchInfo(): UpdateInfo | Promise }, + timeoutMs: number, + distTag?: string, +): Promise { + let timer: ReturnType | undefined; + try { + return await Promise.race([ + Promise.resolve(notifier.fetchInfo()), + new Promise((_, reject) => { + timer = setTimeout( + () => reject(new UpdateCheckTimeoutError(timeoutMs, distTag)), + timeoutMs, + ); + }), + ]); + } finally { + if (timer !== undefined) clearTimeout(timer); + } +} + export interface UpdateObject { message: string; update: UpdateInfo; @@ -77,10 +121,22 @@ export async function checkForUpdatesDetailed(): Promise { if (isNightly) { const [nightlyUpdateInfo, latestUpdateInfo] = await Promise.all([ - createNotifier('nightly').fetchInfo(), - createNotifier('latest').fetchInfo(), + fetchInfoWithTimeout( + createNotifier('nightly'), + FETCH_TIMEOUT_MS, + 'nightly', + ), + fetchInfoWithTimeout( + createNotifier('latest'), + FETCH_TIMEOUT_MS, + 'latest', + ), ]); + debugLogger.debug( + `fetchInfo returned nightly=${JSON.stringify(nightlyUpdateInfo)} latest=${JSON.stringify(latestUpdateInfo)} for current=${version}`, + ); + const bestUpdate = getBestAvailableUpdate( nightlyUpdateInfo, latestUpdateInfo, @@ -99,7 +155,15 @@ export async function checkForUpdatesDetailed(): Promise { }; } } else { - const updateInfo = await createNotifier('latest').fetchInfo(); + const updateInfo = await fetchInfoWithTimeout( + createNotifier('latest'), + FETCH_TIMEOUT_MS, + 'latest', + ); + + debugLogger.debug( + `fetchInfo returned ${JSON.stringify(updateInfo)} for current=${version}`, + ); if (updateInfo && semver.gt(updateInfo.latest, version)) { return {