fix(notifications): persist delivery health status (#850)
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
Release Workflow / request-docs-version-update (push) Has been cancelled

* fix(notifications): persist delivery health status

* fix: pr feedback double update
This commit is contained in:
Nico 2026-05-02 11:51:16 +02:00 committed by GitHub
parent 3d5a0a9b75
commit f83b765d04
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
16 changed files with 2697 additions and 161 deletions

View file

@ -76,171 +76,154 @@ export const createClient = (config: Config = {}): Client => {
};
const request: Client['request'] = async (options) => {
const { opts, url } = await beforeRequest(options);
const requestInit: ReqInit = {
redirect: 'follow',
...opts,
body: getValidRequestBody(opts),
};
const throwOnError = options.throwOnError ?? _config.throwOnError;
const responseStyle = options.responseStyle ?? _config.responseStyle;
let request = new Request(url, requestInit);
for (const fn of interceptors.request.fns) {
if (fn) {
request = await fn(request, opts);
}
}
// fetch must be assigned here, otherwise it would throw the error:
// TypeError: Failed to execute 'fetch' on 'Window': Illegal invocation
const _fetch = opts.fetch!;
let response: Response;
let request: Request | undefined;
let response: Response | undefined;
try {
response = await _fetch(request);
} catch (error) {
// Handle fetch exceptions (AbortError, network errors, etc.)
let finalError = error;
const { opts, url } = await beforeRequest(options);
const requestInit: ReqInit = {
redirect: 'follow',
...opts,
body: getValidRequestBody(opts),
};
for (const fn of interceptors.error.fns) {
request = new Request(url, requestInit);
for (const fn of interceptors.request.fns) {
if (fn) {
finalError = (await fn(error, undefined as any, request, opts)) as unknown;
request = await fn(request, opts);
}
}
finalError = finalError || ({} as unknown);
// fetch must be assigned here, otherwise it would throw the error:
// TypeError: Failed to execute 'fetch' on 'Window': Illegal invocation
const _fetch = opts.fetch!;
if (opts.throwOnError) {
throw finalError;
response = await _fetch(request);
for (const fn of interceptors.response.fns) {
if (fn) {
response = await fn(response, request, opts);
}
}
// Return error response
return opts.responseStyle === 'data'
? undefined
: {
error: finalError,
request,
response: undefined as any,
};
}
const result = {
request,
response,
};
for (const fn of interceptors.response.fns) {
if (fn) {
response = await fn(response, request, opts);
}
}
if (response.ok) {
const parseAs =
(opts.parseAs === 'auto'
? getParseAs(response.headers.get('Content-Type'))
: opts.parseAs) ?? 'json';
const result = {
request,
response,
};
if (response.status === 204 || response.headers.get('Content-Length') === '0') {
let emptyData: any;
switch (parseAs) {
case 'arrayBuffer':
case 'blob':
case 'text':
emptyData = await response[parseAs]();
break;
case 'formData':
emptyData = new FormData();
break;
case 'stream':
emptyData = response.body;
break;
case 'json':
default:
emptyData = {};
break;
}
return opts.responseStyle === 'data'
? emptyData
: {
data: emptyData,
...result,
};
}
if (response.ok) {
const parseAs =
(opts.parseAs === 'auto'
? getParseAs(response.headers.get('Content-Type'))
: opts.parseAs) ?? 'json';
if (response.status === 204 || response.headers.get('Content-Length') === '0') {
let emptyData: any;
let data: any;
switch (parseAs) {
case 'arrayBuffer':
case 'blob':
case 'text':
emptyData = await response[parseAs]();
break;
case 'formData':
emptyData = new FormData();
case 'text':
data = await response[parseAs]();
break;
case 'json': {
// Some servers return 200 with no Content-Length and empty body.
// response.json() would throw; read as text and parse if non-empty.
const text = await response.text();
data = text ? JSON.parse(text) : {};
break;
}
case 'stream':
emptyData = response.body;
break;
case 'json':
default:
emptyData = {};
break;
return opts.responseStyle === 'data'
? response.body
: {
data: response.body,
...result,
};
}
if (parseAs === 'json') {
if (opts.responseValidator) {
await opts.responseValidator(data);
}
if (opts.responseTransformer) {
data = await opts.responseTransformer(data);
}
}
return opts.responseStyle === 'data'
? emptyData
? data
: {
data: emptyData,
data,
...result,
};
}
let data: any;
switch (parseAs) {
case 'arrayBuffer':
case 'blob':
case 'formData':
case 'text':
data = await response[parseAs]();
break;
case 'json': {
// Some servers return 200 with no Content-Length and empty body.
// response.json() would throw; read as text and parse if non-empty.
const text = await response.text();
data = text ? JSON.parse(text) : {};
break;
}
case 'stream':
return opts.responseStyle === 'data'
? response.body
: {
data: response.body,
...result,
};
const textError = await response.text();
let jsonError: unknown;
try {
jsonError = JSON.parse(textError);
} catch {
// noop
}
if (parseAs === 'json') {
if (opts.responseValidator) {
await opts.responseValidator(data);
}
throw jsonError ?? textError;
} catch (error) {
let finalError = error;
if (opts.responseTransformer) {
data = await opts.responseTransformer(data);
for (const fn of interceptors.error.fns) {
if (fn) {
finalError = await fn(finalError, response, request, options as ResolvedRequestOptions);
}
}
return opts.responseStyle === 'data'
? data
finalError = finalError || {};
if (throwOnError) {
throw finalError;
}
// TODO: we probably want to return error and improve types
return responseStyle === 'data'
? undefined
: {
data,
...result,
error: finalError,
request,
response,
};
}
const textError = await response.text();
let jsonError: unknown;
try {
jsonError = JSON.parse(textError);
} catch {
// noop
}
const error = jsonError ?? textError;
let finalError = error;
for (const fn of interceptors.error.fns) {
if (fn) {
finalError = (await fn(error, response, request, opts)) as string;
}
}
finalError = finalError || ({} as string);
if (opts.throwOnError) {
throw finalError;
}
// TODO: we probably want to return error and improve types
return opts.responseStyle === 'data'
? undefined
: {
error: finalError,
...result,
};
};
const makeMethodFn = (method: Uppercase<HttpMethod>) => (options: RequestOptions) =>
@ -251,7 +234,6 @@ export const createClient = (config: Config = {}): Client => {
return createSseClient({
...opts,
body: opts.body as BodyInit | null | undefined,
headers: opts.headers as unknown as Record<string, string>,
method,
onRequest: async (url, init) => {
let request = new Request(url, init);

View file

@ -94,6 +94,7 @@ export interface ResolvedRequestOptions<
ThrowOnError extends boolean = boolean,
Url extends string = string,
> extends RequestOptions<unknown, TResponseStyle, ThrowOnError, Url> {
headers: Headers;
serializedBody?: string;
}
@ -127,8 +128,10 @@ export type RequestResult<
error: TError extends Record<string, unknown> ? TError[keyof TError] : TError;
}
) & {
request: Request;
response: Response;
/** request may be undefined, because error may be from building the request object itself */
request?: Request;
/** response may be undefined, because error may be from building the request object itself or from a network error */
response?: Response;
}
>;

View file

@ -219,8 +219,10 @@ export const mergeHeaders = (
type ErrInterceptor<Err, Res, Req, Options> = (
error: Err,
response: Res,
request: Req,
/** response may be undefined due to a network error where no response object is produced */
response: Res | undefined,
/** request may be undefined, because error may be from building the request object itself */
request: Req | undefined,
options: Options,
) => Err | Promise<Err>;

View file

@ -3610,6 +3610,9 @@ export type GetScheduleNotificationsResponses = {
id: number;
name: string;
enabled: boolean;
status: 'healthy' | 'error' | 'unknown';
lastChecked: number | null;
lastError: string | null;
type: 'email' | 'slack' | 'discord' | 'gotify' | 'ntfy' | 'pushover' | 'telegram' | 'generic' | 'custom';
config: {
type: 'email';
@ -3711,6 +3714,9 @@ export type UpdateScheduleNotificationsResponses = {
id: number;
name: string;
enabled: boolean;
status: 'healthy' | 'error' | 'unknown';
lastChecked: number | null;
lastError: string | null;
type: 'email' | 'slack' | 'discord' | 'gotify' | 'ntfy' | 'pushover' | 'telegram' | 'generic' | 'custom';
config: {
type: 'email';
@ -4342,6 +4348,9 @@ export type ListNotificationDestinationsResponses = {
id: number;
name: string;
enabled: boolean;
status: 'healthy' | 'error' | 'unknown';
lastChecked: number | null;
lastError: string | null;
type: 'email' | 'slack' | 'discord' | 'gotify' | 'ntfy' | 'pushover' | 'telegram' | 'generic' | 'custom';
config: {
type: 'email';
@ -4485,6 +4494,9 @@ export type CreateNotificationDestinationResponses = {
id: number;
name: string;
enabled: boolean;
status: 'healthy' | 'error' | 'unknown';
lastChecked: number | null;
lastError: string | null;
type: 'email' | 'slack' | 'discord' | 'gotify' | 'ntfy' | 'pushover' | 'telegram' | 'generic' | 'custom';
config: {
type: 'email';
@ -4603,6 +4615,9 @@ export type GetNotificationDestinationResponses = {
id: number;
name: string;
enabled: boolean;
status: 'healthy' | 'error' | 'unknown';
lastChecked: number | null;
lastError: string | null;
type: 'email' | 'slack' | 'discord' | 'gotify' | 'ntfy' | 'pushover' | 'telegram' | 'generic' | 'custom';
config: {
type: 'email';
@ -4756,6 +4771,9 @@ export type UpdateNotificationDestinationResponses = {
id: number;
name: string;
enabled: boolean;
status: 'healthy' | 'error' | 'unknown';
lastChecked: number | null;
lastError: string | null;
type: 'email' | 'slack' | 'discord' | 'gotify' | 'ntfy' | 'pushover' | 'telegram' | 'generic' | 'custom';
config: {
type: 'email';

View file

@ -28,6 +28,7 @@ const invalidatingEvents = new Set<ServerEventType>([
"backup:completed",
"volume:updated",
"volume:status_changed",
"notification:updated",
"mirror:completed",
"doctor:started",
"doctor:completed",

View file

@ -40,6 +40,7 @@ import {
Lock,
Mail,
AtSign,
AlertCircle,
MessageSquare,
Pencil,
Server,
@ -55,6 +56,7 @@ import type { GetNotificationDestinationResponse } from "~/client/api-client/typ
import { useTimeFormat } from "~/client/lib/datetime";
type NotificationConfig = GetNotificationDestinationResponse["config"];
type NotificationStatus = GetNotificationDestinationResponse["status"];
type Props = {
icon: React.ReactNode;
label: string;
@ -72,6 +74,17 @@ function ConfigRow({ icon, label, value, mono }: Props) {
);
}
function getStatusLabel(enabled: boolean, status: NotificationStatus) {
return enabled ? status : "disabled";
}
function getStatusClass(enabled: boolean, status: NotificationStatus) {
if (!enabled) return "bg-gray-500";
if (status === "healthy") return "bg-success";
if (status === "error") return "bg-red-500";
return "bg-yellow-500";
}
function NotificationConfigRows({ config }: { config: NotificationConfig }) {
switch (config.type) {
case "slack":
@ -228,13 +241,8 @@ export function NotificationDetailsPage({ notificationId }: { notificationId: st
<h2 className="text-lg font-semibold tracking-tight">{data.name}</h2>
<Separator orientation="vertical" className="h-4 mx-1" />
<Badge variant="outline" className="capitalize gap-1.5">
<span
className={cn("w-2 h-2 rounded-full shrink-0", {
"bg-success": data.enabled,
"bg-red-500": !data.enabled,
})}
/>
{data.enabled ? "Enabled" : "Disabled"}
<span className={cn("w-2 h-2 rounded-full shrink-0", getStatusClass(data.enabled, data.status))} />
{getStatusLabel(data.enabled, data.status)}
</Badge>
<Badge variant="secondary" className="capitalize">
{data.type}
@ -289,6 +297,14 @@ export function NotificationDetailsPage({ notificationId }: { notificationId: st
<NotificationConfigRows config={data.config} />
</div>
</Card>
<Card className={cn("px-6 py-6", { hidden: !data.lastError })}>
<CardTitle className="flex items-center gap-2 mb-3 text-red-600">
<AlertCircle className="h-4 w-4" />
Last Delivery Error
</CardTitle>
<p className="text-sm text-muted-foreground wrap-break-word">{data.lastError}</p>
</Card>
</div>
<AlertDialog open={showDeleteConfirm} onOpenChange={setShowDeleteConfirm}>

View file

@ -28,6 +28,15 @@ type NotificationRow = {
name: string;
type: "email" | "slack" | "discord" | "gotify" | "ntfy" | "pushover" | "telegram" | "custom" | "generic";
enabled: boolean;
status: "healthy" | "error" | "unknown";
};
const getNotificationStatus = (row: NotificationRow) => (row.enabled ? row.status : "disabled");
const getNotificationStatusVariant = (row: NotificationRow) => {
if (!row.enabled) return "neutral";
if (row.status === "healthy") return "success";
if (row.status === "error") return "error";
return "warning";
};
const notificationColumns: ColumnDef<NotificationRow>[] = [
@ -43,16 +52,13 @@ const notificationColumns: ColumnDef<NotificationRow>[] = [
filterFn: (row, id, value) => row.getValue(id) === value,
},
{
accessorFn: (row) => (row.enabled ? "enabled" : "disabled"),
accessorFn: getNotificationStatus,
id: "status",
header: ({ column }) => (
<DataTableSortHeader column={column} title="Status" sortDirection={column.getIsSorted()} center />
),
cell: ({ row }) => (
<StatusDot
variant={row.original.enabled ? "success" : "neutral"}
label={row.original.enabled ? "Enabled" : "Disabled"}
/>
<StatusDot variant={getNotificationStatusVariant(row.original)} label={getNotificationStatus(row.original)} />
),
filterFn: (row, id, value) => row.getValue(id) === value,
},
@ -138,7 +144,9 @@ export function NotificationsPage() {
<SelectValue placeholder="All status" />
</SelectTrigger>
<SelectContent>
<SelectItem value="enabled">Enabled</SelectItem>
<SelectItem value="healthy">Healthy</SelectItem>
<SelectItem value="error">Error</SelectItem>
<SelectItem value="unknown">Unknown</SelectItem>
<SelectItem value="disabled">Disabled</SelectItem>
</SelectContent>
</Select>

View file

@ -0,0 +1,3 @@
ALTER TABLE `notification_destinations_table` ADD `status` text DEFAULT 'unknown' NOT NULL;--> statement-breakpoint
ALTER TABLE `notification_destinations_table` ADD `last_checked` integer;--> statement-breakpoint
ALTER TABLE `notification_destinations_table` ADD `last_error` text;

File diff suppressed because it is too large Load diff

View file

@ -41,6 +41,12 @@ const serverEventPayloads = {
"volume:unmounted": payload<{ organizationId: string; volumeName: string }>(),
"volume:updated": payload<{ organizationId: string; volumeName: string }>(),
"volume:status_changed": payload<{ organizationId: string; volumeName: string; status: string }>(),
"notification:updated": payload<{
organizationId: string;
notificationId: number;
notificationName: string;
status: "healthy" | "error" | "unknown";
}>(),
"doctor:started": payload<{ organizationId: string; repositoryId: string; repositoryName: string }>(),
"doctor:completed": payload<
{

View file

@ -337,6 +337,9 @@ export const notificationDestinationsTable = sqliteTable("notification_destinati
id: int().primaryKey({ autoIncrement: true }),
name: text().notNull(),
enabled: int("enabled", { mode: "boolean" }).notNull().default(true),
status: text().$type<"healthy" | "error" | "unknown">().notNull().default("unknown"),
lastChecked: int("last_checked", { mode: "number" }),
lastError: text("last_error"),
type: text().$type<NotificationType>().notNull(),
config: text("config", { mode: "json" }).$type<NotificationConfig>().notNull(),
createdAt: int("created_at", { mode: "number" })

View file

@ -20,6 +20,7 @@ const broadcastEvents = [
"volume:mounted",
"volume:unmounted",
"volume:updated",
"notification:updated",
"mirror:started",
"mirror:completed",
"restore:started",

View file

@ -0,0 +1,86 @@
import { afterEach, describe, expect, test, vi } from "vitest";
import { db } from "~/server/db/db";
import { notificationDestinationsTable } from "~/server/db/schema";
import { withContext } from "~/server/core/request-context";
import { createTestSession } from "~/test/helpers/auth";
import * as shoutrrr from "~/server/utils/shoutrrr";
import { notificationsService } from "../notifications.service";
import { serverEvents } from "~/server/core/events";
afterEach(() => {
vi.restoreAllMocks();
});
describe("notificationsService.testDestination", () => {
test("marks the destination as error when delivery fails", async () => {
const { organizationId, user } = await createTestSession();
await withContext({ organizationId, userId: user.id }, async () => {
const destination = await notificationsService.createDestination("Broken webhook", {
type: "custom",
shoutrrrUrl: "discord://token@webhookid",
});
vi.spyOn(shoutrrr, "sendNotification").mockResolvedValue({ success: false, error: "webhook rejected" });
const event = new Promise<Parameters<typeof serverEvents.emit>[1]>((resolve) => {
serverEvents.once("notification:updated", resolve);
});
await expect(notificationsService.testDestination(destination.id)).rejects.toThrow("webhook rejected");
await expect(event).resolves.toEqual(
expect.objectContaining({
organizationId,
notificationId: destination.id,
notificationName: "Broken webhook",
status: "error",
}),
);
const updated = await db.query.notificationDestinationsTable.findFirst({
where: { id: destination.id },
});
expect(updated).toEqual(
expect.objectContaining({
status: "error",
lastChecked: expect.any(Number),
lastError: expect.stringContaining("webhook rejected"),
}),
);
});
});
test("marks the destination as healthy when delivery succeeds", async () => {
const { organizationId, user } = await createTestSession();
await withContext({ organizationId, userId: user.id }, async () => {
const [destination] = await db
.insert(notificationDestinationsTable)
.values({
name: "Recovered webhook",
type: "custom",
config: { type: "custom", shoutrrrUrl: "discord://token@webhookid" },
organizationId,
status: "error",
lastError: "previous failure",
})
.returning();
vi.spyOn(shoutrrr, "sendNotification").mockResolvedValue({ success: true });
await expect(notificationsService.testDestination(destination.id)).resolves.toEqual({ success: true });
const updated = await db.query.notificationDestinationsTable.findFirst({
where: { id: destination.id },
});
expect(updated).toEqual(
expect.objectContaining({
status: "healthy",
lastChecked: expect.any(Number),
lastError: null,
}),
);
});
});
});

View file

@ -6,6 +6,9 @@ const notificationDestinationSchema = z.object({
id: z.number(),
name: z.string(),
enabled: z.boolean(),
status: z.enum(["healthy", "error", "unknown"]),
lastChecked: z.number().nullable(),
lastError: z.string().nullable(),
type: z.enum(NOTIFICATION_TYPES),
config: notificationConfigSchema,
createdAt: z.number(),

View file

@ -18,6 +18,7 @@ import { config } from "~/server/core/config";
import { getOrganizationId } from "~/server/core/request-context";
import { formatBytes } from "~/utils/format-bytes";
import { decryptNotificationConfig, encryptNotificationConfig } from "./notification-config-secrets";
import { serverEvents } from "~/server/core/events";
const getCustomShoutrrrWebhookUrl = (shoutrrrUrl: string) => {
if (!URL.canParse(shoutrrrUrl)) {
@ -188,21 +189,51 @@ const deleteDestination = async (id: number) => {
);
};
const updateDeliveryStatus = async (destinationId: number, result: { success: boolean; error?: string }) => {
const [updated] = await db
.update(notificationDestinationsTable)
.set({
status: result.success ? "healthy" : "error",
lastChecked: Date.now(),
lastError: result.success ? null : (result.error ?? "Unknown error"),
updatedAt: Date.now(),
})
.where(eq(notificationDestinationsTable.id, destinationId))
.returning();
if (updated) {
serverEvents.emit("notification:updated", {
organizationId: updated.organizationId,
notificationId: updated.id,
notificationName: updated.name,
status: updated.status,
});
}
};
const testDestination = async (id: number) => {
const destination = await getDestination(id);
let result: Awaited<ReturnType<typeof sendNotification>>;
const decryptedConfig = await decryptNotificationConfig(destination.config);
assertNotificationWebhookOriginAllowed(decryptedConfig);
try {
const decryptedConfig = await decryptNotificationConfig(destination.config);
assertNotificationWebhookOriginAllowed(decryptedConfig);
const shoutrrrUrl = buildShoutrrrUrl(decryptedConfig);
const shoutrrrUrl = buildShoutrrrUrl(decryptedConfig);
logger.debug("Testing notification with Shoutrrr URL:", shoutrrrUrl);
logger.debug("Testing notification with Shoutrrr URL:", shoutrrrUrl);
const result = await sendNotification({
shoutrrrUrl,
title: "Zerobyte Test Notification",
body: `This is a test notification from Zerobyte for destination: ${destination.name}`,
});
result = await sendNotification({
shoutrrrUrl,
title: "Zerobyte Test Notification",
body: `This is a test notification from Zerobyte for destination: ${destination.name}`,
});
} catch (error) {
await updateDeliveryStatus(destination.id, { success: false, error: toMessage(error) });
throw error;
}
await updateDeliveryStatus(destination.id, result);
if (!result.success) {
throw new InternalServerError(`Failed to send test notification: ${result.error}`);
@ -394,6 +425,7 @@ const sendBackupNotification = async (
const shoutrrrUrl = buildShoutrrrUrl(decryptedConfig);
const result = await sendNotification({ shoutrrrUrl, title, body });
await updateDeliveryStatus(assignment.destination.id, result);
if (result.success) {
logger.info(
@ -405,6 +437,7 @@ const sendBackupNotification = async (
);
}
} catch (error) {
await updateDeliveryStatus(assignment.destination.id, { success: false, error: toMessage(error) });
logger.error(
`Error sending notification to ${assignment.destination.name} for backup ${scheduleId}: ${toMessage(error)}`,
);

View file

@ -5,12 +5,12 @@ export default defineConfig({
input: `http://${config.serverIp}:3000/api/v1/openapi.json`,
output: {
path: "./app/client/api-client",
header: [
"// @ts-nocheck",
"// This file is auto-generated by @hey-api/openapi-ts",
],
header: ["// @ts-nocheck", "// This file is auto-generated by @hey-api/openapi-ts"],
postProcess: [
"oxfmt",
{
command: "vp",
args: ["fmt", "--write", "{{path}}"],
},
{ command: "bun", args: ["scripts/patch-api-client.ts"] },
],
},