fix(restic): separate error summary from diagnostic details (#694)
Some checks failed
Release Workflow / determine-release-type (push) Has been cancelled
Release Workflow / checks (push) Has been cancelled
Release Workflow / e2e-tests (push) Has been cancelled
Release Workflow / build-images (push) Has been cancelled
Release Workflow / publish-release (push) Has been cancelled
Some checks failed
Release Workflow / determine-release-type (push) Has been cancelled
Release Workflow / checks (push) Has been cancelled
Release Workflow / e2e-tests (push) Has been cancelled
Release Workflow / build-images (push) Has been cancelled
Release Workflow / publish-release (push) Has been cancelled
This commit is contained in:
parent
962ab01ea4
commit
05dd440dea
12 changed files with 117 additions and 15 deletions
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
|
|
@ -205,7 +235,7 @@ describe("stop backup", () => {
|
|||
// assert
|
||||
const updatedSchedule = await backupsService.getScheduleById(schedule.id);
|
||||
expect(updatedSchedule.lastBackupStatus).toBe("warning");
|
||||
expect(updatedSchedule.lastBackupError).toBe("Backup was stopped by user");
|
||||
expect(updatedSchedule.lastBackupError).toBe("Backup was stopped by the user");
|
||||
});
|
||||
|
||||
test("should throw ConflictError when trying to stop non-running backup", async () => {
|
||||
|
|
|
|||
|
|
@ -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<BackupContext>,
|
||||
): Promise<void> => {
|
||||
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)}`);
|
||||
|
|
@ -323,7 +324,7 @@ const stopBackup = async (scheduleId: number) => {
|
|||
} finally {
|
||||
await scheduleQueries.updateStatus(scheduleId, organizationId, {
|
||||
lastBackupStatus: "warning",
|
||||
lastBackupError: "Backup was stopped by user",
|
||||
lastBackupError: "Backup was stopped by the user",
|
||||
});
|
||||
}
|
||||
};
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
}
|
||||
|
|
|
|||
|
|
@ -206,6 +206,24 @@ describe("safeSpawn", () => {
|
|||
expect(result.error).toBe("err_last");
|
||||
});
|
||||
|
||||
test("stderr keeps the captured 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("stderr keeps only the last 50 stderr lines", async () => {
|
||||
const result = await safeSpawn({
|
||||
command: "sh",
|
||||
args: ["-c", "i=1; while [ $i -le 55 ]; do echo err_$i >&2; i=$((i+1)); done; exit 1"],
|
||||
});
|
||||
|
||||
expect(result.stderr).toBe(Array.from({ length: 50 }, (_, index) => `err_${index + 6}`).join("\n"));
|
||||
});
|
||||
|
||||
test("returns exitCode -1 when the command is not found", async () => {
|
||||
const result = await safeSpawn({
|
||||
command: "this-command-does-not-exist-zerobyte",
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
};
|
||||
|
|
|
|||
|
|
@ -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({
|
||||
|
|
|
|||
|
|
@ -170,7 +170,7 @@ export const backup = async (
|
|||
|
||||
if (options.signal?.aborted) {
|
||||
logger.warn("Restic backup was aborted by signal.");
|
||||
return { result: null, exitCode: res.exitCode, warningDetails: null };
|
||||
return { result: null, exitCode: res.exitCode, warningDetails: "Backup was stopped by the user" };
|
||||
}
|
||||
|
||||
if (res.exitCode === 3) {
|
||||
|
|
@ -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();
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
|
|
|
|||
|
|
@ -10,12 +10,16 @@ const resticErrorCodes: Record<number, string> = {
|
|||
|
||||
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";
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -59,8 +59,11 @@ export type SpawnResult = {
|
|||
exitCode: number;
|
||||
summary: string;
|
||||
error: string;
|
||||
stderr?: string;
|
||||
};
|
||||
|
||||
const MAX_STDERR_LINES = 50;
|
||||
|
||||
export function safeSpawn(params: SafeSpawnParamsLines): Promise<SpawnResult>;
|
||||
export function safeSpawn(params: SafeSpawnParamsRaw): Promise<SpawnResult>;
|
||||
export function safeSpawn(params: SafeSpawnParams): Promise<SpawnResult> {
|
||||
|
|
@ -70,6 +73,7 @@ export function safeSpawn(params: SafeSpawnParams): Promise<SpawnResult> {
|
|||
|
||||
let lastStdout = "";
|
||||
let lastStderr = "";
|
||||
const stderrLines: string[] = [];
|
||||
|
||||
return new Promise<SpawnResult>((resolve) => {
|
||||
const child = spawn(command, args, {
|
||||
|
|
@ -101,6 +105,10 @@ export function safeSpawn(params: SafeSpawnParams): Promise<SpawnResult> {
|
|||
|
||||
rlErr.on("line", (line) => {
|
||||
if (onStderr) onStderr(line);
|
||||
stderrLines.push(line);
|
||||
if (stderrLines.length > MAX_STDERR_LINES) {
|
||||
stderrLines.shift();
|
||||
}
|
||||
const trimmed = line.trim();
|
||||
if (trimmed.length > 0) {
|
||||
lastStderr = line;
|
||||
|
|
@ -115,6 +123,7 @@ export function safeSpawn(params: SafeSpawnParams): Promise<SpawnResult> {
|
|||
exitCode: -1,
|
||||
summary: lastStdout,
|
||||
error: err.message || lastStderr,
|
||||
stderr: stderrLines.join("\n"),
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -123,6 +132,7 @@ export function safeSpawn(params: SafeSpawnParams): Promise<SpawnResult> {
|
|||
exitCode: code ?? -1,
|
||||
summary: lastStdout,
|
||||
error: lastStderr,
|
||||
stderr: stderrLines.join("\n"),
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in a new issue