refactor(backup): move retry delay minutes conversion at the contract level

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,
 	};

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", () => {

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<ListBackupSchedulesResponseDto>(schedules, 200);
+		return c.json<ListBackupSchedulesResponseDto>(listBackupSchedulesResponse.parse(schedules), 200);
 	})
 	.get("/:shortId", getBackupScheduleDto, async (c) => {
 		const shortId = asShortId(c.req.param("shortId"));
 		const schedule = await getScheduleByIdOrShortId(shortId);
 
-		return c.json<GetBackupScheduleDto>(schedule, 200);
+		return c.json<GetBackupScheduleDto>(getBackupScheduleResponse.parse(schedule), 200);
 	})
 	.get("/volume/:volumeShortId", getBackupScheduleForVolumeDto, async (c) => {
 		const volumeShortId = asShortId(c.req.param("volumeShortId"));

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(),
+	maxRetries: z.number().min(0).max(32).default(2).optional(),
-	retryDelay: z.number().min(1).max(1440).optional(),
+	retryDelay: z
+		.number()
+		.min(1)
+		.max(1440)
+		.default(15)
+		.transform((minutes) => minutes * 60000),
 });
 
 export type CreateBackupScheduleBody = z.infer<typeof createBackupScheduleBody>;
@@ -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<typeof updateBackupScheduleBody>;

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();
 

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
+	let errorNotificationMessage = `${errorDetails}`;
-		? `${errorDetails}`
+	if (!manual && currentRetryCount > 0) {
-		: `${errorDetails}\n\nFailed after ${maxRetries} retry attempts.`;
+		errorNotificationMessage = `${errorDetails}\n\nFailed after ${currentRetryCount} retry attempts.`;
+	}
 
 	notificationsService
 		.sendBackupNotification(scheduleId, "failure", {