Skip to content
Merged
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
30 changes: 27 additions & 3 deletions backend/src/invoices/invoices.service.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
import { BadRequestException, Injectable } from '@nestjs/common';
import {
BadRequestException,
ConflictException,
Injectable,
} from '@nestjs/common';
import { GenerateInvoiceProvider } from './providers/generate-invoice.provider';
import { FindInvoicesProvider } from './providers/find-invoices.provider';
import { PdfInvoiceProvider } from './providers/pdf-invoice.provider';
Expand Down Expand Up @@ -69,8 +73,28 @@ export class InvoicesService {

this.statusTransitionProvider.validateTransition(invoice.status, to);

invoice.status = to;
return this.invoicesRepository.save(invoice);
if (invoice.status === to) {
return invoice;
}

const result = await this.invoicesRepository
.createQueryBuilder()
.update(Invoice)
.set({ status: to })
.where('id = :id AND status = :currentStatus', {
id: invoiceId,
currentStatus: invoice.status,
})
.returning('*')
.execute();

if (result.affected !== 1) {
throw new ConflictException(
`Invoice "${invoiceId}" was updated by another request`,
);
}

return this.invoicesRepository.create(result.raw[0]);
}

validateTransition(from: InvoiceStatus, to: InvoiceStatus): void {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Injectable, Logger } from '@nestjs/common';
import { randomUUID } from 'node:crypto';
import { Invoice } from '../entities/invoice.entity';

export interface PdfRetryFailure {
Expand All @@ -8,6 +9,7 @@ export interface PdfRetryFailure {
error: string;
failedAt: Date;
attemptCount: number;
correlationId: string;
}

@Injectable()
Expand All @@ -19,6 +21,7 @@ export class PdfGenerationFallbackProvider {
invoice: Invoice,
error: unknown,
attemptCount: number,
correlationId = randomUUID(),
): void {
const failure: PdfRetryFailure = {
invoiceId: invoice.id,
Expand All @@ -27,14 +30,14 @@ export class PdfGenerationFallbackProvider {
error: error instanceof Error ? error.message : String(error),
failedAt: new Date(),
attemptCount,
correlationId,
};

this.failedQueue.push(failure);

this.logger.error(
`PDF generation permanently failed for invoice ${invoice.invoiceNumber} ` +
`after ${attemptCount} attempts. Queued for manual retry. ` +
`Error: ${failure.error}`,
`PDF generation permanently failed after ${attemptCount} attempts. ` +
`Queued for manual retry (reference ${correlationId}).`,
);
}

Expand Down
79 changes: 75 additions & 4 deletions backend/src/invoices/providers/pdf-generation.provider.spec.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Logger, ServiceUnavailableException } from '@nestjs/common';
import { Test } from '@nestjs/testing';
import {
PdfGenerationProvider,
Expand Down Expand Up @@ -148,14 +149,16 @@ describe('PdfGenerationProvider', () => {
expect(pdfInvoiceProvider.generate).toHaveBeenCalledTimes(3);
});

it('queues permanent failure after exhausting retries', async () => {
it('queues permanent failure and returns an actionable error', async () => {
const transientErr = Object.assign(new Error('timeout'), {
code: 'ETIMEDOUT',
});
pdfInvoiceProvider.generate.mockRejectedValue(transientErr);

const invoice = makeInvoice();
await expect(provider.generate(invoice)).rejects.toThrow('timeout');
const generation = provider.generate(invoice);
await expect(generation).rejects.toThrow(ServiceUnavailableException);
await expect(generation).rejects.toThrow(/queued for retry.*reference/i);

expect(pdfInvoiceProvider.generate).toHaveBeenCalledTimes(
PDF_RETRY_MAX_ATTEMPTS,
Expand All @@ -165,6 +168,9 @@ describe('PdfGenerationProvider', () => {
expect(queue[0].invoiceId).toBe(invoice.id);
expect(queue[0].invoiceNumber).toBe('INV-00001');
expect(queue[0].attemptCount).toBe(PDF_RETRY_MAX_ATTEMPTS);
expect(queue[0].correlationId).toMatch(
/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i,
);
});

it('does not retry on non-transient error', async () => {
Expand All @@ -174,14 +180,59 @@ describe('PdfGenerationProvider', () => {

const invoice = makeInvoice();
await expect(provider.generate(invoice)).rejects.toThrow(
'invalid invoice data',
ServiceUnavailableException,
);

expect(pdfInvoiceProvider.generate).toHaveBeenCalledTimes(1);
const queue = fallbackProvider.getFailedQueue();
expect(queue).toHaveLength(1);
expect(queue[0].attemptCount).toBe(1);
});

it('does not return an empty buffer as a successful PDF', async () => {
pdfInvoiceProvider.generate.mockResolvedValue(Buffer.alloc(0));

await expect(provider.generate(makeInvoice())).rejects.toThrow(
ServiceUnavailableException,
);
expect(fallbackProvider.getFailedQueue()).toHaveLength(1);
});

it('returns an actionable error and logs only a reference when fallback fails', async () => {
const invoice = makeInvoice();
pdfInvoiceProvider.generate.mockRejectedValue(
new Error('sensitive invoice details'),
);
jest
.spyOn(fallbackProvider, 'handlePermanentFailure')
.mockImplementation(() => {
throw new Error('fallback internals');
});
const logSpy = jest
.spyOn(Logger.prototype, 'error')
.mockImplementation(() => undefined);

let thrown: unknown;
try {
await provider.generate(invoice);
} catch (error) {
thrown = error;
}

expect(thrown).toBeInstanceOf(ServiceUnavailableException);
expect(thrown).toMatchObject({
message: expect.stringMatching(
/Retry later or contact support with reference [0-9a-f-]+/i,
),
});
expect(logSpy).toHaveBeenCalledTimes(1);
const logMessage = logSpy.mock.calls[0][0] as string;
expect(logMessage).toMatch(/reference [0-9a-f-]+/i);
expect(logMessage).not.toContain(invoice.id);
expect(logMessage).not.toContain(invoice.invoiceNumber);
expect(logMessage).not.toContain('sensitive invoice details');
expect(logMessage).not.toContain('fallback internals');
});
});

describe('PdfGenerationFallbackProvider', () => {
Expand All @@ -191,9 +242,22 @@ describe('PdfGenerationFallbackProvider', () => {
provider = new PdfGenerationFallbackProvider();
});

afterEach(() => {
jest.restoreAllMocks();
});

it('queues failure on handlePermanentFailure', () => {
const invoice = makeInvoice();
provider.handlePermanentFailure(invoice, new Error('boom'), 3);
const correlationId = 'correlation-123';
const logSpy = jest
.spyOn(Logger.prototype, 'error')
.mockImplementation(() => undefined);
provider.handlePermanentFailure(
invoice,
new Error('boom'),
3,
correlationId,
);

const queue = provider.getFailedQueue();
expect(queue).toHaveLength(1);
Expand All @@ -203,6 +267,13 @@ describe('PdfGenerationFallbackProvider', () => {
expect(queue[0].error).toBe('boom');
expect(queue[0].attemptCount).toBe(3);
expect(queue[0].failedAt).toBeInstanceOf(Date);
expect(queue[0].correlationId).toBe(correlationId);
expect(logSpy).toHaveBeenCalledWith(expect.stringContaining(correlationId));
const logMessage = logSpy.mock.calls[0][0] as string;
expect(logMessage).not.toContain(invoice.id);
expect(logMessage).not.toContain(invoice.invoiceNumber);
expect(logMessage).not.toContain(invoice.userId);
expect(logMessage).not.toContain('boom');
});

it('removeFromQueue removes and returns the failure', () => {
Expand Down
49 changes: 38 additions & 11 deletions backend/src/invoices/providers/pdf-generation.provider.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
import { Injectable, Logger } from '@nestjs/common';
import {
Injectable,
Logger,
ServiceUnavailableException,
} from '@nestjs/common';
import { Invoice } from '../entities/invoice.entity';
import { PdfInvoiceProvider } from './pdf-invoice.provider';
import { PdfGenerationFallbackProvider } from './pdf-generation-fallback.provider';
import { withRetry } from '../../utils/retry.util';
import { randomUUID } from 'node:crypto';

export const PDF_RETRY_MAX_ATTEMPTS = 3;
export const PDF_RETRY_BASE_DELAY_MS = 500;
Expand Down Expand Up @@ -68,30 +73,52 @@ export class PdfGenerationProvider {
) {}

async generate(invoice: Invoice): Promise<Buffer> {
const correlationId = randomUUID();
let attemptCount = 0;

try {
return await withRetry(
() => this.pdfInvoiceProvider.generate(invoice),
async () => {
attemptCount += 1;
const pdf = await this.pdfInvoiceProvider.generate(invoice);
if (!Buffer.isBuffer(pdf) || pdf.length === 0) {
throw new Error('PDF generator returned an empty document');
}
return pdf;
},
{
maxAttempts: PDF_RETRY_MAX_ATTEMPTS,
baseDelayMs: PDF_RETRY_BASE_DELAY_MS,
maxDelayMs: PDF_RETRY_MAX_DELAY_MS,
isRetryable: isTransientPdfError,
onRetry: (error, attempt, nextDelayMs) => {
onRetry: (_error, attempt, nextDelayMs) => {
this.logger.warn(
`PDF generation attempt ${attempt} failed for invoice ` +
`${invoice.invoiceNumber}: ${(error as Error).message}. ` +
`Retrying in ${nextDelayMs}ms...`,
`PDF generation attempt ${attempt} failed ` +
`(reference ${correlationId}). Retrying in ${nextDelayMs}ms...`,
);
},
},
);
} catch (error) {
this.fallbackProvider.handlePermanentFailure(
invoice,
error,
PDF_RETRY_MAX_ATTEMPTS,
try {
this.fallbackProvider.handlePermanentFailure(
invoice,
error,
attemptCount,
correlationId,
);
} catch {
this.logger.error(
`PDF generation and fallback handling failed (reference ${correlationId})`,
);
throw new ServiceUnavailableException(
`Invoice PDF is unavailable. Retry later or contact support with reference ${correlationId}.`,
);
}

throw new ServiceUnavailableException(
`Invoice PDF is unavailable and has been queued for retry. Contact support with reference ${correlationId} if the problem continues.`,
);
throw error;
}
}
}
Loading