Refactor handling of letter not found

Author: francisco-videira-nhs

.github/PULL_REQUEST_TEMPLATE.md

@@ -27,6 +27,12 @@
 - [ ] This PR is a result of pair or mob programming
 - [ ] If I have used the 'skip-trivy-package' label I have done so responsibly and in the knowledge that this is being fixed as part of a separate ticket/PR.
 
+## DT3-Specific Checklist
+
+<!-- Go over all the following points, and put an `x` in all the boxes that apply. -->
+
+- [ ] If I have added a new resource (SQS, Lambda, Gateway, DDB table, etc), I have created the appropriate alarms
+
 ---
 
 ## Sensitive Information Declaration

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

@@ -7,7 +7,7 @@ import {
 } from "./db";
 import LetterQueueRepository from "../letter-queue-repository";
 import { InsertPendingLetter } from "../types";
-import { LetterAlreadyExistsError } from "../errors";
+import { LetterAlreadyExistsError } from "../errors/letter-already-exists-error";
 import { createTestLogger } from "./logs";
 
 function createLetter(letterId = "letter1"): InsertPendingLetter {

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

@@ -140,7 +140,7 @@ describe("LetterRepository", () => {
     await expect(
       letterRepository.getLetterById("supplier1", "letter1"),
     ).rejects.toThrow(
-      "Letter with id letter1 not found for supplier supplier1",
+      "Letter not found: supplierId=supplier1, letterId=letter1",
     );
   });
 

internal/datastore/src/errors.ts …rc/errors/letter-already-exists-error.tsinternal/datastore/src/errors.ts renamed to internal/datastore/src/errors/letter-already-exists-error.ts internal/datastore/src/errors.ts renamed to internal/datastore/src/errors/letter-already-exists-error.ts

@@ -0,0 +1,12 @@
+/**
+ * Error thrown when a letter is not found in the database.
+ */
+export default class LetterNotFoundError extends Error {
+  constructor(
+    public readonly supplierId: string,
+    public readonly letterId: string,
+  ) {
+    super(`Letter not found: supplierId=${supplierId}, letterId=${letterId}`);
+    this.name = "LetterNotFoundError";
+  }
+}

internal/datastore/src/errors/letter-not-found-error.ts

@@ -1,5 +1,5 @@
 export * from "./types";
-export * from "./errors";
+export * from "./errors/letter-already-exists-error";
 export * from "./mi-repository";
 export * from "./letter-repository";
 export * from "./supplier-repository";

internal/datastore/src/index.ts

@@ -5,7 +5,7 @@ import {
   PendingLetter,
   PendingLetterSchema,
 } from "./types";
-import { LetterAlreadyExistsError } from "./errors";
+import { LetterAlreadyExistsError } from "./errors/letter-already-exists-error";
 
 type LetterQueueRepositoryConfig = {
   letterQueueTableName: string;

internal/datastore/src/letter-queue-repository.ts

@@ -18,6 +18,7 @@ import {
   LetterSchemaBase,
   UpdateLetter,
 } from "./types";
+import LetterNotFoundError from "./errors/letter-not-found-error";
 
 export type PagingOptions = Partial<{
   exclusiveStartKey: Record<string, any>;
@@ -117,9 +118,7 @@ export class LetterRepository {
     );
 
     if (!result.Item) {
-      throw new Error(
-        `Letter with id ${letterId} not found for supplier ${supplierId}`,
-      );
+      throw new LetterNotFoundError(supplierId, letterId);
     }
     return LetterSchema.parse(result.Item);
   }

internal/datastore/src/letter-repository.ts

@@ -3,6 +3,7 @@ import pino from "pino";
 import { getSignedUrl } from "@aws-sdk/s3-request-presigner";
 import { S3Client } from "@aws-sdk/client-s3";
 import { SQSClient } from "@aws-sdk/client-sqs";
+import LetterNotFoundError from "@internal/datastore/src/errors/letter-not-found-error";
 import {
   enqueueLetterUpdateRequests,
   getLetterById,
@@ -104,13 +105,11 @@ describe("getLetterById", () => {
     const mockRepo = {
       getLetterById: jest
         .fn()
-        .mockRejectedValue(
-          new Error("Letter with id l1 not found for supplier s1"),
-        ),
+        .mockRejectedValue(new LetterNotFoundError("supplier1", "letter1")),
     };
 
     await expect(
-      getLetterById("supplierid", "letter1", mockRepo as any),
+      getLetterById("supplier1", "letter1", mockRepo as any),
     ).rejects.toThrow("No resource found with that ID");
   });
 
@@ -171,9 +170,7 @@ describe("getLetterDataUrl function", () => {
     deps.letterRepo = {
       getLetterById: jest
         .fn()
-        .mockRejectedValue(
-          new Error("Letter with id l1 not found for supplier s1"),
-        ),
+        .mockRejectedValue(new LetterNotFoundError("supplier1", "letter42")),
     } as unknown as LetterRepository;
 
     await expect(

lambdas/api-handler/src/services/__tests__/letter-operations.test.ts

@@ -2,18 +2,12 @@ import { LetterBase, LetterRepository } from "@internal/datastore";
 import { getSignedUrl } from "@aws-sdk/s3-request-presigner";
 import { GetObjectCommand, S3Client } from "@aws-sdk/client-s3";
 import { SendMessageBatchCommand } from "@aws-sdk/client-sqs";
+import LetterNotFoundError from "@internal/datastore/src/errors/letter-not-found-error";
 import NotFoundError from "../errors/not-found-error";
 import { UpdateLetterCommand } from "../contracts/letters";
 import { ApiErrorDetail } from "../contracts/errors";
 import { Deps } from "../config/deps";
 
-function isNotFoundError(error: any) {
-  return (
-    error instanceof Error &&
-    /^Letter with id \w+ not found for supplier \w+$/.test(error.message)
-  );
-}
-
 async function getDownloadUrl(
   s3Uri: string,
   s3Client: S3Client,
@@ -50,7 +44,7 @@ export const getLetterById = async (
   try {
     letter = await letterRepo.getLetterById(supplierId, letterId);
   } catch (error) {
-    if (isNotFoundError(error)) {
+    if (error instanceof LetterNotFoundError) {
       throw new NotFoundError(ApiErrorDetail.NotFoundLetterId);
     }
     throw error;
@@ -74,7 +68,7 @@ export const getLetterDataUrl = async (
       deps.env.DOWNLOAD_URL_TTL_SECONDS,
     );
   } catch (error) {
-    if (isNotFoundError(error)) {
+    if (error instanceof LetterNotFoundError) {
       throw new NotFoundError(ApiErrorDetail.NotFoundLetterId);
     }
     throw error;