From 9094e181b95d4792c9376f7a9a9b3e89e4e9b37c Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 21 Sep 2026 20:37:42 -0700 Subject: [PATCH] fix(update): include npm errors in failure reports (#155337) * fix(update): include npm errors in failure reports Record bounded, sanitized npm error facts before command tails and preserve them alongside permission guidance. Render codes, excerpts, and next steps without changing the diagnostic schema or retention policy. * test(update): require retained npm facts in CLI outcomes --- docs/cli/update.md | 12 ++ .../update-cli/update-command-package.test.ts | 12 +- src/infra/package-update-manager-preflight.ts | 1 + src/infra/update-failure-report-prepare.ts | 36 ++--- src/infra/update-npm-failure.test.ts | 131 ++++++++++++++++++ src/infra/update-npm-failure.ts | 103 ++++++++++++++ src/infra/update-runner-command.ts | 10 ++ 7 files changed, 288 insertions(+), 17 deletions(-) create mode 100644 src/infra/update-npm-failure.test.ts create mode 100644 src/infra/update-npm-failure.ts diff --git a/docs/cli/update.md b/docs/cli/update.md index 3964d8e66f9e..769c95f1aa57 100644 --- a/docs/cli/update.md +++ b/docs/cli/update.md @@ -141,6 +141,18 @@ that resolution step visible. Older updater processes cannot recover details the already discarded; a report generated by newer code only includes facts that were recorded by the updater that handled the failure. +Npm install failures record a recognized error code (or `unknown`) and a sanitized +excerpt of `npm ERR!` / `npm error` lines in update history and the reviewed report. +The existing history format retains at most five lines of 200 UTF-8 bytes each; +permission guidance reserves one of those entries. Home paths, credentials, and +authenticated registry URLs are redacted before the excerpt is recorded. +For `EACCES` or `EPERM`, check `npm prefix -g` and run the update as the installing +account with write access to that prefix. For `ENOSPC`, free space on the prefix +and npm cache volumes. For `E404` or `ETARGET`, check the configured registry and +the requested version or tag. The generated report includes the applicable next +step. An already-running older updater cannot gain this diagnostic capture from +its candidate package. + ## Automation and SSH For an authorized update on another host, use the target installation's owning diff --git a/src/cli/update-cli/update-command-package.test.ts b/src/cli/update-cli/update-command-package.test.ts index afa3b8f38071..da00eba5a56a 100644 --- a/src/cli/update-cli/update-command-package.test.ts +++ b/src/cli/update-cli/update-command-package.test.ts @@ -110,7 +110,17 @@ it.each(["guidance", "staging"])( installTarget: target, }; const permissionFacts = [ - expect.objectContaining({ code: "global-install-permission-denied" }), + { + check: "package-install", + code: "global-install-permission-denied", + message: "Package update cannot write [redacted-path]", + }, + ...[ + "npm error code EACCES", + "npm error syscall rename", + "npm error path [redacted-path]", + "npm error EACCES: permission denied, rename [redacted-path]", + ].map((message) => ({ check: "npm", code: "EACCES", message })), ]; if (consumer === "staging") { await expect(stagePackageInstallUpdate(params)).rejects.toMatchObject({ diff --git a/src/infra/package-update-manager-preflight.ts b/src/infra/package-update-manager-preflight.ts index d9081d9338d7..c9987294a3b1 100644 --- a/src/infra/package-update-manager-preflight.ts +++ b/src/infra/package-update-manager-preflight.ts @@ -225,6 +225,7 @@ async function permissionFailure( { check: "package-install", code: UPDATE_GLOBAL_PERMISSION_REASON, message }, env, ), + ...(step.failureFacts ?? []).slice(0, 4), ], }; } diff --git a/src/infra/update-failure-report-prepare.ts b/src/infra/update-failure-report-prepare.ts index 454bad68d4eb..b7357bd79e64 100644 --- a/src/infra/update-failure-report-prepare.ts +++ b/src/infra/update-failure-report-prepare.ts @@ -25,6 +25,7 @@ import { isPublicUpdateFailureCode, projectPublicUpdateFailureIdentifiers, } from "./update-failure-public-identifiers.js"; +import { formatNpmFailureFacts } from "./update-npm-failure.js"; import { updatePreflightDetailMessage } from "./update-preflight-details.js"; import { LEGACY_UPDATE_RUN_ADVISORY, @@ -300,24 +301,27 @@ async function renderBoundedDiagnostics( ? diagnostic : `${exit} (${diagnostic})`; diagnostics.push(`Failed phase ${phase}: ${detail}${termination}`); + diagnostics.push(...formatNpmFailureFacts(step.failureFacts ?? [], context)); diagnostics.push( ...(await Promise.all( - normalizeUpdateFailureFacts(step.failureFacts ?? [], context.env).map(async (fact) => - formatUpdateFailureFact({ - ...(await projectPublicUpdateFailureIdentifiers(fact)), - ...(fact.location ? { location: fact.location } : {}), - ...(fact.affectedKey ? { affectedKey: sanitizeFactConfigKey(fact.affectedKey) } : {}), - ...(fact.message - ? { - message: - updatePreflightDetailMessage(fact.code) ?? - (fact.errorName - ? redactSupportDiagnosticLine(fact.message, context) - : redactPublicSupportDiagnosticLine(fact.message, context)), - } - : {}), - }), - ), + normalizeUpdateFailureFacts(step.failureFacts ?? [], context.env) + .filter((fact) => fact.check !== "npm") + .map(async (fact) => + formatUpdateFailureFact({ + ...(await projectPublicUpdateFailureIdentifiers(fact)), + ...(fact.location ? { location: fact.location } : {}), + ...(fact.affectedKey ? { affectedKey: sanitizeFactConfigKey(fact.affectedKey) } : {}), + ...(fact.message + ? { + message: + updatePreflightDetailMessage(fact.code) ?? + (fact.errorName + ? redactSupportDiagnosticLine(fact.message, context) + : redactPublicSupportDiagnosticLine(fact.message, context)), + } + : {}), + }), + ), )), ); } diff --git a/src/infra/update-npm-failure.test.ts b/src/infra/update-npm-failure.test.ts new file mode 100644 index 000000000000..aa41bf18c8b4 --- /dev/null +++ b/src/infra/update-npm-failure.test.ts @@ -0,0 +1,131 @@ +import { describe, expect, it } from "vitest"; +import { runCommandWithTimeout } from "../process/exec.js"; +import { classifyPackageUpdatePermissionFailure } from "./package-update-manager-preflight.js"; +import { prepareUpdateFailureReport } from "./update-failure-report-prepare.js"; +import { updateRunStepsFromResultStep } from "./update-run-step.js"; +import { runStep } from "./update-runner-command.js"; + +const context = { env: { HOME: "/home/example" }, stateDir: "/npm-report-state" }; + +describe("npm install failure reports", () => { + it.each(["EACCES", undefined])( + "retains spawned npm diagnostics in direct and recorded reports (code=%s)", + async (code) => { + const token = `npm_${"synthetic".repeat(5)}`; + const stderr = [ + "npm warn unrelated warning", + ...(code ? [`npm ERR! code ${code}`] : []), + "npm ERR! install failed while preparing package", + "npm ERR! path /home/example/private directory/package", + `npm ERR! token=${token}`, + "npm ERR! registry https://example-user:synthetic-password@registry.example.test/pkg", + "npm ERR! omitted line", + ].join("\n"); + const step = await runStep({ + name: "package-install", + argv: [ + process.execPath, + "-e", + "process.stderr.write(process.argv[1]); process.exitCode = 1", + stderr, + ], + cwd: process.cwd(), + env: context.env, + runCommand: runCommandWithTimeout, + stepIndex: 0, + totalSteps: 1, + }); + expect(step.exitCode).toBe(1); + expect(step.failureFacts?.[0]).toMatchObject({ check: "npm", code: code ?? "unknown" }); + for (const recorded of [false, true]) { + const report = await prepareUpdateFailureReport( + { + attemptId: "npm-fixture", + result: { + mode: "npm", + status: "error", + reason: "global-install-failed", + durationMs: 1, + steps: recorded ? [] : [step], + }, + ...(recorded + ? { recordedRun: { runId: "npm-fixture", steps: updateRunStepsFromResultStep(step) } } + : {}), + }, + context, + ); + expect(report.body).toContain(`npm failure code: ${code ?? "unknown"}`); + expect(report.body).toContain("npm ERR! install failed while preparing package"); + expect(report.body).toContain("[redacted-path]"); + for (const privateText of [ + token, + "/home/example", + "private directory", + "example-user", + "synthetic-password", + "unrelated warning", + ]) { + expect(report.body).not.toContain(privateText); + expect(JSON.stringify(step.failureFacts)).not.toContain(privateText); + } + if (code) { + expect(report.body).toContain("Next step: Check the npm global prefix"); + } + } + if (code) { + const classified = await classifyPackageUpdatePermissionFailure( + step, + { manager: "npm", command: "npm", globalRoot: process.cwd(), packageRoot: process.cwd() }, + context.env, + ); + const report = await prepareUpdateFailureReport( + { + attemptId: "npm-permission", + result: { mode: "npm", status: "error", durationMs: 1, steps: [classified] }, + }, + context, + ); + expect(report.body).toContain("npm failure code: EACCES"); + expect(report.body).toContain("npm ERR! install failed while preparing package"); + } + }, + ); + + it.each([ + ["ENOSPC", "Free disk space"], + ["E404", "Check the configured npm registry"], + ["ETARGET", "Check the configured npm registry"], + ["ECONNRESET", "npm failure code: ECONNRESET"], + ["PRIVATE_IDENTIFIER", "npm failure code: unknown"], + ])("bounds stdout diagnostics and classifies %s", async (code, guidance) => { + const step = await runStep({ + name: "package-install-omit-optional", + argv: ["npm", "install", "-g", "openclaw"], + cwd: process.cwd(), + env: context.env, + runCommand: async () => ({ + code: 1, + stderr: "", + stdout: [ + `npm error code ${code}`, + ...Array.from({ length: 20 }, () => `npm error ${"🦞".repeat(200)}`), + ].join("\n"), + }), + stepIndex: 0, + totalSteps: 1, + }); + const excerpt = step.failureFacts?.map((fact) => fact.message).join("\n") ?? ""; + expect(Buffer.byteLength(excerpt)).toBeLessThanOrEqual(1024); + expect(excerpt.split("\n").length).toBeLessThanOrEqual(12); + const report = await prepareUpdateFailureReport( + { + attemptId: "npm-bound", + result: { mode: "npm", status: "error", steps: [step], durationMs: 1 }, + }, + context, + ); + expect(report.body).toContain(guidance); + expect(report.body).not.toContain("PRIVATE_IDENTIFIER"); + expect(report.body).not.toContain("\ufffd"); + }); +}); diff --git a/src/infra/update-npm-failure.ts b/src/infra/update-npm-failure.ts new file mode 100644 index 000000000000..0cd669be78c9 --- /dev/null +++ b/src/infra/update-npm-failure.ts @@ -0,0 +1,103 @@ +import { stripAnsi } from "../../packages/terminal-core/src/ansi.js"; +import { resolveStateDir } from "../config/paths.js"; +import { + redactSupportDiagnosticLine, + type SupportRedactionContext, +} from "../logging/diagnostic-support-redaction.js"; +import { truncateUtf8Prefix } from "../utils/utf8-truncate.js"; +import type { UpdateFailureFact } from "./update-failure-facts.js"; + +const NPM_FAILURE_CODES = [ + "EACCES", + "EPERM", + "ENOTEMPTY", + "EEXIST", + "ENOENT", + "E404", + "ETARGET", + "ENOSPC", + "ENOTFOUND", + "EAI_AGAIN", + "ECONNREFUSED", + "ECONNRESET", + "ETIMEDOUT", + "ENETUNREACH", + "EHOSTUNREACH", + "EPIPE", + "E401", + "E403", + "EOTP", + "ERESOLVE", + "EBADENGINE", + "EINTEGRITY", + "EUSAGE", + "EOVERRIDE", + "EINVALIDTAGNAME", + "EUNSUPPORTEDPROTOCOL", + "CERT_HAS_EXPIRED", + "UNABLE_TO_VERIFY_LEAF_SIGNATURE", + "SELF_SIGNED_CERT_IN_CHAIN", + "DEPTH_ZERO_SELF_SIGNED_CERT", + "unknown", +] as const; +type NpmFailureCode = (typeof NPM_FAILURE_CODES)[number]; +type NpmFailureFact = UpdateFailureFact & { check: "npm"; code: NpmFailureCode }; + +function npmFailureCode(value: string | undefined): NpmFailureCode { + return NPM_FAILURE_CODES.find((code) => code === value) ?? "unknown"; +} + +function sanitizeNpmLine(line: string, context: SupportRedactionContext): string { + return truncateUtf8Prefix( + redactSupportDiagnosticLine(line, context).replace( + /^(npm (?:ERR!|error) code)\s+\S+/u, + (_match, prefix: string) => `${prefix} ${npmFailureCode(line.split(/\s+/u)[3])}`, + ), + 200, + ); +} + +/** Capture npm's error lines before command tails or permission guidance replace them. */ +export function createNpmFailureFacts( + stdout: string, + stderr: string, + env: NodeJS.ProcessEnv = process.env, +): NpmFailureFact[] { + const lines = stripAnsi(`${stderr}\n${stdout}`) + .split(/[\r\n\u2028\u2029]/u) + .map((line) => line.trim()) + .filter((line) => /^npm (?:ERR!|error)(?:\s|$)/u.test(line)); + const code = npmFailureCode( + lines.map((line) => /^npm (?:ERR!|error) code (\S+)/u.exec(line)?.[1]).find(Boolean), + ); + const context = { env, stateDir: resolveStateDir(env) }; + // The existing ledger admits five 200-character facts. Stay within that contract + // and a stricter UTF-8 budget instead of introducing a second diagnostic store. + return (lines.length ? lines.slice(0, 5) : ["npm error (no error lines captured)"]).map( + (line) => ({ check: "npm", code, message: sanitizeNpmLine(line, context) }), + ); +} + +export function formatNpmFailureFacts( + facts: readonly UpdateFailureFact[], + context: SupportRedactionContext, +): string[] { + const npm = facts.filter((fact) => fact.check === "npm").slice(0, 5); + if (!npm.length) { + return []; + } + const code = npmFailureCode(npm[0]?.code); + const remedy = + code === "EACCES" || code === "EPERM" + ? "Check the npm global prefix and run the update as its owning account: https://docs.openclaw.ai/cli/update." + : code === "ENOSPC" + ? "Free disk space on the npm prefix and cache volumes, then retry the update." + : code === "E404" || code === "ETARGET" + ? "Check the configured npm registry and requested package version or tag, then retry the update." + : undefined; + return [ + `npm failure code: ${code}`, + ...npm.flatMap((fact) => (fact.message ? [sanitizeNpmLine(fact.message, context)] : [])), + ...(remedy ? [`Next step: ${remedy}`] : []), + ]; +} diff --git a/src/infra/update-runner-command.ts b/src/infra/update-runner-command.ts index dbe1ef44f729..c69d8aa9c052 100644 --- a/src/infra/update-runner-command.ts +++ b/src/infra/update-runner-command.ts @@ -3,6 +3,7 @@ import { formatErrorMessage } from "./errors.js"; import { trimLogTail } from "./restart-sentinel.js"; import { createUpdateErrorFact, createUpdateFailureFact } from "./update-failure-facts.js"; import { createGlobalInstallEnv } from "./update-global.js"; +import { createNpmFailureFacts } from "./update-npm-failure.js"; import { UPDATE_RUN_HEARTBEAT_MS } from "./update-run-timeouts.js"; import type { CommandRunner, @@ -73,6 +74,15 @@ export async function runStep(opts: RunStepOptions): Promise { const durationMs = Date.now() - started; const stdoutTail = trimLogTail(result.stdout, MAX_LOG_CHARS); const stderrTail = trimLogTail(result.stderr, MAX_LOG_CHARS); + if ( + !failureFacts && + result.code !== 0 && + ["package-install", "package-install-omit-optional", "package-pack"].includes(name) && + (/(?:^|[\\/])npm(?:\.cmd|\.exe)?$/iu.test(argv[0] ?? "") || + /\bnpm (?:ERR!|error)(?:\s|$)/u.test(`${result.stderr}\n${result.stdout}`)) + ) { + failureFacts = createNpmFailureFacts(result.stdout, result.stderr, env); + } failureFacts ??= result.code !== 0 || result.killed || result.termination === "timeout" ? [