diff --git a/app/client/modules/backups/components/create-schedule-form/utils.ts b/app/client/modules/backups/components/create-schedule-form/utils.ts index f8bbd036..bae1367d 100644 --- a/app/client/modules/backups/components/create-schedule-form/utils.ts +++ b/app/client/modules/backups/components/create-schedule-form/utils.ts @@ -27,7 +27,7 @@ export const backupScheduleToFormValues = (schedule?: BackupSchedule): InternalF oneFileSystem: schedule.oneFileSystem ?? false, customResticParamsText: schedule.customResticParams?.join("\n") ?? "", maxRetries: schedule.maxRetries, - retryDelay: schedule.retryDelay ? schedule.retryDelay / (60 * 1000) : undefined, // Convert ms to minutes + retryDelay: schedule.retryDelay, ...cronValues, ...schedule.retentionPolicy, }; diff --git a/app/server/modules/backups/__tests__/backups.execution.test.ts b/app/server/modules/backups/__tests__/backups.execution.test.ts index ccb0df7f..fbb20003 100644 --- a/app/server/modules/backups/__tests__/backups.execution.test.ts +++ b/app/server/modules/backups/__tests__/backups.execution.test.ts @@ -11,10 +11,12 @@ import { TEST_ORG_ID } from "~/test/helpers/organization"; import * as context from "~/server/core/request-context"; import * as spawnModule from "@zerobyte/core/node"; import type { SafeSpawnParams } from "@zerobyte/core/node"; +import { logger } from "@zerobyte/core/node"; import { restic } from "~/server/core/restic"; import { NotFoundError, BadRequestError } from "http-errors-enhanced"; import { repositoriesService } from "~/server/modules/repositories/repositories.service"; import { repoMutex } from "~/server/core/repository-mutex"; +import { notificationsService } from "~/server/modules/notifications/notifications.service"; const setup = () => { const resticBackupMock = vi.fn((_: SafeSpawnParams) => @@ -86,6 +88,52 @@ describe("backup execution - validation failures", () => { expect(result.error.message).toBe("Backup schedule not found"); } }); + + test("does not claim retries when none were scheduled", async () => { + const { resticBackupMock } = setup(); + const notificationSpy = vi.spyOn(notificationsService, "sendBackupNotification").mockResolvedValue(); + const volume = await createTestVolume(); + const repository = await createTestRepository(); + const schedule = await createTestBackupSchedule({ + volumeId: volume.id, + repositoryId: repository.id, + cronExpression: "* * * * *", + maxRetries: 2, + retryDelay: 15 * 60 * 1000, + }); + + resticBackupMock.mockImplementationOnce(() => + Promise.resolve({ exitCode: 1, summary: generateBackupOutput(), error: "failed" }), + ); + + await backupsExecutionService.executeBackup(schedule.id); + + expect(notificationSpy).toHaveBeenCalled(); + expect(notificationSpy.mock.calls.at(-1)?.[2]?.error).toBe("failed"); + }); + + test("does not log an invalid cron error for manual-only failures", async () => { + const { resticBackupMock } = setup(); + const errorSpy = vi.spyOn(logger, "error").mockImplementation(() => {}); + const volume = await createTestVolume(); + const repository = await createTestRepository(); + const schedule = await createTestBackupSchedule({ + volumeId: volume.id, + repositoryId: repository.id, + enabled: false, + cronExpression: "", + }); + + resticBackupMock.mockImplementationOnce(() => + Promise.resolve({ exitCode: 1, summary: generateBackupOutput(), error: "manual failure" }), + ); + + await backupsExecutionService.executeBackup(schedule.id, true); + + expect( + errorSpy.mock.calls.some(([message]) => String(message).includes('Failed to parse cron expression ""')), + ).toBe(false); + }); }); describe("stop backup", () => { diff --git a/app/server/modules/backups/backups.controller.ts b/app/server/modules/backups/backups.controller.ts index 9a47a756..ffcf2bae 100644 --- a/app/server/modules/backups/backups.controller.ts +++ b/app/server/modules/backups/backups.controller.ts @@ -33,6 +33,8 @@ import { type GetMirrorCompatibilityDto, type ReorderBackupSchedulesDto, type GetBackupProgressDto, + listBackupSchedulesResponse, + getBackupScheduleResponse, } from "./backups.dto"; import { backupsService } from "./backups.service"; import { @@ -54,13 +56,13 @@ export const backupScheduleController = new Hono() .get("/", listBackupSchedulesDto, async (c) => { const schedules = await backupsService.listSchedules(); - return c.json(schedules, 200); + return c.json(listBackupSchedulesResponse.parse(schedules), 200); }) .get("/:shortId", getBackupScheduleDto, async (c) => { const shortId = asShortId(c.req.param("shortId")); const schedule = await getScheduleByIdOrShortId(shortId); - return c.json(schedule, 200); + return c.json(getBackupScheduleResponse.parse(schedule), 200); }) .get("/volume/:volumeShortId", getBackupScheduleForVolumeDto, async (c) => { const volumeShortId = asShortId(c.req.param("volumeShortId")); diff --git a/app/server/modules/backups/backups.dto.ts b/app/server/modules/backups/backups.dto.ts index 6e68aed5..dbaa580a 100644 --- a/app/server/modules/backups/backups.dto.ts +++ b/app/server/modules/backups/backups.dto.ts @@ -32,7 +32,7 @@ const backupScheduleSchema = z.object({ oneFileSystem: z.boolean(), customResticParams: z.array(z.string()).nullable(), maxRetries: z.number(), - retryDelay: z.number(), + retryDelay: z.number().transform((ms) => Math.round(ms / 60000)), lastBackupAt: z.number().nullable(), lastBackupStatus: z.enum(["success", "error", "in_progress", "warning"]).nullable(), lastBackupError: z.string().nullable(), @@ -130,8 +130,13 @@ export const createBackupScheduleBody = z.object({ oneFileSystem: z.boolean().optional(), tags: z.array(z.string()).optional(), customResticParams: z.array(z.string()).optional(), - maxRetries: z.number().min(0).max(32).optional(), - retryDelay: z.number().min(1).max(1440).optional(), + maxRetries: z.number().min(0).max(32).default(2).optional(), + retryDelay: z + .number() + .min(1) + .max(1440) + .default(15) + .transform((minutes) => minutes * 60000), }); export type CreateBackupScheduleBody = z.infer; @@ -170,7 +175,12 @@ export const updateBackupScheduleBody = z.object({ tags: z.array(z.string()).optional(), customResticParams: z.array(z.string()).optional(), maxRetries: z.number().min(0).max(32).optional(), - retryDelay: z.number().min(1).max(1440).optional(), + retryDelay: z + .number() + .min(1) + .max(1440) + .default(15) + .transform((minutes) => minutes * 60000), }); export type UpdateBackupScheduleBody = z.infer; diff --git a/app/server/modules/backups/backups.service.ts b/app/server/modules/backups/backups.service.ts index 1699656d..a01047b3 100644 --- a/app/server/modules/backups/backups.service.ts +++ b/app/server/modules/backups/backups.service.ts @@ -112,7 +112,7 @@ const createSchedule = async (data: CreateBackupScheduleBody) => { nextBackupAt: nextBackupAt, shortId: generateShortId(), maxRetries: data.maxRetries, - retryDelay: data.retryDelay ? data.retryDelay * 60 * 1000 : undefined, + retryDelay: data.retryDelay, organizationId, }) .returning(); @@ -165,11 +165,10 @@ const updateSchedule = async (scheduleIdOrShortId: number | string, data: Update const cronExpression = data.cronExpression ?? schedule.cronExpression; const nextBackupAt = data.cronExpression === "" ? null : data.cronExpression ? calculateNextRun(cronExpression) : schedule.nextBackupAt; - const retryDelay = data.retryDelay ? data.retryDelay * 60 * 1000 : schedule.retryDelay; const [updated] = await db .update(backupSchedulesTable) - .set({ ...data, repositoryId: repository.id, nextBackupAt, updatedAt: Date.now(), retryDelay }) + .set({ ...data, repositoryId: repository.id, nextBackupAt, updatedAt: Date.now() }) .where(and(eq(backupSchedulesTable.id, schedule.id), eq(backupSchedulesTable.organizationId, organizationId))) .returning(); diff --git a/app/server/modules/backups/helpers/backup-lifecycle.ts b/app/server/modules/backups/helpers/backup-lifecycle.ts index bc9c74d6..4bb33b0f 100644 --- a/app/server/modules/backups/helpers/backup-lifecycle.ts +++ b/app/server/modules/backups/helpers/backup-lifecycle.ts @@ -231,9 +231,9 @@ export async function handleBackupFailure( const maxRetries = schedule.maxRetries; const shouldRetry = currentRetryCount < maxRetries; const nextRetryBackupAt = Date.now() + schedule.retryDelay; - const nextScheduledBackupAt = calculateNextRun(schedule.cronExpression); + const nextScheduledBackupAt = schedule.cronExpression ? calculateNextRun(schedule.cronExpression) : null; - if (!manual && shouldRetry && nextRetryBackupAt < nextScheduledBackupAt) { + if (!manual && shouldRetry && nextScheduledBackupAt && nextRetryBackupAt < nextScheduledBackupAt) { await scheduleQueries.updateStatus(scheduleId, organizationId, { nextBackupAt: nextRetryBackupAt, failureRetryCount: currentRetryCount + 1, @@ -293,9 +293,10 @@ export async function handleBackupFailure( status: "error", }); - const errorNotificationMessage = manual - ? `${errorDetails}` - : `${errorDetails}\n\nFailed after ${maxRetries} retry attempts.`; + let errorNotificationMessage = `${errorDetails}`; + if (!manual && currentRetryCount > 0) { + errorNotificationMessage = `${errorDetails}\n\nFailed after ${currentRetryCount} retry attempts.`; + } notificationsService .sendBackupNotification(scheduleId, "failure", {