fix(restic): separate error summary from diagnostic details
This commit is contained in:
parent
8e57031cfd
commit
026098a3a9
12 changed files with 100 additions and 12 deletions
|
|
@ -92,9 +92,9 @@ export const createApp = () => {
|
||||||
logger.error(err.cause.message);
|
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;
|
return app;
|
||||||
|
|
|
||||||
|
|
@ -177,6 +177,36 @@ describe("stop backup", () => {
|
||||||
expect(updatedSchedule.lastBackupError).toBe("error: open /mnt/data/private.db: permission denied");
|
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 () => {
|
test("should stop a running backup", async () => {
|
||||||
// arrange
|
// arrange
|
||||||
const { resticBackupMock } = setup();
|
const { resticBackupMock } = setup();
|
||||||
|
|
|
||||||
|
|
@ -4,7 +4,7 @@ import { restic } from "../../core/restic";
|
||||||
import { logger } from "@zerobyte/core/node";
|
import { logger } from "@zerobyte/core/node";
|
||||||
import { cache, cacheKeys } from "../../utils/cache";
|
import { cache, cacheKeys } from "../../utils/cache";
|
||||||
import { getVolumePath } from "../volumes/helpers";
|
import { getVolumePath } from "../volumes/helpers";
|
||||||
import { toMessage } from "../../utils/errors";
|
import { toErrorDetails, toMessage } from "../../utils/errors";
|
||||||
import { serverEvents } from "../../core/events";
|
import { serverEvents } from "../../core/events";
|
||||||
import { notificationsService } from "../notifications/notifications.service";
|
import { notificationsService } from "../notifications/notifications.service";
|
||||||
import { repoMutex } from "../../core/repository-mutex";
|
import { repoMutex } from "../../core/repository-mutex";
|
||||||
|
|
@ -225,11 +225,12 @@ const handleBackupFailure = async (
|
||||||
partialContext?: Partial<BackupContext>,
|
partialContext?: Partial<BackupContext>,
|
||||||
): Promise<void> => {
|
): Promise<void> => {
|
||||||
const errorMessage = toMessage(error);
|
const errorMessage = toMessage(error);
|
||||||
|
const errorDetails = toErrorDetails(error);
|
||||||
|
|
||||||
await scheduleQueries.updateStatus(scheduleId, organizationId, {
|
await scheduleQueries.updateStatus(scheduleId, organizationId, {
|
||||||
lastBackupAt: Date.now(),
|
lastBackupAt: Date.now(),
|
||||||
lastBackupStatus: "error",
|
lastBackupStatus: "error",
|
||||||
lastBackupError: errorMessage,
|
lastBackupError: errorDetails,
|
||||||
});
|
});
|
||||||
|
|
||||||
if (partialContext?.schedule && partialContext?.volume && partialContext?.repository) {
|
if (partialContext?.schedule && partialContext?.volume && partialContext?.repository) {
|
||||||
|
|
@ -252,7 +253,7 @@ const handleBackupFailure = async (
|
||||||
volumeName: ctx.volume.name,
|
volumeName: ctx.volume.name,
|
||||||
repositoryName: ctx.repository.name,
|
repositoryName: ctx.repository.name,
|
||||||
scheduleName: ctx.schedule.name,
|
scheduleName: ctx.schedule.name,
|
||||||
error: errorMessage,
|
error: errorDetails,
|
||||||
})
|
})
|
||||||
.catch((notifError) => {
|
.catch((notifError) => {
|
||||||
logger.error(`Failed to send backup failure notification: ${toMessage(notifError)}`);
|
logger.error(`Failed to send backup failure notification: ${toMessage(notifError)}`);
|
||||||
|
|
|
||||||
|
|
@ -341,7 +341,8 @@ describe("repositories updates", () => {
|
||||||
|
|
||||||
expect(res.status).toBe(500);
|
expect(res.status).toBe(500);
|
||||||
const body = await res.json();
|
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 {
|
} finally {
|
||||||
deleteSnapshotSpy.mockRestore();
|
deleteSnapshotSpy.mockRestore();
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -206,6 +206,15 @@ describe("safeSpawn", () => {
|
||||||
expect(result.error).toBe("err_last");
|
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 () => {
|
test("returns exitCode -1 when the command is not found", async () => {
|
||||||
const result = await safeSpawn({
|
const result = await safeSpawn({
|
||||||
command: "this-command-does-not-exist-zerobyte",
|
command: "this-command-does-not-exist-zerobyte",
|
||||||
|
|
|
||||||
|
|
@ -1,11 +1,20 @@
|
||||||
import { HttpError } from "http-errors-enhanced";
|
import { HttpError } from "http-errors-enhanced";
|
||||||
import { sanitizeSensitiveData } from "@zerobyte/core/node";
|
import { sanitizeSensitiveData } from "@zerobyte/core/node";
|
||||||
|
import { ResticError } from "@zerobyte/core/restic";
|
||||||
|
|
||||||
export const handleServiceError = (error: unknown) => {
|
export const handleServiceError = (error: unknown) => {
|
||||||
if (error instanceof HttpError) {
|
if (error instanceof HttpError) {
|
||||||
return { message: sanitizeSensitiveData(error.message), status: error.statusCode };
|
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 };
|
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);
|
const message = err instanceof Error ? err.message : String(err);
|
||||||
return sanitizeSensitiveData(message);
|
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);
|
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 () => {
|
test("returns { result: null } when the abort signal is triggered", async () => {
|
||||||
const controller = new AbortController();
|
const controller = new AbortController();
|
||||||
setup({
|
setup({
|
||||||
|
|
|
||||||
|
|
@ -181,7 +181,7 @@ export const backup = async (
|
||||||
logger.error(`Restic backup failed: ${res.error}`);
|
logger.error(`Restic backup failed: ${res.error}`);
|
||||||
logger.error(`Command executed: restic ${args.join(" ")}`);
|
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();
|
const lastLine = res.summary.trim();
|
||||||
|
|
|
||||||
|
|
@ -126,7 +126,7 @@ export const ls = async (
|
||||||
|
|
||||||
if (res.exitCode !== 0) {
|
if (res.exitCode !== 0) {
|
||||||
logger.error(`Restic ls failed: ${res.error}`);
|
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) {
|
if (totalNodes > offset + limit) {
|
||||||
|
|
|
||||||
|
|
@ -134,7 +134,7 @@ export const restore = async (
|
||||||
|
|
||||||
if (res.exitCode !== 0) {
|
if (res.exitCode !== 0) {
|
||||||
logger.error(`Restic restore failed: ${res.error}`);
|
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();
|
const lastLine = res.summary.trim();
|
||||||
|
|
|
||||||
|
|
@ -10,12 +10,16 @@ const resticErrorCodes: Record<number, string> = {
|
||||||
|
|
||||||
export class ResticError extends Error {
|
export class ResticError extends Error {
|
||||||
code: number;
|
code: number;
|
||||||
|
summary: string;
|
||||||
|
details: string;
|
||||||
|
|
||||||
constructor(code: number, stderr: string) {
|
constructor(code: number, details: string) {
|
||||||
const message = resticErrorCodes[code] || `Unknown restic error with code ${code}`;
|
const summary = resticErrorCodes[code] || `Unknown restic error with code ${code}`;
|
||||||
super(`${message}\n${stderr}`);
|
super(details ? `${summary}\n${details}` : summary);
|
||||||
|
|
||||||
this.code = code;
|
this.code = code;
|
||||||
|
this.summary = summary;
|
||||||
|
this.details = details;
|
||||||
this.name = "ResticError";
|
this.name = "ResticError";
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -59,6 +59,7 @@ export type SpawnResult = {
|
||||||
exitCode: number;
|
exitCode: number;
|
||||||
summary: string;
|
summary: string;
|
||||||
error: string;
|
error: string;
|
||||||
|
stderr?: string;
|
||||||
};
|
};
|
||||||
|
|
||||||
export function safeSpawn(params: SafeSpawnParamsLines): Promise<SpawnResult>;
|
export function safeSpawn(params: SafeSpawnParamsLines): Promise<SpawnResult>;
|
||||||
|
|
@ -70,6 +71,7 @@ export function safeSpawn(params: SafeSpawnParams): Promise<SpawnResult> {
|
||||||
|
|
||||||
let lastStdout = "";
|
let lastStdout = "";
|
||||||
let lastStderr = "";
|
let lastStderr = "";
|
||||||
|
let stderr = "";
|
||||||
|
|
||||||
return new Promise<SpawnResult>((resolve) => {
|
return new Promise<SpawnResult>((resolve) => {
|
||||||
const child = spawn(command, args, {
|
const child = spawn(command, args, {
|
||||||
|
|
@ -101,6 +103,7 @@ export function safeSpawn(params: SafeSpawnParams): Promise<SpawnResult> {
|
||||||
|
|
||||||
rlErr.on("line", (line) => {
|
rlErr.on("line", (line) => {
|
||||||
if (onStderr) onStderr(line);
|
if (onStderr) onStderr(line);
|
||||||
|
stderr += `${line}\n`;
|
||||||
const trimmed = line.trim();
|
const trimmed = line.trim();
|
||||||
if (trimmed.length > 0) {
|
if (trimmed.length > 0) {
|
||||||
lastStderr = line;
|
lastStderr = line;
|
||||||
|
|
@ -115,6 +118,7 @@ export function safeSpawn(params: SafeSpawnParams): Promise<SpawnResult> {
|
||||||
exitCode: -1,
|
exitCode: -1,
|
||||||
summary: lastStdout,
|
summary: lastStdout,
|
||||||
error: err.message || lastStderr,
|
error: err.message || lastStderr,
|
||||||
|
stderr: stderr.trim(),
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|
@ -123,6 +127,7 @@ export function safeSpawn(params: SafeSpawnParams): Promise<SpawnResult> {
|
||||||
exitCode: code ?? -1,
|
exitCode: code ?? -1,
|
||||||
summary: lastStdout,
|
summary: lastStdout,
|
||||||
error: lastStderr,
|
error: lastStderr,
|
||||||
|
stderr: stderr.trim(),
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue