diff --git a/app/server/app.ts b/app/server/app.ts index fca8126f..bdf541fe 100644 --- a/app/server/app.ts +++ b/app/server/app.ts @@ -92,9 +92,9 @@ export const createApp = () => { logger.error(err.cause.message); } - const { status, message } = handleServiceError(err); + const { status, message, details } = handleServiceError(err); - return c.json({ message }, status as 500); + return c.json(details ? { message, details } : { message }, status as 500); }); return app; diff --git a/app/server/modules/backups/__tests__/backups.execution.test.ts b/app/server/modules/backups/__tests__/backups.execution.test.ts index bc984c58..6d273d7e 100644 --- a/app/server/modules/backups/__tests__/backups.execution.test.ts +++ b/app/server/modules/backups/__tests__/backups.execution.test.ts @@ -177,6 +177,36 @@ describe("stop backup", () => { expect(updatedSchedule.lastBackupError).toBe("error: open /mnt/data/private.db: permission denied"); }); + test("should store restic diagnostic details instead of the generic summary on hard failure", async () => { + const { resticBackupMock } = setup(); + const volume = await createTestVolume(); + const repository = await createTestRepository(); + const schedule = await createTestBackupSchedule({ + volumeId: volume.id, + repositoryId: repository.id, + }); + + resticBackupMock.mockImplementationOnce((params: SafeSpawnParams) => { + params.onStderr?.("Permissions 0755 for '/tmp/zerobyte-ssh-key' are too open."); + params.onStderr?.("This private key will be ignored."); + + return Promise.resolve({ + exitCode: 1, + summary: "", + error: "ssh command exited", + stderr: "Permissions 0755 for '/tmp/zerobyte-ssh-key' are too open.\nThis private key will be ignored.", + }); + }); + + await backupsExecutionService.executeBackup(schedule.id); + + const updatedSchedule = await backupsService.getScheduleById(schedule.id); + expect(updatedSchedule.lastBackupStatus).toBe("error"); + expect(updatedSchedule.lastBackupError).toBe( + "Permissions 0755 for '/tmp/zerobyte-ssh-key' are too open.\nThis private key will be ignored.", + ); + }); + test("should stop a running backup", async () => { // arrange const { resticBackupMock } = setup(); diff --git a/app/server/modules/backups/backups.execution.ts b/app/server/modules/backups/backups.execution.ts index 11a8ba4e..d6c68518 100644 --- a/app/server/modules/backups/backups.execution.ts +++ b/app/server/modules/backups/backups.execution.ts @@ -4,7 +4,7 @@ import { restic } from "../../core/restic"; import { logger } from "@zerobyte/core/node"; import { cache, cacheKeys } from "../../utils/cache"; import { getVolumePath } from "../volumes/helpers"; -import { toMessage } from "../../utils/errors"; +import { toErrorDetails, toMessage } from "../../utils/errors"; import { serverEvents } from "../../core/events"; import { notificationsService } from "../notifications/notifications.service"; import { repoMutex } from "../../core/repository-mutex"; @@ -225,11 +225,12 @@ const handleBackupFailure = async ( partialContext?: Partial, ): Promise => { const errorMessage = toMessage(error); + const errorDetails = toErrorDetails(error); await scheduleQueries.updateStatus(scheduleId, organizationId, { lastBackupAt: Date.now(), lastBackupStatus: "error", - lastBackupError: errorMessage, + lastBackupError: errorDetails, }); if (partialContext?.schedule && partialContext?.volume && partialContext?.repository) { @@ -252,7 +253,7 @@ const handleBackupFailure = async ( volumeName: ctx.volume.name, repositoryName: ctx.repository.name, scheduleName: ctx.schedule.name, - error: errorMessage, + error: errorDetails, }) .catch((notifError) => { logger.error(`Failed to send backup failure notification: ${toMessage(notifError)}`); diff --git a/app/server/modules/repositories/__tests__/repositories.controller.test.ts b/app/server/modules/repositories/__tests__/repositories.controller.test.ts index ed8d0b75..7b255ec3 100644 --- a/app/server/modules/repositories/__tests__/repositories.controller.test.ts +++ b/app/server/modules/repositories/__tests__/repositories.controller.test.ts @@ -341,7 +341,8 @@ describe("repositories updates", () => { expect(res.status).toBe(500); const body = await res.json(); - expect(body.message).toContain("Command failed"); + expect(body.message).toBe("Command failed: An error occurred while executing the command."); + expect(body.details).toBe("Fatal: unexpected HTTP response (403): 403 Forbidden"); } finally { deleteSnapshotSpy.mockRestore(); } diff --git a/app/server/utils/__tests__/spawn.test.ts b/app/server/utils/__tests__/spawn.test.ts index 7758eea8..66164c42 100644 --- a/app/server/utils/__tests__/spawn.test.ts +++ b/app/server/utils/__tests__/spawn.test.ts @@ -206,6 +206,15 @@ describe("safeSpawn", () => { expect(result.error).toBe("err_last"); }); + test("stderr keeps the full stderr output", async () => { + const result = await safeSpawn({ + command: "sh", + args: ["-c", "echo err_first >&2 && echo err_last >&2 && exit 1"], + }); + + expect(result.stderr).toBe("err_first\nerr_last"); + }); + test("returns exitCode -1 when the command is not found", async () => { const result = await safeSpawn({ command: "this-command-does-not-exist-zerobyte", diff --git a/app/server/utils/errors.ts b/app/server/utils/errors.ts index b592505e..d1adf687 100644 --- a/app/server/utils/errors.ts +++ b/app/server/utils/errors.ts @@ -1,11 +1,20 @@ import { HttpError } from "http-errors-enhanced"; import { sanitizeSensitiveData } from "@zerobyte/core/node"; +import { ResticError } from "@zerobyte/core/restic"; export const handleServiceError = (error: unknown) => { if (error instanceof HttpError) { return { message: sanitizeSensitiveData(error.message), status: error.statusCode }; } + if (error instanceof ResticError) { + return { + message: sanitizeSensitiveData(error.summary), + details: error.details ? sanitizeSensitiveData(error.details) : undefined, + status: 500 as const, + }; + } + return { message: sanitizeSensitiveData(toMessage(error)), status: 500 as const }; }; @@ -13,3 +22,11 @@ export const toMessage = (err: unknown): string => { const message = err instanceof Error ? err.message : String(err); return sanitizeSensitiveData(message); }; + +export const toErrorDetails = (err: unknown): string => { + if (err instanceof ResticError) { + return sanitizeSensitiveData(err.details || err.summary); + } + + return toMessage(err); +}; diff --git a/packages/core/src/restic/commands/__tests__/backup.test.ts b/packages/core/src/restic/commands/__tests__/backup.test.ts index 476c253f..9c4eb763 100644 --- a/packages/core/src/restic/commands/__tests__/backup.test.ts +++ b/packages/core/src/restic/commands/__tests__/backup.test.ts @@ -301,6 +301,27 @@ describe("backup command", () => { expect((error as ResticError).code).toBe(12); }); + test("stores restic summary separately from diagnostic stderr", async () => { + setup({ + spawnResult: { + exitCode: 1, + summary: "", + error: "ssh command exited", + }, + onSpawnCall: (params) => { + params.onStderr?.("Permissions 0755 for '/tmp/zerobyte-ssh-key' are too open."); + params.onStderr?.("This private key will be ignored."); + }, + }); + + const error = await backup(config, "/mnt/data", { organizationId: "org-1" }, mockDeps).catch((e) => e); + expect(error).toBeInstanceOf(ResticError); + expect((error as ResticError).summary).toBe("Command failed: An error occurred while executing the command."); + expect((error as ResticError).details).toBe( + "Permissions 0755 for '/tmp/zerobyte-ssh-key' are too open.\nThis private key will be ignored.", + ); + }); + test("returns { result: null } when the abort signal is triggered", async () => { const controller = new AbortController(); setup({ diff --git a/packages/core/src/restic/commands/backup.ts b/packages/core/src/restic/commands/backup.ts index 1a9078e1..ed414fcc 100644 --- a/packages/core/src/restic/commands/backup.ts +++ b/packages/core/src/restic/commands/backup.ts @@ -181,7 +181,7 @@ export const backup = async ( logger.error(`Restic backup failed: ${res.error}`); logger.error(`Command executed: restic ${args.join(" ")}`); - throw new ResticError(res.exitCode, res.error); + throw new ResticError(res.exitCode, stderrLines.join("\n") || res.stderr || res.error); } const lastLine = res.summary.trim(); diff --git a/packages/core/src/restic/commands/ls.ts b/packages/core/src/restic/commands/ls.ts index 1d703025..4ea52265 100644 --- a/packages/core/src/restic/commands/ls.ts +++ b/packages/core/src/restic/commands/ls.ts @@ -126,7 +126,7 @@ export const ls = async ( if (res.exitCode !== 0) { logger.error(`Restic ls failed: ${res.error}`); - throw new ResticError(res.exitCode, res.error); + throw new ResticError(res.exitCode, res.stderr || res.error); } if (totalNodes > offset + limit) { diff --git a/packages/core/src/restic/commands/restore.ts b/packages/core/src/restic/commands/restore.ts index 8788f0c1..2f298e49 100644 --- a/packages/core/src/restic/commands/restore.ts +++ b/packages/core/src/restic/commands/restore.ts @@ -134,7 +134,7 @@ export const restore = async ( if (res.exitCode !== 0) { logger.error(`Restic restore failed: ${res.error}`); - throw new ResticError(res.exitCode, res.error); + throw new ResticError(res.exitCode, res.stderr || res.error); } const lastLine = res.summary.trim(); diff --git a/packages/core/src/restic/error.ts b/packages/core/src/restic/error.ts index 60e039b7..7f631543 100644 --- a/packages/core/src/restic/error.ts +++ b/packages/core/src/restic/error.ts @@ -10,12 +10,16 @@ const resticErrorCodes: Record = { export class ResticError extends Error { code: number; + summary: string; + details: string; - constructor(code: number, stderr: string) { - const message = resticErrorCodes[code] || `Unknown restic error with code ${code}`; - super(`${message}\n${stderr}`); + constructor(code: number, details: string) { + const summary = resticErrorCodes[code] || `Unknown restic error with code ${code}`; + super(details ? `${summary}\n${details}` : summary); this.code = code; + this.summary = summary; + this.details = details; this.name = "ResticError"; } } diff --git a/packages/core/src/utils/spawn.ts b/packages/core/src/utils/spawn.ts index 35b2e0e0..5d173b23 100644 --- a/packages/core/src/utils/spawn.ts +++ b/packages/core/src/utils/spawn.ts @@ -59,6 +59,7 @@ export type SpawnResult = { exitCode: number; summary: string; error: string; + stderr?: string; }; export function safeSpawn(params: SafeSpawnParamsLines): Promise; @@ -70,6 +71,7 @@ export function safeSpawn(params: SafeSpawnParams): Promise { let lastStdout = ""; let lastStderr = ""; + let stderr = ""; return new Promise((resolve) => { const child = spawn(command, args, { @@ -101,6 +103,7 @@ export function safeSpawn(params: SafeSpawnParams): Promise { rlErr.on("line", (line) => { if (onStderr) onStderr(line); + stderr += `${line}\n`; const trimmed = line.trim(); if (trimmed.length > 0) { lastStderr = line; @@ -115,6 +118,7 @@ export function safeSpawn(params: SafeSpawnParams): Promise { exitCode: -1, summary: lastStdout, error: err.message || lastStderr, + stderr: stderr.trim(), }); }); @@ -123,6 +127,7 @@ export function safeSpawn(params: SafeSpawnParams): Promise { exitCode: code ?? -1, summary: lastStdout, error: lastStderr, + stderr: stderr.trim(), }); }); });