From 23e36b893534fe59447eeb0c6f1a06049bf91b74 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 10 Jul 2026 19:41:17 -0700 Subject: [PATCH] fix: surface API 500 causes, guide update EACCES, non-zero daemon signal exit Addresses three user-reported bugs: - API 500 diagnosability: rethrowAsApiError now preserves the original error as Error `cause` and the /api boundary logs stack + cause for 5xx, so the opaque "task write API returns 500 for every task" failures are traceable (client body stays generic in production). - In-app "Update now": detect EACCES/EPERM install failures and return actionable remediation (sudo fn update / reinstall without sudo / brew upgrade) instead of raw npm stderr; do not retry --force for this class. - Daemon restart: `fn daemon` and `fn serve` exit 128+signal (SIGTERM=143, SIGINT=130) on signal-initiated shutdown so Restart=on-failure restarts a memory-pressure kill. Interactive `fn dashboard` TUI intentionally unchanged. Adds regression tests (update-check EACCES/EPERM, daemon exit codes) and three @runfusion/fusion patch changesets. Co-Authored-By: Claude Opus 4.8 --- .changeset/daemon-signal-nonzero-exit.md | 7 +++ .changeset/task-api-500-diagnostics.md | 7 +++ .changeset/update-now-eacces-guidance.md | 7 +++ .../cli/src/commands/__tests__/daemon.test.ts | 16 +++++ packages/cli/src/commands/daemon.ts | 21 +++++-- packages/cli/src/commands/serve.ts | 18 ++++-- .../src/__tests__/update-check.test.ts | 33 ++++++++++ packages/dashboard/src/api-error.ts | 30 +++++++-- packages/dashboard/src/server.ts | 13 +++- packages/dashboard/src/update-check.ts | 63 +++++++++++++++++++ 10 files changed, 201 insertions(+), 14 deletions(-) create mode 100644 .changeset/daemon-signal-nonzero-exit.md create mode 100644 .changeset/task-api-500-diagnostics.md create mode 100644 .changeset/update-now-eacces-guidance.md diff --git a/.changeset/daemon-signal-nonzero-exit.md b/.changeset/daemon-signal-nonzero-exit.md new file mode 100644 index 0000000000..39bd68aa04 --- /dev/null +++ b/.changeset/daemon-signal-nonzero-exit.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Daemon exits non-zero on signal termination so Restart=on-failure restarts it after a memory-pressure kill. +category: fix +dev: `fn daemon` and `fn serve` (packages/cli/src/commands/daemon.ts, serve.ts) now exit with the POSIX 128+signal code (SIGTERM=143, SIGINT=130) on signal-initiated graceful shutdown instead of 0. Previously a memory-pressure SIGTERM produced exit 0, which `Restart=on-failure` treated as a clean stop, leaving the daemon dead. A deliberate `systemctl stop` still won't restart (systemd honors the requested inactive state regardless of exit code); a non-signal shutdown still exits 0. The interactive TUI launcher (`fn dashboard`) is intentionally unchanged — it has its own signal-name-keyed restart supervisor. diff --git a/.changeset/task-api-500-diagnostics.md b/.changeset/task-api-500-diagnostics.md new file mode 100644 index 0000000000..6a631b4ba5 --- /dev/null +++ b/.changeset/task-api-500-diagnostics.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Server now logs the underlying stack behind an API 500 so opaque task-endpoint failures are diagnosable. +category: fix +dev: `rethrowAsApiError` (packages/dashboard/src/api-error.ts) now preserves the original error as Error `cause` instead of discarding it, and `sendErrorResponse`/the `/api` error boundary (packages/dashboard/src/server.ts) log the stack + cause chain for 5xx (not just the message). The client-facing body stays generic in production. Unblocks root-causing the reported "task write API returns 500 for every task" (GET/DELETE/PATCH/retry/archive/reset on /api/tasks/:id) whose cause was previously never recorded server-side. diff --git a/.changeset/update-now-eacces-guidance.md b/.changeset/update-now-eacces-guidance.md new file mode 100644 index 0000000000..0d0e142c2c --- /dev/null +++ b/.changeset/update-now-eacces-guidance.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: "Update now" now explains permission (EACCES) failures and how to fix them instead of showing raw npm errors. +category: fix +dev: `performUpdateInstall` (packages/dashboard/src/update-check.ts) detects EACCES/EPERM install failures (by error code or stderr text) and returns actionable remediation — run `sudo fn update`, reinstall without sudo, or `brew upgrade fusion` for Homebrew installs — rather than the raw `npm error EACCES … rename '/usr/lib/node_modules/@runfusion/fusion'`. Occurs when Fusion was installed via `sudo npm i -g` (root-owned global dir); `--force` is not retried for this class since it cannot grant write permission. diff --git a/packages/cli/src/commands/__tests__/daemon.test.ts b/packages/cli/src/commands/__tests__/daemon.test.ts index 7313b39174..2764a34cfd 100644 --- a/packages/cli/src/commands/__tests__/daemon.test.ts +++ b/packages/cli/src/commands/__tests__/daemon.test.ts @@ -823,6 +823,22 @@ describe("runDaemon", () => { await triggerSignal("SIGINT"); }); + // FNXC:DaemonSignalExit 2026-07-10-14:00: a memory-pressure SIGTERM must exit + // non-zero (128+signal) so a `Restart=on-failure` supervisor restarts the + // daemon instead of treating the kill as a clean stop. Regression for the + // "daemon exits clean under memory pressure and isn't restarted" report. + it("exits 143 on SIGTERM-initiated shutdown", async () => { + await runDaemon({}); + await triggerSignal("SIGTERM"); + expect(process.exit).toHaveBeenCalledWith(143); + }); + + it("exits 130 on SIGINT-initiated shutdown", async () => { + await runDaemon({}); + await triggerSignal("SIGINT"); + expect(process.exit).toHaveBeenCalledWith(130); + }); + it("auto-loads installed plugins during startup", async () => { const { PluginLoader } = await import("@fusion/core"); diff --git a/packages/cli/src/commands/daemon.ts b/packages/cli/src/commands/daemon.ts index fae75af26a..cddbcdb194 100644 --- a/packages/cli/src/commands/daemon.ts +++ b/packages/cli/src/commands/daemon.ts @@ -962,7 +962,20 @@ export async function runDaemon(opts: DaemonOptions = {}) { let shuttingDown = false; - const shutdown = async () => { + /* + FNXC:DaemonSignalExit 2026-07-10-14:00: + When the host terminates the daemon under memory pressure it sends SIGTERM, which + this handler turns into a graceful shutdown. Exiting 0 on a signal made a + memory-pressure kill indistinguishable from a clean operator stop, so a + `Restart=on-failure` systemd unit treated it as success and left the daemon dead. + Exit with the POSIX 128+signal convention (SIGINT=130, SIGTERM=143) so + `Restart=on-failure` restarts an externally-killed daemon; a deliberate + `systemctl stop` still won't restart (systemd honors the requested inactive + state regardless of exit code). A non-signal shutdown() caller still exits 0. + */ + const SIGNAL_EXIT_CODES: Record = { SIGINT: 130, SIGTERM: 143 }; + + const shutdown = async (signal?: NodeJS.Signals) => { if (shuttingDown) return; shuttingDown = true; @@ -1006,14 +1019,14 @@ export async function runDaemon(opts: DaemonOptions = {}) { } store.close(); - process.exit(0); + process.exit(signal ? (SIGNAL_EXIT_CODES[signal] ?? 128) : 0); }; process.on("SIGINT", () => { - void shutdown(); + void shutdown("SIGINT"); }); process.on("SIGTERM", () => { - void shutdown(); + void shutdown("SIGTERM"); }); // Ignore SIGHUP so the daemon survives SSH session disconnects diff --git a/packages/cli/src/commands/serve.ts b/packages/cli/src/commands/serve.ts index 09d9caca35..ef09dffa79 100644 --- a/packages/cli/src/commands/serve.ts +++ b/packages/cli/src/commands/serve.ts @@ -1080,7 +1080,17 @@ export async function runServe( let shuttingDown = false; - const shutdown = async () => { + /* + FNXC:DaemonSignalExit 2026-07-10-14:00: + Same invariant as `fn daemon`: a memory-pressure SIGTERM must exit non-zero so a + `Restart=on-failure` supervisor restarts the server rather than treating the kill + as a clean stop. Exit 128+signal (SIGINT=130, SIGTERM=143); a non-signal caller + still exits 0. A deliberate `systemctl stop` won't restart regardless (systemd + honors the requested inactive state). + */ + const SIGNAL_EXIT_CODES: Record = { SIGINT: 130, SIGTERM: 143 }; + + const shutdown = async (signal?: NodeJS.Signals) => { if (shuttingDown) return; shuttingDown = true; @@ -1151,14 +1161,14 @@ export async function runServe( stopDiagnosticInterval(); store.close(); - process.exit(0); + process.exit(signal ? (SIGNAL_EXIT_CODES[signal] ?? 128) : 0); }; process.on("SIGINT", () => { - void shutdown(); + void shutdown("SIGINT"); }); process.on("SIGTERM", () => { - void shutdown(); + void shutdown("SIGTERM"); }); // Ignore SIGHUP so the server survives SSH session disconnects. diff --git a/packages/dashboard/src/__tests__/update-check.test.ts b/packages/dashboard/src/__tests__/update-check.test.ts index 8ad02cdc9d..350d1328ec 100644 --- a/packages/dashboard/src/__tests__/update-check.test.ts +++ b/packages/dashboard/src/__tests__/update-check.test.ts @@ -227,6 +227,39 @@ describe("update-check", () => { expect(execFake).toHaveBeenCalledTimes(1); }); + // FNXC:UpdateInstallPermissions 2026-07-10-14:00: a root-owned global dir + // (from `sudo npm i -g`) makes the non-root in-app updater fail with EACCES/ + // EPERM. It must surface actionable remediation, not raw npm stderr, and must + // NOT retry with --force (which cannot grant write permission). + it("performUpdateInstall returns actionable guidance on an EACCES permission failure", async () => { + const execFake = vi.fn().mockRejectedValue( + Object.assign(new Error("EACCES"), { + code: "EACCES", + stderr: "npm error EACCES: permission denied, rename '/usr/lib/node_modules/@runfusion/fusion'", + }), + ); + + const result = await performUpdateInstall("1.0.0", "2.0.0", { exec: execFake, fusionDir }); + expect(result.updated).toBe(false); + expect(result.error).toMatch(/EACCES\/EPERM|not writable|sudo/i); + // Must not fall through to raw npm stderr, and must not retry with --force. + expect(result.error).not.toContain("rename '/usr/lib/node_modules"); + expect(execFake).toHaveBeenCalledTimes(1); + }); + + it("performUpdateInstall detects EPERM from stderr text without an error code", async () => { + const execFake = vi.fn().mockRejectedValue( + Object.assign(new Error("install failed"), { + stderr: "npm error code EPERM\nnpm error operation not permitted", + }), + ); + + const result = await performUpdateInstall("1.0.0", "2.0.0", { exec: execFake, fusionDir }); + expect(result.updated).toBe(false); + expect(result.error).toMatch(/not writable|sudo/i); + expect(execFake).toHaveBeenCalledTimes(1); + }); + describe("frequency", () => { beforeEach(() => { __resetStartupRefreshFlag(); diff --git a/packages/dashboard/src/api-error.ts b/packages/dashboard/src/api-error.ts index 0b972b5d83..bd6b25c199 100644 --- a/packages/dashboard/src/api-error.ts +++ b/packages/dashboard/src/api-error.ts @@ -9,6 +9,15 @@ export interface ApiErrorResponse { export interface SendErrorOptions { details?: Record; logger?: RuntimeLogger; + /* + FNXC:ApiErrorDiagnostics 2026-07-10-14:00: + The original thrown error behind a 5xx. When present, its stack (and any `cause` + chain) is logged so server-side 500s are root-causable. Previously only the error + *message* was logged and `rethrowAsApiError` discarded the stack, leaving the + full-TaskDetail 500s on /api/tasks/:id (GET/DELETE/PATCH/retry/archive/reset) + untraceable across releases. + */ + error?: unknown; } export class ApiError extends Error { @@ -16,12 +25,19 @@ export class ApiError extends Error { public readonly details?: Record; public readonly isOperational: boolean; - constructor(statusCode: number, message: string, details?: Record) { + constructor(statusCode: number, message: string, details?: Record, cause?: unknown) { super(message); this.name = "ApiError"; this.statusCode = statusCode; this.details = details; this.isOperational = true; + // FNXC:ApiErrorDiagnostics 2026-07-10-14:00: preserve the wrapped error's + // stack/chain via Error `cause` so the boundary can log where the 500 came + // from (assigned directly rather than via super(message,{cause}) to stay + // independent of the compiled lib target). + if (cause !== undefined) { + (this as { cause?: unknown }).cause = cause; + } } } @@ -34,11 +50,17 @@ export function sendErrorResponse( if (statusCode >= 500) { const request = res.req; const logger = options?.logger ?? createRuntimeLogger("api:error"); + // FNXC:ApiErrorDiagnostics 2026-07-10-14:00: log the underlying stack and + // cause (not just the message) so a 500 can be traced to its origin. + const originalError = options?.error; + const cause = originalError instanceof Error ? (originalError as { cause?: unknown }).cause : undefined; logger.error("Request failed", { method: request?.method, path: request?.originalUrl ?? request?.path, statusCode, message, + stack: originalError instanceof Error ? originalError.stack : undefined, + cause: cause instanceof Error ? (cause.stack ?? cause.message) : cause !== undefined ? String(cause) : undefined, }); } @@ -63,16 +85,16 @@ export function catchHandler(fn: AsyncHandler): RequestHandler { } if (error instanceof ApiError) { - sendErrorResponse(res, error.statusCode, error.message, { details: error.details }); + sendErrorResponse(res, error.statusCode, error.message, { details: error.details, error }); return; } if (error instanceof Error) { - sendErrorResponse(res, 500, error.message); + sendErrorResponse(res, 500, error.message, { error }); return; } - sendErrorResponse(res, 500, "Internal server error"); + sendErrorResponse(res, 500, "Internal server error", { error }); } }; } diff --git a/packages/dashboard/src/server.ts b/packages/dashboard/src/server.ts index 1572eb6844..d8558722ee 100644 --- a/packages/dashboard/src/server.ts +++ b/packages/dashboard/src/server.ts @@ -1930,13 +1930,22 @@ export function createServer(store: TaskStore, options?: ServerOptions): ReturnT // API Error Handling Middleware - MUST be after API routes but before SPA fallback // This ensures API errors return JSON instead of falling through to the SPA fallback (which returns HTML) + /* + FNXC:ApiErrorDiagnostics 2026-07-10-14:00: + The /api error boundary is the chokepoint for every unhandled per-request error. + It must LOG the underlying error (stack + cause), not just echo a message, so a + 500 is root-causable server-side — the reported "task write API returns 500 for + every task" was undiagnosable because the wrapped error's origin was never + recorded. The client-facing body stays generic in production (avoid leaking + internals); pass `error: err` so sendErrorResponse logs the stack/cause. + */ app.use("/api", (err: unknown, _req: express.Request, res: express.Response, _next: express.NextFunction) => { if (res.headersSent) { return; } if (err instanceof ApiError) { - sendErrorResponse(res, err.statusCode, err.message, { details: err.details }); + sendErrorResponse(res, err.statusCode, err.message, { details: err.details, error: err }); return; } @@ -1948,7 +1957,7 @@ export function createServer(store: TaskStore, options?: ServerOptions): ReturnT ? err.message : fallbackMessage; - sendErrorResponse(res, 500, message); + sendErrorResponse(res, 500, message, { error: err }); }); if (!isHeadless) { diff --git a/packages/dashboard/src/update-check.ts b/packages/dashboard/src/update-check.ts index dc58922589..a593be339d 100644 --- a/packages/dashboard/src/update-check.ts +++ b/packages/dashboard/src/update-check.ts @@ -105,6 +105,57 @@ function getInstallErrorMessage(error: unknown): string { return error instanceof Error ? error.message : String(error); } +/* +FNXC:UpdateInstallPermissions 2026-07-10-14:00: +The in-app "Update now" button runs `npm install -g @runfusion/fusion@latest` as the +(typically non-root) dashboard process. When Fusion was installed via `sudo npm i -g`, +the global package dir is root-owned, so npm's rename() fails with EACCES/EPERM and the +button ALWAYS fails. Previously the raw npm stderr was surfaced with no explanation. +Detect the permission class and return an actionable remediation instead of a cryptic +EACCES, mirroring the CLI's Homebrew-path awareness. (`--force` cannot grant write +permission, so we do not retry it for this class — unlike bin-collision errors.) +*/ +function isPermissionInstallError(error: unknown): boolean { + const installError = error as InstallError & { code?: string }; + if (installError?.code === "EACCES" || installError?.code === "EPERM") return true; + const message = [installError?.message, installError?.stderr, installError?.stdout] + .filter((part): part is string => typeof part === "string" && part.length > 0) + .join("\n"); + return /\bEACCES\b|\bEPERM\b|permission denied|operation not permitted/i.test(message); +} + +/** Best-effort path of the running Fusion binary, used to tailor remediation. */ +function detectRunningBinaryPath(): string | null { + const argvPath = process.argv[1]; + if (typeof argvPath === "string" && argvPath.length > 0) return argvPath; + return typeof process.execPath === "string" ? process.execPath : null; +} + +function isHomebrewInstall(binaryPath: string | null): boolean { + if (!binaryPath) return false; + return ( + binaryPath.startsWith("/opt/homebrew/") || + binaryPath.startsWith("/usr/local/Homebrew/") || + binaryPath.startsWith("/home/linuxbrew/") + ); +} + +function getPermissionRemediationMessage(binaryPath: string | null): string { + if (isHomebrewInstall(binaryPath)) { + return ( + "Update failed: this Fusion install is managed by Homebrew and cannot be updated with npm. " + + "Update it from a terminal with: brew upgrade fusion" + ); + } + return ( + "Update failed: the global npm directory is not writable by the Fusion process (EACCES/EPERM). " + + "This happens when Fusion was installed with `sudo npm i -g`, leaving a root-owned package directory " + + "that the dashboard (running as a normal user) cannot replace. Update from a terminal instead:\n" + + " • sudo fn update (or: sudo npm i -g @runfusion/fusion@latest)\n" + + " • or reinstall without sudo so the global directory is user-owned" + ); +} + function getInstallOptions(): { timeout: number; maxBuffer: number } { return { timeout: INSTALL_TIMEOUT_MS, @@ -169,6 +220,18 @@ export async function performUpdateInstall( updated: true, }; } catch (error) { + // FNXC:UpdateInstallPermissions 2026-07-10-14:00: a root-owned global dir + // (from `sudo npm i -g`) yields EACCES/EPERM the non-root updater cannot + // recover from — return actionable guidance rather than raw npm stderr. + if (isPermissionInstallError(error)) { + return { + currentVersion, + latestVersion, + updated: false, + error: getPermissionRemediationMessage(detectRunningBinaryPath()), + }; + } + if (!isBinCollisionInstallError(error)) { return { currentVersion,