Skip to content

Commit a8f0f98

Browse files
committed
Skip replayed letter event
1 parent 65d727a commit a8f0f98

6 files changed

Lines changed: 66 additions & 17 deletions

File tree

internal/datastore/src/__test__/letter-repository.test.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
import { LetterRepository } from "../letter-repository";
1010
import { InsertLetter, Letter, UpdateLetter } from "../types";
1111
import { LogStream, createTestLogger } from "./logs";
12+
import { LetterAlreadyExistsError } from "../letter-already-exists-error";
1213

1314
function createLetter(
1415
supplierId: string,
@@ -149,9 +150,7 @@ describe("LetterRepository", () => {
149150
await letterRepository.putLetter(createLetter("supplier1", "letter1"));
150151
await expect(
151152
letterRepository.putLetter(createLetter("supplier1", "letter1")),
152-
).rejects.toThrow(
153-
"Letter with id letter1 already exists for supplier supplier1",
154-
);
153+
).rejects.toThrow(LetterAlreadyExistsError);
155154
});
156155

157156
test("rethrows errors from DynamoDB when creating a letter", async () => {

internal/datastore/src/letter-repository.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
LetterSchemaBase,
1919
UpdateLetter,
2020
} from "./types";
21+
import { LetterAlreadyExistsError } from "./letter-already-exists-error";
2122

2223
export type PagingOptions = Partial<{
2324
exclusiveStartKey: Record<string, any>;
@@ -62,9 +63,7 @@ export class LetterRepository {
6263
error instanceof Error &&
6364
error.name === "ConditionalCheckFailedException"
6465
) {
65-
throw new Error(
66-
`Letter with id ${letter.id} already exists for supplier ${letter.supplierId}`,
67-
);
66+
throw new LetterAlreadyExistsError(letter.supplierId, letter.id);
6867
}
6968
throw error;
7069
}

lambdas/upsert-letter/jest.config.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@ export const baseJestConfig = {
99
},
1010
],
1111
},
12+
transformIgnorePatterns: [
13+
"node_modules/(?!(@nhsdigital/nhs-notify-event-schemas-supplier-config)/)",
14+
],
1215

1316
// Automatically clear mock calls, instances, contexts and results before every test
1417
clearMocks: true,

lambdas/upsert-letter/src/handler/__tests__/upsert-handler.test.ts

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
import { SQSEvent, SQSRecord } from "aws-lambda";
22
import pino from "pino";
3-
import { LetterRepository } from "internal/datastore/src";
3+
import {
4+
LetterAlreadyExistsError,
5+
LetterRepository,
6+
} from "@internal/datastore";
47
import { LetterRequestPreparedEventV2 } from "@nhsdigital/nhs-notify-event-schemas-letter-rendering";
58
import { LetterRequestPreparedEvent } from "@nhsdigital/nhs-notify-event-schemas-letter-rendering-v1";
69
import {
@@ -199,7 +202,11 @@ describe("createUpsertLetterHandler", () => {
199202
putLetter: jest.fn(),
200203
updateLetterStatus: jest.fn(),
201204
} as unknown as LetterRepository,
202-
logger: { error: jest.fn(), info: jest.fn() } as unknown as pino.Logger,
205+
logger: {
206+
error: jest.fn(),
207+
warn: jest.fn(),
208+
info: jest.fn(),
209+
} as unknown as pino.Logger,
203210
env: {
204211
LETTERS_TABLE_NAME: "LETTERS_TABLE_NAME",
205212
LETTER_TTL_HOURS: 12_960,
@@ -301,6 +308,31 @@ describe("createUpsertLetterHandler", () => {
301308
);
302309
});
303310

311+
it("does not treat a replayed insert as a failure", async () => {
312+
const v1message = {
313+
letterEvent: createPreparedV1Event(),
314+
supplierSpec: {
315+
supplierId: "supplier1",
316+
specId: "spec1",
317+
priority: 10,
318+
billingId: "billing1",
319+
},
320+
};
321+
const evt: SQSEvent = createSQSEvent([
322+
createSqsRecord("msg2", JSON.stringify(v1message)),
323+
]);
324+
(mockedDeps.letterRepo.putLetter as jest.Mock).mockRejectedValue(
325+
new LetterAlreadyExistsError("supplier1", "letter1"),
326+
);
327+
328+
const result = await createUpsertLetterHandler(mockedDeps)(
329+
evt,
330+
{} as any,
331+
{} as any,
332+
);
333+
expect(result!.batchItemFailures).toEqual([]);
334+
});
335+
304336
test("unknown supplier has metric emitted with 'unknown' supplier dimension", async () => {
305337
const letterEvent = createSupplierStatusChangeEventWithoutSupplier();
306338

lambdas/upsert-letter/src/handler/upsert-handler.ts

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
import { SQSBatchItemFailure, SQSEvent, SQSHandler } from "aws-lambda";
2-
import { InsertLetter, UpdateLetter } from "@internal/datastore";
2+
import {
3+
InsertLetter,
4+
LetterAlreadyExistsError,
5+
UpdateLetter,
6+
} from "@internal/datastore";
37
import {
48
$LetterRequestPreparedEvent,
59
LetterRequestPreparedEvent,
@@ -69,14 +73,26 @@ function getOperationFromType(type: string): UpsertOperation {
6973
supplierSpec.priority,
7074
supplierSpec.billingId, // use billingId for now
7175
);
72-
await deps.letterRepo.putLetter(letterToInsert);
76+
try {
77+
await deps.letterRepo.putLetter(letterToInsert);
7378

74-
deps.logger.info({
75-
description: "Inserted letter",
76-
eventId: preparedRequest.id,
77-
letterId: letterToInsert.id,
78-
supplierId: letterToInsert.supplierId,
79-
});
79+
deps.logger.info({
80+
description: "Inserted letter",
81+
eventId: preparedRequest.id,
82+
letterId: letterToInsert.id,
83+
supplierId: letterToInsert.supplierId,
84+
});
85+
} catch (error) {
86+
if (error instanceof LetterAlreadyExistsError) {
87+
deps.logger.warn({
88+
description: "Letter already exists",
89+
supplierId: letterToInsert.supplierId,
90+
letterId: letterToInsert.id,
91+
});
92+
} else {
93+
throw error;
94+
}
95+
}
8096
},
8197
};
8298
}

package-lock.json

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)