mirror of
https://github.com/openclaw/openclaw.git
synced 2026-10-04 02:00:10 +00:00
fix(ci): retry transient npm advisory timeouts (#137716)
Co-authored-by: Dallin Romney <6581799+RomneyDa@users.noreply.github.com>
This commit is contained in:
Vincent Koc
GitHub
parent
d2f262ac22
commit
3d30b3accb
3 changed files with 231 additions and 78 deletions
|
|
@ -47,6 +47,8 @@ const AUDIT_ADVISORY_VERSION_OVERRIDES = [
|
|||
},
|
||||
];
|
||||
|
||||
class AdvisoryRequestTimeoutError extends Error {}
|
||||
|
||||
/** @typedef {{ write: (chunk: string) => boolean }} AuditOutput */
|
||||
/**
|
||||
* @typedef {object} PnpmAuditOptions
|
||||
|
|
@ -753,7 +755,9 @@ export async function withAdvisoryRequestTimeout({ label, timeoutMs, run }) {
|
|||
/** @type {Promise<never>} */
|
||||
const timeoutPromise = new Promise((_resolve, reject) => {
|
||||
timeout = setTimeout(() => {
|
||||
const error = new Error(`${label} exceeded timeout of ${resolvedTimeoutMs}ms`);
|
||||
const error = new AdvisoryRequestTimeoutError(
|
||||
`${label} exceeded timeout of ${resolvedTimeoutMs}ms`,
|
||||
);
|
||||
controller.abort(error);
|
||||
reject(error);
|
||||
}, resolvedTimeoutMs);
|
||||
|
|
@ -849,35 +853,44 @@ export async function fetchBulkAdvisories({
|
|||
timeoutMs = resolveBulkAdvisoryRequestTimeoutMs(),
|
||||
}) {
|
||||
const url = `${registryBaseUrl}${BULK_ADVISORY_PATH}`;
|
||||
return await withAdvisoryRequestTimeout({
|
||||
label: "Bulk advisory request",
|
||||
timeoutMs,
|
||||
run: async ({ signal, timeoutPromise }) => {
|
||||
const response = await fetchImpl(url, {
|
||||
method: "POST",
|
||||
headers: {
|
||||
accept: "application/json",
|
||||
"content-type": "application/json",
|
||||
},
|
||||
body: JSON.stringify(payload),
|
||||
signal,
|
||||
});
|
||||
const request = async () =>
|
||||
await withAdvisoryRequestTimeout({
|
||||
label: "Bulk advisory request",
|
||||
timeoutMs,
|
||||
run: async ({ signal, timeoutPromise }) => {
|
||||
const response = await fetchImpl(url, {
|
||||
method: "POST",
|
||||
headers: {
|
||||
accept: "application/json",
|
||||
"content-type": "application/json",
|
||||
},
|
||||
body: JSON.stringify(payload),
|
||||
signal,
|
||||
});
|
||||
|
||||
if (!response.ok) {
|
||||
const bodyText = await readBoundedBulkAdvisoryErrorText(response, undefined, {
|
||||
if (!response.ok) {
|
||||
const bodyText = await readBoundedBulkAdvisoryErrorText(response, undefined, {
|
||||
timeoutPromise,
|
||||
});
|
||||
throw new Error(
|
||||
`Bulk advisory request failed (${response.status} ${response.statusText}): ${bodyText}`,
|
||||
);
|
||||
}
|
||||
|
||||
return await readBulkAdvisoryJson(response, responseBodyMaxBytes, {
|
||||
signal,
|
||||
timeoutPromise,
|
||||
});
|
||||
throw new Error(
|
||||
`Bulk advisory request failed (${response.status} ${response.statusText}): ${bodyText}`,
|
||||
);
|
||||
}
|
||||
|
||||
return await readBulkAdvisoryJson(response, responseBodyMaxBytes, {
|
||||
signal,
|
||||
timeoutPromise,
|
||||
});
|
||||
},
|
||||
});
|
||||
},
|
||||
});
|
||||
try {
|
||||
return await request();
|
||||
} catch (error) {
|
||||
if (!(error instanceof AdvisoryRequestTimeoutError)) {
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
return await request();
|
||||
}
|
||||
|
||||
/** @param {PnpmAuditOptions} [options] */
|
||||
|
|
|
|||
|
|
@ -84,7 +84,8 @@ function publishedAdvisory(range = ">= 0.8.0, < 1.0.0") {
|
|||
|
||||
function withPublicUpstream(npmFetch: typeof fetch, advisories: unknown[] = []): typeof fetch {
|
||||
return async (input, init) => {
|
||||
const url = new URL(String(input));
|
||||
const url =
|
||||
input instanceof URL ? input : new URL(input instanceof Request ? input.url : input);
|
||||
if (url.pathname.endsWith("/advisories/bulk")) {
|
||||
return npmFetch(input, init);
|
||||
}
|
||||
|
|
@ -255,7 +256,9 @@ describe("dependency-vulnerability-gate", () => {
|
|||
blocks: false,
|
||||
},
|
||||
];
|
||||
it.each(releaseLocks.flatMap((lockfile) => npmCases.map((entry) => ({ lockfile, ...entry }))))(
|
||||
it.each(
|
||||
releaseLocks.flatMap((lockfile) => npmCases.map((entry) => Object.assign({ lockfile }, entry))),
|
||||
)(
|
||||
"$lockfile: $name stays isolated from the safe product version",
|
||||
async ({ lockfile, location, metadata, severity, title, graph, blocks }) => {
|
||||
await withLockfiles(async (rootDir) => {
|
||||
|
|
@ -351,7 +354,8 @@ describe("dependency-vulnerability-gate", () => {
|
|||
it("writes partial coverage without claiming an unavailable upstream check passed", async () => {
|
||||
await withLockfiles(async (rootDir) => {
|
||||
const fetchSpy = vi.spyOn(globalThis, "fetch").mockImplementation(async (input) => {
|
||||
const url = new URL(String(input));
|
||||
const url =
|
||||
input instanceof URL ? input : new URL(input instanceof Request ? input.url : input);
|
||||
return url.pathname.endsWith("/advisories/bulk")
|
||||
? Response.json({})
|
||||
: new Response("unavailable", { status: 503 });
|
||||
|
|
@ -457,20 +461,59 @@ describe("dependency-vulnerability-gate", () => {
|
|||
});
|
||||
});
|
||||
|
||||
it("propagates release-tool advisory transport failures", async () => {
|
||||
it("recovers a release-tool advisory timeout through the shared fetch boundary", async () => {
|
||||
await withLockfiles(async (rootDir) => {
|
||||
await writeNpmLock(rootDir, releaseLocks[0], {
|
||||
"node_modules/tool-only": { version: "1.0.0" },
|
||||
"node_modules/tool-only": { version: "1.0.0", dev: true },
|
||||
});
|
||||
vi.stubEnv("OPENCLAW_PNPM_AUDIT_BULK_TIMEOUT_MS", "5");
|
||||
let calls = 0;
|
||||
try {
|
||||
const report = await runDependencyVulnerabilityGate({
|
||||
rootDir,
|
||||
fetchImpl: withPublicUpstream(async (_url, init) => {
|
||||
if (!requestPayload(init)["tool-only"]) {
|
||||
return new Response("{}");
|
||||
}
|
||||
calls += 1;
|
||||
if (calls === 2) {
|
||||
return new Response("{}");
|
||||
}
|
||||
return await new Promise<Response>((_resolve, reject) => {
|
||||
const signal = init?.signal;
|
||||
signal?.addEventListener("abort", () => reject(signal.reason as Error), {
|
||||
once: true,
|
||||
});
|
||||
});
|
||||
}),
|
||||
});
|
||||
expect(calls).toBe(2);
|
||||
expect(report.blockers).toEqual([]);
|
||||
} finally {
|
||||
vi.unstubAllEnvs();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
it("propagates release-tool advisory HTTP failures without retrying", async () => {
|
||||
await withLockfiles(async (rootDir) => {
|
||||
await writeNpmLock(rootDir, releaseLocks[0], {
|
||||
"node_modules/tool-only": { version: "1.0.0", dev: true },
|
||||
});
|
||||
let calls = 0;
|
||||
await expect(
|
||||
runDependencyVulnerabilityGate({
|
||||
rootDir,
|
||||
fetchImpl: async (_url, init) =>
|
||||
requestPayload(init)["tool-only"]
|
||||
? new Response("fixture unavailable", { status: 503 })
|
||||
: new Response("{}"),
|
||||
fetchImpl: withPublicUpstream(async (_url, init) => {
|
||||
if (!requestPayload(init)["tool-only"]) {
|
||||
return new Response("{}");
|
||||
}
|
||||
calls += 1;
|
||||
return new Response("fixture unavailable", { status: 503 });
|
||||
}),
|
||||
}),
|
||||
).rejects.toThrow("Bulk advisory request failed (503");
|
||||
expect(calls).toBe(1);
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -3,7 +3,7 @@ import { mkdtemp, rm, writeFile } from "node:fs/promises";
|
|||
import { tmpdir } from "node:os";
|
||||
import path from "node:path";
|
||||
import { toErrorObject as toLintErrorObject } from "@openclaw/normalization-core/error-coercion";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
collectAllResolvedPackagesFromLockfile,
|
||||
collectProdResolvedPackagesFromLockfile,
|
||||
|
|
@ -313,30 +313,119 @@ snapshots:
|
|||
},
|
||||
);
|
||||
|
||||
it("aborts stalled bulk advisory requests", async () => {
|
||||
let signal: AbortSignal | undefined;
|
||||
it("retries a timed-out bulk advisory request with a fresh request lifecycle", async () => {
|
||||
const signals: AbortSignal[] = [];
|
||||
const fetchImpl = vi.fn(((_url, init) => {
|
||||
const signal = init?.signal;
|
||||
if (signal) {
|
||||
signals.push(signal);
|
||||
}
|
||||
if (signals.length === 2) {
|
||||
return Promise.resolve(new Response("{}", { status: 200 }));
|
||||
}
|
||||
return new Promise((_resolve, reject) => {
|
||||
signal?.addEventListener(
|
||||
"abort",
|
||||
() => reject(toLintErrorObject(signal.reason, "Non-Error rejection")),
|
||||
{ once: true },
|
||||
);
|
||||
});
|
||||
}) as typeof fetch);
|
||||
|
||||
await expect(
|
||||
fetchBulkAdvisories({
|
||||
payload: { axios: ["1.0.0"] },
|
||||
timeoutMs: 5,
|
||||
fetchImpl,
|
||||
}),
|
||||
).resolves.toEqual({});
|
||||
expect(fetchImpl).toHaveBeenCalledTimes(2);
|
||||
expect(signals).toHaveLength(2);
|
||||
expect(signals[0]).not.toBe(signals[1]);
|
||||
expect(signals[0]?.aborted).toBe(true);
|
||||
expect(signals[1]?.aborted).toBe(false);
|
||||
});
|
||||
|
||||
it("stops after two timed-out bulk advisory requests", async () => {
|
||||
const signals: AbortSignal[] = [];
|
||||
const fetchImpl = vi.fn(((_url, init) => {
|
||||
const signal = init?.signal;
|
||||
if (signal) {
|
||||
signals.push(signal);
|
||||
}
|
||||
return new Promise((_resolve, reject) => {
|
||||
signal?.addEventListener(
|
||||
"abort",
|
||||
() => reject(toLintErrorObject(signal.reason, "Non-Error rejection")),
|
||||
{ once: true },
|
||||
);
|
||||
});
|
||||
}) as typeof fetch);
|
||||
const request = fetchBulkAdvisories({
|
||||
payload: { axios: ["1.0.0"] },
|
||||
timeoutMs: 5,
|
||||
fetchImpl: ((_url, init) => {
|
||||
signal = init?.signal ?? undefined;
|
||||
return new Promise((_resolve, reject) => {
|
||||
signal?.addEventListener(
|
||||
"abort",
|
||||
() =>
|
||||
reject(
|
||||
toLintErrorObject(signal?.reason ?? new Error("aborted"), "Non-Error rejection"),
|
||||
),
|
||||
{
|
||||
once: true,
|
||||
},
|
||||
);
|
||||
});
|
||||
}) as typeof fetch,
|
||||
fetchImpl,
|
||||
});
|
||||
|
||||
await expect(request).rejects.toThrow(/Bulk advisory request exceeded timeout/u);
|
||||
expect(signal?.aborted).toBe(true);
|
||||
expect(fetchImpl).toHaveBeenCalledTimes(2);
|
||||
expect(signals).toHaveLength(2);
|
||||
expect(signals[0]).not.toBe(signals[1]);
|
||||
expect(signals.every((signal) => signal.aborted)).toBe(true);
|
||||
});
|
||||
|
||||
it("does not retry an untagged error with the timeout message", async () => {
|
||||
const fetchImpl = vi.fn(async () => {
|
||||
throw new Error("Bulk advisory request exceeded timeout of 5ms");
|
||||
});
|
||||
|
||||
await expect(
|
||||
fetchBulkAdvisories({
|
||||
payload: { axios: ["1.0.0"] },
|
||||
timeoutMs: 5,
|
||||
fetchImpl,
|
||||
}),
|
||||
).rejects.toThrow("Bulk advisory request exceeded timeout of 5ms");
|
||||
expect(fetchImpl).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it.each([
|
||||
{
|
||||
caseName: "HTTP failures",
|
||||
responseBodyMaxBytes: undefined,
|
||||
response: () =>
|
||||
new Response("registry failure", { status: 500, statusText: "Internal Error" }),
|
||||
expectedError: /Bulk advisory request failed \(500 Internal Error\)/u,
|
||||
},
|
||||
{
|
||||
caseName: "invalid JSON",
|
||||
responseBodyMaxBytes: undefined,
|
||||
response: () => new Response("{", { status: 200 }),
|
||||
expectedError: /JSON/u,
|
||||
},
|
||||
{
|
||||
caseName: "empty bodies",
|
||||
responseBodyMaxBytes: undefined,
|
||||
response: () => new Response("", { status: 200 }),
|
||||
expectedError: /Bulk advisory response body was empty/u,
|
||||
},
|
||||
{
|
||||
caseName: "oversized bodies",
|
||||
responseBodyMaxBytes: 4,
|
||||
response: () => new Response("12345", { status: 200 }),
|
||||
expectedError: /Bulk advisory response body exceeded 4 bytes/u,
|
||||
},
|
||||
])("does not retry $caseName", async ({ responseBodyMaxBytes, response, expectedError }) => {
|
||||
const fetchImpl = vi.fn(async () => response());
|
||||
|
||||
await expect(
|
||||
fetchBulkAdvisories({
|
||||
payload: { axios: ["1.0.0"] },
|
||||
...(responseBodyMaxBytes ? { responseBodyMaxBytes } : {}),
|
||||
fetchImpl,
|
||||
}),
|
||||
).rejects.toThrow(expectedError);
|
||||
expect(fetchImpl).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it("clamps oversized bulk advisory request timers before scheduling", async () => {
|
||||
|
|
@ -366,43 +455,49 @@ snapshots:
|
|||
});
|
||||
|
||||
it("cancels stalled successful bulk advisory response bodies on request timeout", async () => {
|
||||
let cancelled = false;
|
||||
const body = new ReadableStream({
|
||||
pull() {
|
||||
return new Promise(() => {});
|
||||
},
|
||||
cancel() {
|
||||
cancelled = true;
|
||||
},
|
||||
});
|
||||
let cancellations = 0;
|
||||
const request = fetchBulkAdvisories({
|
||||
payload: { axios: ["1.0.0"] },
|
||||
timeoutMs: 5,
|
||||
fetchImpl: async () => new Response(body, { status: 200 }),
|
||||
fetchImpl: async () =>
|
||||
new Response(
|
||||
new ReadableStream({
|
||||
pull() {
|
||||
return new Promise(() => {});
|
||||
},
|
||||
cancel() {
|
||||
cancellations += 1;
|
||||
},
|
||||
}),
|
||||
{ status: 200 },
|
||||
),
|
||||
});
|
||||
|
||||
await expect(request).rejects.toThrow(/Bulk advisory request exceeded timeout/u);
|
||||
expect(cancelled).toBe(true);
|
||||
expect(cancellations).toBe(2);
|
||||
});
|
||||
|
||||
it("cancels stalled failed bulk advisory response bodies on request timeout", async () => {
|
||||
let cancelled = false;
|
||||
const body = new ReadableStream({
|
||||
pull() {
|
||||
return new Promise(() => {});
|
||||
},
|
||||
cancel() {
|
||||
cancelled = true;
|
||||
},
|
||||
});
|
||||
let cancellations = 0;
|
||||
const request = fetchBulkAdvisories({
|
||||
payload: { axios: ["1.0.0"] },
|
||||
timeoutMs: 5,
|
||||
fetchImpl: async () => new Response(body, { status: 500, statusText: "Internal Error" }),
|
||||
fetchImpl: async () =>
|
||||
new Response(
|
||||
new ReadableStream({
|
||||
pull() {
|
||||
return new Promise(() => {});
|
||||
},
|
||||
cancel() {
|
||||
cancellations += 1;
|
||||
},
|
||||
}),
|
||||
{ status: 500, statusText: "Internal Error" },
|
||||
),
|
||||
});
|
||||
|
||||
await expect(request).rejects.toThrow(/Bulk advisory request exceeded timeout/u);
|
||||
expect(cancelled).toBe(true);
|
||||
expect(cancellations).toBe(2);
|
||||
});
|
||||
|
||||
it("bounds successful bulk advisory response bodies", async () => {
|
||||
|
|
@ -491,7 +586,9 @@ snapshots:
|
|||
const exitCode = await runPnpmAuditProd({
|
||||
rootDir: tempDir,
|
||||
fetchImpl: async (input) => {
|
||||
expect(String(input)).toMatch(/\/-\/npm\/v1\/security\/advisories\/bulk$/u);
|
||||
const url =
|
||||
input instanceof URL ? input.href : input instanceof Request ? input.url : input;
|
||||
expect(url).toMatch(/\/-\/npm\/v1\/security\/advisories\/bulk$/u);
|
||||
return new Response(
|
||||
JSON.stringify(
|
||||
blocked
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue