Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
-- CreateTable
CREATE TABLE "NotificationDeliveryAttempt" (
"id" TEXT NOT NULL PRIMARY KEY,
"outboxId" TEXT NOT NULL,
"attemptNumber" INTEGER NOT NULL,
"outcome" TEXT NOT NULL,
"failureCategory" TEXT,
"errorMessage" TEXT,
"startedAt" DATETIME NOT NULL,
"completedAt" DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP,
"durationMs" INTEGER,
CONSTRAINT "NotificationDeliveryAttempt_outboxId_fkey" FOREIGN KEY ("outboxId") REFERENCES "NotificationOutbox" ("id") ON DELETE RESTRICT ON UPDATE CASCADE
);

-- CreateIndex
CREATE INDEX "NotificationDeliveryAttempt_outboxId_idx" ON "NotificationDeliveryAttempt"("outboxId");

-- CreateIndex
CREATE INDEX "NotificationDeliveryAttempt_outcome_idx" ON "NotificationDeliveryAttempt"("outcome");

-- CreateIndex
CREATE INDEX "NotificationDeliveryAttempt_failureCategory_idx" ON "NotificationDeliveryAttempt"("failureCategory");

-- CreateIndex
CREATE INDEX "NotificationDeliveryAttempt_startedAt_idx" ON "NotificationDeliveryAttempt"("startedAt");
29 changes: 29 additions & 0 deletions app/backend/prisma/schema.prisma
Original file line number Diff line number Diff line change
Expand Up @@ -633,12 +633,41 @@ model NotificationOutbox {
createdAt DateTime @default(now())
updatedAt DateTime @updatedAt

attempts NotificationDeliveryAttempt[]

@@index([status])
@@index([recipient])
@@index([scheduledFor])
@@index([createdAt])
}

/// Records every individual delivery attempt for a NotificationOutbox
/// record (issue #716). NotificationOutbox only ever holds the *latest*
/// attempt (lastAttemptAt/lastError get overwritten on each retry), so
/// this table is what actually makes a status timeline possible.
enum DeliveryAttemptOutcome {
success
failed
}

model NotificationDeliveryAttempt {
id String @id @default(cuid())
outboxId String
outbox NotificationOutbox @relation(fields: [outboxId], references: [id])
attemptNumber Int
outcome DeliveryAttemptOutcome
failureCategory String?
errorMessage String?
startedAt DateTime
completedAt DateTime @default(now())
durationMs Int?

@@index([outboxId])
@@index([outcome])
@@index([failureCategory])
@@index([startedAt])
}

/// Stores idempotency keys to prevent duplicate requests
model IdempotencyKey {
id String @id @default(cuid())
Expand Down
40 changes: 40 additions & 0 deletions app/backend/src/notifications/notification-failure-classifier.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/**
* Classifies a raw notification-delivery error into a small, bounded set
* of categories (issue #716).
*
* This exists specifically to avoid the cardinality-explosion pattern
* already present elsewhere in this codebase (e.g. MetricsService's
* incrementCallbackFailure/incrementTxSubmissionFailure, which pass raw,
* unbounded error text as a Prometheus label). A fixed category set keeps
* both the metric and the persisted failureCategory column meaningful for
* filtering/aggregation, while errorMessage still keeps the full raw text
* for debugging.
*/
export type NotificationFailureCategory =
| 'timeout'
| 'rate_limited'
| 'invalid_recipient'
| 'provider_error'
| 'unknown';

export function classifyNotificationFailure(
error: unknown,
): NotificationFailureCategory {
const message = (
error instanceof Error ? error.message : String(error)
).toLowerCase();

if (/timed?\s?out|timeout|etimedout/.test(message)) {
return 'timeout';
}
if (/rate.?limit|429|too many requests/.test(message)) {
return 'rate_limited';
}
if (/invalid (recipient|email|phone|address)|malformed|bad recipient/.test(message)) {
return 'invalid_recipient';
}
if (/5\d{2}\b|provider error|upstream error|service unavailable/.test(message)) {
return 'provider_error';
}
return 'unknown';
}
12 changes: 10 additions & 2 deletions app/backend/src/notifications/notifications.processor.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,11 @@ describe('NotificationProcessor', () => {
update: jest.Mock;
};
};
let metricsMock: { incrementCallbackFailure: jest.Mock };
let metricsMock: {
incrementCallbackFailure: jest.Mock;
incrementNotificationDeliveryAttempt: jest.Mock;
incrementNotificationDeliveryFailureByCategory: jest.Mock;
};

const makeJob = (
overrides: Partial<{
Expand Down Expand Up @@ -42,7 +46,11 @@ describe('NotificationProcessor', () => {
update: jest.fn().mockResolvedValue({}),
},
};
metricsMock = { incrementCallbackFailure: jest.fn() };
metricsMock = {
incrementCallbackFailure: jest.fn(),
incrementNotificationDeliveryAttempt: jest.fn(),
incrementNotificationDeliveryFailureByCategory: jest.fn(),
};

const module: TestingModule = await Test.createTestingModule({
providers: [
Expand Down
60 changes: 60 additions & 0 deletions app/backend/src/notifications/notifications.processor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { PrismaService } from '../prisma/prisma.service';

import { DlqService } from '../jobs/dlq.service';
import { MetricsService } from '../observability/metrics/metrics.service';
import { classifyNotificationFailure } from './notification-failure-classifier';

@Processor('notifications', {
concurrency: parseInt(process.env.QUEUE_CONCURRENCY || '5'),
Expand Down Expand Up @@ -104,6 +105,32 @@ export class NotificationProcessor extends WorkerHost {
`Failed to update outbox record ${job.data.outboxId} to sent: ${err instanceof Error ? err.message : String(err)}`,
);
}

this.metricsService.incrementNotificationDeliveryAttempt(
job.data.type,
'success',
);

try {
const startedAt = job.processedOn ? new Date(job.processedOn) : new Date();
const completedAt = new Date();
await this.prisma.notificationDeliveryAttempt.create({
data: {
outboxId: job.data.outboxId,
attemptNumber: job.attemptsMade + 1,
outcome: 'success',
startedAt,
completedAt,
durationMs: completedAt.getTime() - startedAt.getTime(),
},
});
} catch (err) {
// Swallow — worker events must not throw. The outbox status update
// above is the source of truth; this is best-effort history.
this.logger.error(
`Failed to record delivery attempt for outbox ${job.data.outboxId}: ${err instanceof Error ? err.message : String(err)}`,
);
}
}

@OnWorkerEvent('failed')
Expand Down Expand Up @@ -149,5 +176,38 @@ export class NotificationProcessor extends WorkerHost {
`Failed to update outbox record ${job.data.outboxId} to ${status}: ${err instanceof Error ? err.message : String(err)}`,
);
}

const failureCategory = classifyNotificationFailure(error);
this.metricsService.incrementNotificationDeliveryAttempt(
job.data.type,
'failed',
);
this.metricsService.incrementNotificationDeliveryFailureByCategory(
job.data.type,
failureCategory,
);

try {
const startedAt = job.processedOn ? new Date(job.processedOn) : new Date();
const completedAt = new Date();
await this.prisma.notificationDeliveryAttempt.create({
data: {
outboxId: job.data.outboxId,
attemptNumber: job.attemptsMade,
outcome: 'failed',
failureCategory,
errorMessage: error.message,
startedAt,
completedAt,
durationMs: completedAt.getTime() - startedAt.getTime(),
},
});
} catch (err) {
// Swallow — worker events must not throw. The outbox status update
// above is the source of truth; this is best-effort history.
this.logger.error(
`Failed to record delivery attempt for outbox ${job.data.outboxId}: ${err instanceof Error ? err.message : String(err)}`,
);
}
}
}
67 changes: 66 additions & 1 deletion app/backend/src/notifications/notifications.service.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
import { Injectable, Logger } from '@nestjs/common';
import { InjectQueue } from '@nestjs/bullmq';
import { Queue } from 'bullmq';
import { AuditLog, NotificationOutbox } from '@prisma/client';
import {
AuditLog,
NotificationOutbox,
NotificationDeliveryAttempt,
DeliveryAttemptOutcome,
} from '@prisma/client';
import {
NotificationJobData,
NotificationType,
Expand Down Expand Up @@ -151,6 +156,66 @@ export class NotificationsService {
return this.prisma.notificationOutbox.findUnique({ where: { id } });
}

/**
* Returns the full delivery-attempt timeline for a single outbox record,
* newest first (issue #716). NotificationOutbox only ever holds the
* latest attempt's outcome; this is what makes a real timeline possible.
*/
async getDeliveryAttempts(
outboxId: string,
): Promise<NotificationDeliveryAttempt[]> {
return this.prisma.notificationDeliveryAttempt.findMany({
where: { outboxId },
orderBy: { startedAt: 'desc' },
});
}

/**
* Returns a filtered, paginated slice of delivery attempts across all
* outbox records, for the admin delivery-history endpoint (issue #716).
*/
async getDeliveryHistory(filters: {
outcome?: DeliveryAttemptOutcome;
failureCategory?: string;
type?: string;
from?: Date;
to?: Date;
limit?: number;
offset?: number;
}): Promise<{ items: NotificationDeliveryAttempt[]; total: number }> {
const limit = Math.min(Math.max(filters.limit ?? 50, 1), 200);
const offset = Math.max(filters.offset ?? 0, 0);

const where: {
outcome?: DeliveryAttemptOutcome;
failureCategory?: string;
startedAt?: { gte?: Date; lte?: Date };
outbox?: { type: string };
} = {};

if (filters.outcome) where.outcome = filters.outcome;
if (filters.failureCategory) where.failureCategory = filters.failureCategory;
if (filters.type) where.outbox = { type: filters.type };
if (filters.from || filters.to) {
where.startedAt = {
...(filters.from ? { gte: filters.from } : {}),
...(filters.to ? { lte: filters.to } : {}),
};
}

const [items, total] = await this.prisma.$transaction([
this.prisma.notificationDeliveryAttempt.findMany({
where,
orderBy: { startedAt: 'desc' },
take: limit,
skip: offset,
}),
this.prisma.notificationDeliveryAttempt.count({ where }),
]);

return { items, total };
}

/**
* Returns all outbox records stuck in pending or enqueued status for more
* than 10 minutes, ordered by scheduledFor ascending (oldest first).
Expand Down
75 changes: 75 additions & 0 deletions app/backend/src/notifications/outbox.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {
Controller,
Get,
Param,
Query,
NotFoundException,
UseGuards,
HttpCode,
Expand All @@ -17,6 +18,7 @@ import {
ApiBearerAuth,
} from '@nestjs/swagger';
import { NotificationsService } from './notifications.service';
import { DeliveryAttemptOutcome } from '@prisma/client';
import { ApiKeyGuard } from '../common/guards/api-key.guard';
import { RolesGuard } from '../auth/roles.guard';
import { Roles } from '../auth/roles.decorator';
Expand Down Expand Up @@ -53,6 +55,50 @@ export class OutboxController {
return ApiResponseDto.ok(records, 'Stuck outbox records fetched');
}

/**
* GET /notifications/outbox/delivery-attempts
* Returns a filtered, paginated slice of delivery attempts across all
* outbox records. Requires admin or operator role (issue #716).
*
* NOTE: nested under /notifications/outbox (not a bare top-level
* /notifications/delivery-attempts) because this controller's
* @Controller() prefix is already 'notifications/outbox', and a bare
* @Get() here would collide with the existing listStuck() route above.
*/
@Get('delivery-attempts')
@Roles(AppRole.admin, AppRole.operator)
@HttpCode(HttpStatus.OK)
@ApiOperation({
summary: 'List notification delivery attempts',
description:
'Returns a filtered, paginated slice of NotificationDeliveryAttempt records, newest first.',
})
@ApiOkResponse({ description: 'Delivery attempts returned.' })
@ApiUnauthorizedResponse({ description: 'Missing or invalid API key.' })
@ApiForbiddenResponse({
description: 'Insufficient role (requires admin or operator).',
})
async listDeliveryAttempts(
@Query('outcome') outcome?: DeliveryAttemptOutcome,
@Query('failureCategory') failureCategory?: string,
@Query('type') type?: string,
@Query('from') from?: string,
@Query('to') to?: string,
@Query('limit') limit?: string,
@Query('offset') offset?: string,
) {
const result = await this.notificationsService.getDeliveryHistory({
outcome,
failureCategory,
type,
from: from ? new Date(from) : undefined,
to: to ? new Date(to) : undefined,
limit: limit ? parseInt(limit, 10) : undefined,
offset: offset ? parseInt(offset, 10) : undefined,
});
return ApiResponseDto.ok(result, 'Delivery attempts fetched');
}

/**
* GET /notifications/outbox/:id
* Returns a single outbox record by id. Requires admin or operator role.
Expand All @@ -77,4 +123,33 @@ export class OutboxController {
}
return ApiResponseDto.ok(record, 'Outbox record fetched');
}

/**
* GET /notifications/outbox/:id/attempts
* Returns the full delivery-attempt timeline for one outbox record,
* newest first. Requires admin or operator role (issue #716).
*/
@Get(':id/attempts')
@Roles(AppRole.admin, AppRole.operator)
@HttpCode(HttpStatus.OK)
@ApiOperation({
summary: 'List delivery attempts for a notification outbox record',
description:
'Returns every NotificationDeliveryAttempt row for the given outbox id, newest first.',
})
@ApiOkResponse({ description: 'Delivery attempts returned.' })
@ApiNotFoundResponse({ description: 'Outbox record not found.' })
@ApiUnauthorizedResponse({ description: 'Missing or invalid API key.' })
@ApiForbiddenResponse({
description: 'Insufficient role (requires admin or operator).',
})
async getAttempts(@Param('id') id: string) {
const record = await this.notificationsService.getOutboxRecord(id);
if (!record) {
throw new NotFoundException(`Outbox record with id "${id}" not found`);
}
const attempts = await this.notificationsService.getDeliveryAttempts(id);
return ApiResponseDto.ok(attempts, 'Delivery attempts fetched');
}
}

Loading