Improve error catching

services/app-api/handlers/banners/create.ts

@@ -1,57 +1,62 @@
 import handler from "../handler-lib";
-// utils
-import { hasPermissions } from "../../utils/auth/authorization";
-import { error } from "../../utils/constants/constants";
 // types
 import { UserRoles } from "../../utils/types";
 import { number, object, string } from "yup";
-import { validateData } from "../../utils/validation/validation";
+// utils
 import { putBanner } from "../../storage/banners";
+import { hasPermissions } from "../../utils/auth/authorization";
+import { error } from "../../utils/constants/constants";
+import { validateData } from "../../utils/validation/validation";
 import {
   badRequest,
   created,
   forbidden,
+  internalServerError,
 } from "../../utils/responses/response-lib";
 
+const validationSchema = object().shape({
+  key: string().required(),
+  title: string().required(),
+  description: string().required(),
+  link: string().url().notRequired(),
+  startDate: number().required(),
+  endDate: number().required(),
+});
+
 export const createBanner = handler(async (event, _context) => {
   if (!hasPermissions(event, [UserRoles.ADMIN])) {
     return forbidden(error.UNAUTHORIZED);
-  } else if (!event?.pathParameters?.bannerId!) {
+  }
+  if (!event?.pathParameters?.bannerId!) {
     return badRequest(error.NO_KEY);
-  } else {
-    const unvalidatedPayload = JSON.parse(event!.body!);
+  }
+  const unvalidatedPayload = JSON.parse(event.body!);
 
-    const validationSchema = object().shape({
-      key: string().required(),
-      title: string().required(),
-      description: string().required(),
-      link: string().url().notRequired(),
-      startDate: number().required(),
-      endDate: number().required(),
-    });
+  let validatedPayload;
+  try {
+    validatedPayload = await validateData(validationSchema, unvalidatedPayload);
+  } catch {
+    return badRequest(error.INVALID_DATA);
+  }
 
-    let validatedPayload;
-    try {
-      validatedPayload = await validateData(
-        validationSchema,
-        unvalidatedPayload
-      );
-    } catch {
-      return badRequest(error.INVALID_DATA);
-    }
+  const { title, description, link, startDate, endDate } = validatedPayload;
+  const currentTime = Date.now();
 
-    const newBanner = {
-      key: event.pathParameters.bannerId,
-      createdAt: Date.now(),
-      lastAltered: Date.now(),
-      lastAlteredBy: event?.headers["cognito-identity-id"],
-      title: validatedPayload.title,
-      description: validatedPayload.description,
-      link: validatedPayload.link,
-      startDate: validatedPayload.startDate,
-      endDate: validatedPayload.endDate,
-    };
+  const newBanner = {
+    key: event.pathParameters.bannerId,
+    createdAt: currentTime,
+    lastAltered: currentTime,
+    lastAlteredBy: event?.headers["cognito-identity-id"],
+    title,
+    description,
+    link,
+    startDate,
+    endDate,
+  };
+  try {
     await putBanner(newBanner);
-    return created(newBanner);
+  } catch {
+    return internalServerError(error.DYNAMO_CREATION_ERROR);
   }
+  return created(newBanner);
 });

services/app-api/handlers/banners/delete.ts

@@ -10,11 +10,11 @@ import { badRequest, forbidden, ok } from "../../utils/responses/response-lib";
 export const deleteBanner = handler(async (event, _context) => {
   if (!hasPermissions(event, [UserRoles.ADMIN])) {
     return forbidden(error.UNAUTHORIZED);
-  } else if (!event?.pathParameters?.bannerId!) {
+  }
+  if (!event?.pathParameters?.bannerId!) {
     return badRequest(error.NO_KEY);
-  } else {
-    const bannerId = event?.pathParameters?.bannerId!;
-    await deleteBannerById(bannerId);
-    return ok();
   }
+  const bannerId = event?.pathParameters?.bannerId!;
+  await deleteBannerById(bannerId);
+  return ok();
 });

services/app-api/handlers/reports/create.ts

@@ -75,7 +75,7 @@ export const createReport = handler(
         ? await getEligibleWorkPlan(state)
         : { workPlanMetadata: undefined, workPlanFieldData: undefined };
 
-    // If we recieved no work plan information and we're trying to create a SAR, return NO_WORKPLANS_FOUND
+    // If we received no work plan information and we're trying to create a SAR, return NO_WORKPLANS_FOUND
     if (
       reportType === ReportType.SAR &&
       (!workPlanMetadata || !workPlanFieldData)
@@ -132,10 +132,15 @@ export const createReport = handler(
     }
 
     // Setup validation for what we expect to see in the payload
-    let validatedFieldData = await validateFieldData(
-      creationFieldDataValidationJson,
-      unvalidatedFieldData
-    );
+    let validatedFieldData;
+    try {
+      validatedFieldData = await validateFieldData(
+        creationFieldDataValidationJson,
+        unvalidatedFieldData
+      );
+    } catch {
+      return badRequest(error.INVALID_DATA);
+    }
 
     // If we are creating a SAR and found a Work Plan to copy from, grab its field data and add it to our SAR
     if (workPlanFieldData) {
@@ -147,8 +152,8 @@ export const createReport = handler(
       extractWorkPlanData(validatedFieldData!, reportYear, reportPeriod);
     }
 
-    // Return INVALID_DATA error if field data is not valid.
-    if (!validatedFieldData || Object.keys(validatedFieldData).length === 0) {
+    // Return INVALID_DATA error field data has no valid entries
+    if (validatedFieldData && Object.keys(validatedFieldData).length === 0) {
       return badRequest(error.INVALID_DATA);
     }
     // End Section - Check the payload that was sent with the request and validate it
@@ -189,12 +194,13 @@ export const createReport = handler(
      */
 
     // Validate the metadata for the submission
-    const validatedMetadata = await validateData(metadataValidationSchema, {
-      ...unvalidatedMetadata,
-    });
-
-    // Return INVALID_DATA error if metadata is not valid.
-    if (!validatedMetadata) {
+    let validatedMetadata;
+    try {
+      validatedMetadata = await validateData(metadataValidationSchema, {
+        ...unvalidatedMetadata,
+      });
+    } catch {
+      // Return INVALID_DATA error if metadata is not valid.
       return badRequest(error.INVALID_DATA);
     }
 

services/app-api/handlers/reports/update.test.ts

@@ -132,8 +132,7 @@ describe("Test updateReport API method", () => {
 
     const response = await updateReport(invalidFieldDataSubmissionEvent, null);
 
-    expect(consoleSpy.error).toHaveBeenCalled();
-    expect(response.statusCode).toBe(StatusCodes.InternalServerError);
+    expect(response.statusCode).toBe(StatusCodes.BadRequest);
     expect(response.body).toContain(error.INVALID_DATA);
     expect(putReportFieldData).not.toHaveBeenCalled();
     expect(putReportMetadata).not.toHaveBeenCalled();

services/app-api/handlers/reports/update.ts

@@ -110,12 +110,13 @@ export const updateReport = handler(async (event) => {
   }
 
   // Validate passed field data
-  const validatedFieldData = await validateFieldData(
-    formTemplate.validationJson,
-    unvalidatedFieldData
-  );
-
-  if (!validatedFieldData) {
+  let validatedFieldData;
+  try {
+    validatedFieldData = await validateFieldData(
+      formTemplate.validationJson,
+      unvalidatedFieldData
+    );
+  } catch {
     return badRequest(error.INVALID_DATA);
   }
 
@@ -138,13 +139,14 @@ export const updateReport = handler(async (event) => {
   );
 
   // validate report metadata
-  const validatedMetadata = await validateData(metadataValidationSchema, {
-    ...unvalidatedMetadata,
-    completionStatus,
-  });
-
-  // If metadata fails validation, return 400
-  if (!validatedMetadata) {
+  let validatedMetadata;
+  try {
+    validatedMetadata = await validateData(metadataValidationSchema, {
+      ...unvalidatedMetadata,
+      completionStatus,
+    });
+  } catch {
+    // If metadata fails validation, return 400
     return badRequest(error.INVALID_DATA);
   }
 

services/app-api/utils/auth/authorization.test.ts

@@ -45,7 +45,9 @@ describe("Test authorization with api key and environment variables", () => {
     jest.clearAllMocks();
   });
   test("is not authorized when no api key is passed", async () => {
-    mockVerifier.mockReturnValue(true);
+    mockVerifier.mockImplementation(() => {
+      throw new Error("no key provided");
+    });
     const authStatus = await isAuthenticated(noApiKeyEvent);
     expect(authStatus).toBeFalsy();
   });

services/app-api/utils/auth/authorization.ts

@@ -51,17 +51,12 @@ export const isAuthenticated = async (event: APIGatewayProxyEvent) => {
     clientId: cognitoValues.userPoolClientId,
   });
 
-  let isAuthenticated;
-  if (event?.headers?.["x-api-key"]) {
-    try {
-      isAuthenticated = await verifier.verify(event.headers["x-api-key"]);
-    } catch {
-      // verification failed - unauthenticated
-      isAuthenticated = false;
-    }
+  try {
+    await verifier.verify(event?.headers?.["x-api-key"]!);
+    return true;
+  } catch {
+    return false;
   }
-
-  return !!isAuthenticated;
 };
 
 export const hasPermissions = (