mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-08-11 01:36:35 +00:00
* fix(cli): apply FETCH_TIMEOUT_MS to /update version check and log fetchInfo results (#6857) The FETCH_TIMEOUT_MS = 2000 constant in updateCheck.ts was defined but never wired up. update-notifier's fetchInfo() takes no timeout option, so slow/unreachable registries (corporate proxies, offline networks, scoped .npmrc mirrors without auth) would either hang the check or fall back to whatever update-notifier internally decides — sometimes a stale configstore cache, reported by users as '/update reports up-to-date on 0.19.9 when 0.19.10 is available.' Race fetchInfo() against a bounded timer via Promise.race and surface a new UpdateCheckTimeoutError when it fires, so '/update' returns the existing 'error' status instead of silently reporting 'up to date.' Also log the fetchInfo return value under the UPDATE_CHECK debug tag so the next round of reports can distinguish 'registry returned the wrong version' from 'we compared incorrectly' without adding more speculation. Refs #6857 * fix(cli): carry dist-tag on UpdateCheckTimeoutError and cover nightly timeout paths Address bot review on #6887: - `UpdateCheckTimeoutError` now takes an optional `distTag` argument that is threaded through by `fetchInfoWithTimeout` and appended to the message. The nightly path fires `nightly` and `latest` fetches concurrently via `Promise.all`; without a dist-tag on the error, an oncall reading logs cannot tell which registry endpoint stalled (e.g. a corporate proxy that lets `nightly` through but blocks `latest`). The tag also lands on the error instance as a public `distTag` field so callers can branch on it programmatically. - Add two regression tests for the nightly `Promise.all` timeout path: a single stalled dist-tag (asserts Promise.all propagates the timeout and names the exact tag) and both stalled (full outage — asserts we still surface a typed error with a valid tag). The non-nightly test now also asserts the message contains `for latest`.
This commit is contained in:
parent
2132a6142b
commit
0957e18ea2
2 changed files with 183 additions and 4 deletions
|
|
@ -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)/);
|
||||
}
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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<UpdateInfo> },
|
||||
timeoutMs: number,
|
||||
distTag?: string,
|
||||
): Promise<UpdateInfo> {
|
||||
let timer: ReturnType<typeof setTimeout> | undefined;
|
||||
try {
|
||||
return await Promise.race([
|
||||
Promise.resolve(notifier.fetchInfo()),
|
||||
new Promise<never>((_, 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<UpdateCheckResult> {
|
|||
|
||||
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<UpdateCheckResult> {
|
|||
};
|
||||
}
|
||||
} 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 {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue