fix: correctly propagate agent restic error

This commit is contained in:
Nicolas Meienberger 2026-03-30 20:39:47 +02:00
parent d291bb0382
commit 3162cba8b2
4 changed files with 319 additions and 230 deletions

View file

@ -0,0 +1,71 @@
import { afterEach, expect, mock, spyOn, test } from "bun:test";
import { Effect } from "effect";
import waitForExpect from "wait-for-expect";
import { fromAny } from "@total-typescript/shoehorn";
import { createControllerMessage, parseAgentMessage } from "@zerobyte/contracts/agent-protocol";
import * as resticServer from "@zerobyte/core/restic/server";
import { createControllerSession } from "../controller-session";
afterEach(() => {
mock.restore();
});
test("emits backup.failed when a backup command hits a restic error", async () => {
spyOn(resticServer, "createRestic").mockReturnValue(
fromAny({
backup: () => Effect.fail(new Error("source path missing")),
}),
);
const outboundMessages: string[] = [];
const session = createControllerSession(
fromAny({
send: (message: string) => {
outboundMessages.push(message);
},
}),
);
try {
session.onOpen();
session.onMessage(
createControllerMessage("backup.run", {
jobId: "job-1",
scheduleId: "schedule-1",
organizationId: "org-1",
sourcePath: "/tmp/missing-source",
repositoryConfig: {
backend: "local",
path: "/tmp/test-repository",
},
options: {},
runtime: {
password: "password",
cacheDir: "/tmp/restic-cache",
passFile: "/tmp/restic-pass",
defaultExcludes: [],
},
}),
);
await waitForExpect(() => {
const failedMessage = outboundMessages
.map((message) => parseAgentMessage(message))
.find((message) => message?.success && message.data.type === "backup.failed");
expect(failedMessage?.success).toBe(true);
if (!failedMessage || !failedMessage.success || failedMessage.data.type !== "backup.failed") {
return;
}
expect(failedMessage.data.payload).toEqual({
jobId: "job-1",
scheduleId: "schedule-1",
error: "source path missing",
errorDetails: "source path missing",
});
});
} finally {
session.close();
}
});

View file

@ -43,9 +43,8 @@ export const handleBackupRunCommand = (context: ControllerCommandContext, payloa
const restic = createRestic(deps); const restic = createRestic(deps);
try { yield* restic
const result = yield* Effect.tryPromise(() => .backup(payload.repositoryConfig, payload.sourcePath, {
restic.backup(payload.repositoryConfig, payload.sourcePath, {
organizationId: payload.organizationId, organizationId: payload.organizationId,
...payload.options, ...payload.options,
signal: abortController.signal, signal: abortController.signal,
@ -58,9 +57,11 @@ export const handleBackupRunCommand = (context: ControllerCommandContext, payloa
}), }),
); );
}, },
}), })
); .pipe(
Effect.matchEffect({
onSuccess: (result) => {
return Effect.sync(() => {
if (abortController.signal.aborted) { if (abortController.signal.aborted) {
context.offerOutbound( context.offerOutbound(
createAgentMessage("backup.cancelled", { createAgentMessage("backup.cancelled", {
@ -81,7 +82,10 @@ export const handleBackupRunCommand = (context: ControllerCommandContext, payloa
warningDetails: result.warningDetails ?? undefined, warningDetails: result.warningDetails ?? undefined,
}), }),
); );
} catch (error) { });
},
onFailure: (error) => {
return Effect.sync(() => {
if (abortController.signal.aborted) { if (abortController.signal.aborted) {
context.offerOutbound( context.offerOutbound(
createAgentMessage("backup.cancelled", { createAgentMessage("backup.cancelled", {
@ -101,8 +105,14 @@ export const handleBackupRunCommand = (context: ControllerCommandContext, payloa
errorDetails: toErrorDetails(error), errorDetails: toErrorDetails(error),
}), }),
); );
} finally { });
},
}),
Effect.ensuring(
Effect.sync(() => {
context.deleteRunningJob(payload.jobId); context.deleteRunningJob(payload.jobId);
} }),
),
);
}), }),
).pipe(Effect.asVoid); ).pipe(Effect.asVoid);

View file

@ -1,4 +1,5 @@
import { afterEach, describe, expect, test, vi } from "vitest"; import { afterEach, describe, expect, test, vi } from "vitest";
import { Effect } from "effect";
import * as cleanupModule from "../../helpers/cleanup-temporary-keys"; import * as cleanupModule from "../../helpers/cleanup-temporary-keys";
import * as spawnModule from "../../../utils/spawn"; import * as spawnModule from "../../../utils/spawn";
import { ResticError } from "../../error"; import { ResticError } from "../../error";
@ -62,7 +63,7 @@ const setup = ({ spawnResult = {}, onSpawnCall }: SetupOptions = {}) => {
let capturedArgs: string[] = []; let capturedArgs: string[] = [];
vi.spyOn(cleanupModule, "cleanupTemporaryKeys").mockImplementation(() => Promise.resolve()); vi.spyOn(cleanupModule, "cleanupTemporaryKeys").mockImplementation(() => Promise.resolve());
vi.spyOn(spawnModule, "safeSpawn").mockImplementation((params) => { vi.spyOn(spawnModule, "safeSpawn").mockImplementation((params: SafeSpawnParams) => {
capturedArgs = params.args; capturedArgs = params.args;
return Promise.resolve(onSpawnCall?.(params)).then(() => ({ return Promise.resolve(onSpawnCall?.(params)).then(() => ({
exitCode: 0, exitCode: 0,
@ -87,6 +88,9 @@ const setup = ({ spawnResult = {}, onSpawnCall }: SetupOptions = {}) => {
}; };
}; };
const runBackup = (...args: Parameters<typeof backup>) => Effect.runPromise(backup(...args));
const runBackupError = (...args: Parameters<typeof backup>) => Effect.runPromise(Effect.flip(backup(...args)));
afterEach(() => { afterEach(() => {
vi.restoreAllMocks(); vi.restoreAllMocks();
}); });
@ -95,7 +99,7 @@ describe("backup command", () => {
describe("argument construction", () => { describe("argument construction", () => {
test("passes source path as positional arg when no include list is given", async () => { test("passes source path as positional arg when no include list is given", async () => {
const { getArgs, hasFlag } = setup(); const { getArgs, hasFlag } = setup();
await backup(config, "/mnt/data", { organizationId: "org-1" }, mockDeps); await runBackup(config, "/mnt/data", { organizationId: "org-1" }, mockDeps);
expect(getArgs()).toContain("/mnt/data"); expect(getArgs()).toContain("/mnt/data");
expect(hasFlag("--files-from")).toBe(false); expect(hasFlag("--files-from")).toBe(false);
@ -105,7 +109,7 @@ describe("backup command", () => {
const { getArgs } = setup(); const { getArgs } = setup();
const source = "--help"; const source = "--help";
await backup(config, source, { organizationId: "org-1" }, mockDeps); await runBackup(config, source, { organizationId: "org-1" }, mockDeps);
const separatorIndex = getArgs().indexOf("--"); const separatorIndex = getArgs().indexOf("--");
expect(separatorIndex).toBeGreaterThan(-1); expect(separatorIndex).toBeGreaterThan(-1);
@ -132,7 +136,7 @@ describe("backup command", () => {
}, },
}); });
await backup( await runBackup(
config, config,
"/mnt/data", "/mnt/data",
{ {
@ -152,7 +156,7 @@ describe("backup command", () => {
test("always includes DEFAULT_EXCLUDES as --exclude args", async () => { test("always includes DEFAULT_EXCLUDES as --exclude args", async () => {
const { getOptionValues } = setup(); const { getOptionValues } = setup();
await backup(config, "/mnt/data", { organizationId: "org-1" }, mockDeps); await runBackup(config, "/mnt/data", { organizationId: "org-1" }, mockDeps);
expect(getOptionValues("--exclude").length).toBeGreaterThan(0); expect(getOptionValues("--exclude").length).toBeGreaterThan(0);
}); });
@ -161,7 +165,7 @@ describe("backup command", () => {
describe("exit code handling", () => { describe("exit code handling", () => {
test("returns parsed result on exit code 0", async () => { test("returns parsed result on exit code 0", async () => {
setup(); setup();
const { result, exitCode } = await backup(config, "/mnt/data", { organizationId: "org-1" }, mockDeps); const { result, exitCode } = await runBackup(config, "/mnt/data", { organizationId: "org-1" }, mockDeps);
expect(exitCode).toBe(0); expect(exitCode).toBe(0);
expect(result?.snapshot_id).toBe("abcd1234"); expect(result?.snapshot_id).toBe("abcd1234");
@ -169,7 +173,7 @@ describe("backup command", () => {
test("returns result without throwing on exit code 3 (partial read errors)", async () => { test("returns result without throwing on exit code 3 (partial read errors)", async () => {
setup({ spawnResult: { exitCode: 3 } }); setup({ spawnResult: { exitCode: 3 } });
const { result, exitCode } = await backup(config, "/mnt/data", { organizationId: "org-1" }, mockDeps); const { result, exitCode } = await runBackup(config, "/mnt/data", { organizationId: "org-1" }, mockDeps);
expect(exitCode).toBe(3); expect(exitCode).toBe(3);
expect(result).not.toBeNull(); expect(result).not.toBeNull();
@ -178,15 +182,14 @@ describe("backup command", () => {
test("throws ResticError on non-zero, non-3 exit codes", async () => { test("throws ResticError on non-zero, non-3 exit codes", async () => {
setup({ spawnResult: { exitCode: 1, summary: "", error: "fatal error" } }); setup({ spawnResult: { exitCode: 1, summary: "", error: "fatal error" } });
await expect(backup(config, "/mnt/data", { organizationId: "org-1" }, mockDeps)).rejects.toBeInstanceOf( const error = await runBackupError(config, "/mnt/data", { organizationId: "org-1" }, mockDeps);
ResticError, expect(error).toBeInstanceOf(ResticError);
);
}); });
test("preserves the exit code inside the thrown ResticError", async () => { test("preserves the exit code inside the thrown ResticError", async () => {
setup({ spawnResult: { exitCode: 12, summary: "", error: "wrong password" } }); setup({ spawnResult: { exitCode: 12, summary: "", error: "wrong password" } });
const error = await backup(config, "/mnt/data", { organizationId: "org-1" }, mockDeps).catch((e) => e); const error = await runBackupError(config, "/mnt/data", { organizationId: "org-1" }, mockDeps);
expect(error).toBeInstanceOf(ResticError); expect(error).toBeInstanceOf(ResticError);
expect((error as ResticError).code).toBe(12); expect((error as ResticError).code).toBe(12);
}); });
@ -204,7 +207,7 @@ describe("backup command", () => {
}, },
}); });
const error = await backup(config, "/mnt/data", { organizationId: "org-1" }, mockDeps).catch((e) => e); const error = await runBackupError(config, "/mnt/data", { organizationId: "org-1" }, mockDeps);
expect(error).toBeInstanceOf(ResticError); expect(error).toBeInstanceOf(ResticError);
expect((error as ResticError).summary).toBe("Command failed: An error occurred while executing the command."); expect((error as ResticError).summary).toBe("Command failed: An error occurred while executing the command.");
expect((error as ResticError).details).toBe( expect((error as ResticError).details).toBe(
@ -219,7 +222,7 @@ describe("backup command", () => {
spawnResult: { exitCode: 130, summary: "", error: "" }, spawnResult: { exitCode: 130, summary: "", error: "" },
}); });
const { result, exitCode, warningDetails } = await backup( const { result, exitCode, warningDetails } = await runBackup(
config, config,
"/mnt/data", "/mnt/data",
{ {
@ -238,7 +241,7 @@ describe("backup command", () => {
describe("output parsing", () => { describe("output parsing", () => {
test("returns a fully parsed summary object on valid output", async () => { test("returns a fully parsed summary object on valid output", async () => {
setup(); setup();
const { result } = await backup(config, "/mnt/data", { organizationId: "org-1" }, mockDeps); const { result } = await runBackup(config, "/mnt/data", { organizationId: "org-1" }, mockDeps);
expect(result).toMatchObject({ expect(result).toMatchObject({
message_type: "summary", message_type: "summary",
@ -249,14 +252,14 @@ describe("backup command", () => {
test("returns { result: null } when summary line is not valid JSON", async () => { test("returns { result: null } when summary line is not valid JSON", async () => {
setup({ spawnResult: { summary: "not-json" } }); setup({ spawnResult: { summary: "not-json" } });
const { result } = await backup(config, "/mnt/data", { organizationId: "org-1" }, mockDeps); const { result } = await runBackup(config, "/mnt/data", { organizationId: "org-1" }, mockDeps);
expect(result).toBeNull(); expect(result).toBeNull();
}); });
test("returns { result: null } when summary JSON does not satisfy the schema", async () => { test("returns { result: null } when summary JSON does not satisfy the schema", async () => {
setup({ spawnResult: { summary: JSON.stringify({ message_type: "summary" }) } }); setup({ spawnResult: { summary: JSON.stringify({ message_type: "summary" }) } });
const { result } = await backup(config, "/mnt/data", { organizationId: "org-1" }, mockDeps); const { result } = await runBackup(config, "/mnt/data", { organizationId: "org-1" }, mockDeps);
expect(result).toBeNull(); expect(result).toBeNull();
}); });
@ -267,7 +270,7 @@ describe("backup command", () => {
const progressUpdates: unknown[] = []; const progressUpdates: unknown[] = [];
setup({ onSpawnCall: (params) => params.onStdout?.(VALID_PROGRESS_LINE) }); setup({ onSpawnCall: (params) => params.onStdout?.(VALID_PROGRESS_LINE) });
await backup( await runBackup(
config, config,
"/mnt/data", "/mnt/data",
{ {
@ -294,7 +297,7 @@ describe("backup command", () => {
}); });
await expect( await expect(
backup(config, "/mnt/data", { organizationId: "org-1", onProgress: () => {} }, mockDeps), runBackup(config, "/mnt/data", { organizationId: "org-1", onProgress: () => {} }, mockDeps),
).resolves.toBeDefined(); ).resolves.toBeDefined();
}); });
@ -304,7 +307,7 @@ describe("backup command", () => {
onSpawnCall: (params) => params.onStdout?.(JSON.stringify({ message_type: "verbose_status", action: "scan" })), onSpawnCall: (params) => params.onStdout?.(JSON.stringify({ message_type: "verbose_status", action: "scan" })),
}); });
await backup( await runBackup(
config, config,
"/mnt/data", "/mnt/data",
{ {

View file

@ -1,6 +1,7 @@
import fs from "node:fs/promises"; import fs from "node:fs/promises";
import os from "node:os"; import os from "node:os";
import path from "node:path"; import path from "node:path";
import { Effect } from "effect";
import { throttle } from "es-toolkit"; import { throttle } from "es-toolkit";
import type { CompressionMode, RepositoryConfig } from "../schemas"; import type { CompressionMode, RepositoryConfig } from "../schemas";
import { type ResticBackupProgressDto, resticBackupOutputSchema, resticBackupProgressSchema } from "../restic-dto"; import { type ResticBackupProgressDto, resticBackupOutputSchema, resticBackupProgressSchema } from "../restic-dto";
@ -13,7 +14,7 @@ import { ResticError } from "../error";
import { logger, safeSpawn } from "../../node"; import { logger, safeSpawn } from "../../node";
import type { ResticDeps } from "../types"; import type { ResticDeps } from "../types";
export const backup = async ( export const backup = (
config: RepositoryConfig, config: RepositoryConfig,
source: string, source: string,
options: { options: {
@ -30,7 +31,9 @@ export const backup = async (
customResticParams?: string[]; customResticParams?: string[];
}, },
deps: ResticDeps, deps: ResticDeps,
) => { ) =>
Effect.tryPromise({
try: async () => {
const repoUrl = buildRepoUrl(config); const repoUrl = buildRepoUrl(config);
const env = await buildEnv(config, options.organizationId, deps); const env = await buildEnv(config, options.organizationId, deps);
@ -210,4 +213,6 @@ export const backup = async (
exitCode: res.exitCode, exitCode: res.exitCode,
warningDetails: stderrLines.length > 0 ? stderrLines.join("\n") : null, warningDetails: stderrLines.length > 0 ? stderrLines.join("\n") : null,
}; };
}; },
catch: (error) => (error instanceof Error ? error : new Error(String(error))),
});