refactor(api): error formatting (#51595)

api/src/routes/challenge.test.ts

@@ -112,12 +112,16 @@ describe('challengeRoutes', () => {
     });
 
     describe('POST /coderoad-challenge-completed', () => {
-      test('should return 500 if no tutorialId', async () => {
+      test('should return 400 if no tutorialId', async () => {
         const response = await superRequest('/coderoad-challenge-completed', {
           method: 'POST',
           setCookies
         });
-        expect(response.status).toBe(500);
+        expect(response.body).toEqual({
+          msg: `'tutorialId' not found in request body`,
+          type: 'error'
+        });
+        expect(response.status).toBe(400);
       });
 
       test('should return 400 if no user token', async () => {
@@ -127,11 +131,11 @@ describe('challengeRoutes', () => {
         }).send({
           tutorialId: 'freeCodeCamp/learn-bash-by-building-a-boilerplate:v1.0.0'
         });
-        expect(response.status).toBe(400);
         expect(response.body).toEqual({
           msg: `'Coderoad-User-Token' not found in request headers`,
           type: 'error'
         });
+        expect(response.status).toBe(400);
       });
 
       test('should return 400 if invalid user token', async () => {

api/src/routes/challenge.ts

@@ -10,7 +10,10 @@ import {
   type CompletedChallenge
 } from '../utils/common-challenge-functions';
 import { JWT_SECRET } from '../utils/env';
-import { formatValidationError } from '../utils/error-formatting';
+import {
+  formatCoderoadChallengeCompletedValidation,
+  formatProjectCompletedValidation
+} from '../utils/error-formatting';
 import { getChallenges } from '../utils/get-challenges';
 import { ProgressTimestamp, getPoints } from '../utils/progress';
 import {
@@ -45,44 +48,29 @@ export const challengeRoutes: FastifyPluginCallbackTypebox = (
     '/coderoad-challenge-completed',
     {
       schema: schemas.coderoadChallengeCompleted,
-      attachValidation: true
+      errorHandler(error, request, reply) {
+        if (error.validation) {
+          void reply.code(400);
+          return formatCoderoadChallengeCompletedValidation(error.validation);
+        } else {
+          fastify.errorHandler(error, request, reply);
+        }
+      }
     },
     async (req, reply) => {
-      let userToken;
-
       const { 'coderoad-user-token': encodedUserToken } = req.headers;
       const { tutorialId } = req.body;
 
-      if (!tutorialId) {
-        void reply.code(400);
-        return {
-          type: 'error',
-          msg: `'tutorialId' not found in request body`
-        } as const;
-      }
-
-      if (!encodedUserToken) {
-        void reply.code(400);
-        return {
-          type: 'error',
-          msg: `'Coderoad-User-Token' not found in request headers`
-        } as const;
-      }
-
+      let userToken;
       try {
-        if (typeof encodedUserToken === 'string') {
-          const payload = jwt.verify(
-            encodedUserToken,
-            JWT_SECRET
-          ) as JwtPayload;
-          userToken = payload.userToken;
-        }
+        const payload = jwt.verify(encodedUserToken, JWT_SECRET) as JwtPayload;
+        userToken = payload.userToken;
       } catch {
         void reply.code(400);
         return { type: 'error', msg: `invalid user token` } as const;
       }
 
-      const tutorialRepo = tutorialId?.split(':')[0];
+      const tutorialRepo = tutorialId.split(':')[0];
       const tutorialOrg = tutorialRepo?.split('/')?.[0];
 
       if (tutorialOrg !== 'freeCodeCamp') {
@@ -183,7 +171,7 @@ export const challengeRoutes: FastifyPluginCallbackTypebox = (
       errorHandler(error, request, reply) {
         if (error.validation) {
           void reply.code(400);
-          return formatValidationError(error.validation);
+          return formatProjectCompletedValidation(error.validation);
         } else {
           fastify.errorHandler(error, request, reply);
         }
@@ -270,7 +258,7 @@ export const challengeRoutes: FastifyPluginCallbackTypebox = (
       errorHandler(error, request, reply) {
         if (error.validation) {
           void reply.code(400);
-          return formatValidationError(error.validation);
+          return formatProjectCompletedValidation(error.validation);
         } else {
           fastify.errorHandler(error, request, reply);
         }
@@ -322,7 +310,7 @@ export const challengeRoutes: FastifyPluginCallbackTypebox = (
       errorHandler(error, request, reply) {
         if (error.validation) {
           void reply.code(400);
-          return formatValidationError(error.validation);
+          return formatProjectCompletedValidation(error.validation);
         } else {
           fastify.errorHandler(error, request, reply);
         }

api/src/schemas.ts

@@ -415,6 +415,7 @@ export const schemas = {
     body: Type.Object({
       tutorialId: Type.String()
     }),
+    headers: Type.Object({ 'coderoad-user-token': Type.String() }),
     response: {
       200: Type.Object({
         type: Type.Literal('success'),

api/src/utils/error-formatting.test.ts

@@ -1,72 +0,0 @@
-import { ErrorObject } from 'ajv';
-import { formatValidationError } from './error-formatting';
-
-const missingSolutionError = {
-  instancePath: '',
-  schemaPath: '#/required',
-  keyword: 'required',
-  params: { missingProperty: 'solution' },
-  message: "must have required property 'solution'"
-};
-
-const missingIdError = {
-  instancePath: '',
-  schemaPath: '#/required',
-  keyword: 'required',
-  params: { missingProperty: 'id' },
-  message: "must have required property 'id'"
-};
-
-describe('Error formatting', () => {
-  describe('formatValidationError', () => {
-    it('should handle missing solutions', () => {
-      const formattedError = formatValidationError([missingSolutionError]);
-
-      expect(formattedError).toStrictEqual({
-        type: 'error',
-        message:
-          'You have not provided the valid links for us to inspect your work.'
-      });
-    });
-
-    it('should handle missing ids', () => {
-      const formattedError = formatValidationError([missingIdError]);
-
-      expect(formattedError).toStrictEqual({
-        type: 'error',
-
-        message: 'That does not appear to be a valid challenge submission.'
-      });
-    });
-
-    it('should return a generic error message for other errors', () => {
-      const formattedError = formatValidationError([
-        {
-          ...missingSolutionError,
-          params: { missingProperty: 'notSolution' }
-        }
-      ]);
-
-      expect(formattedError).toStrictEqual({
-        type: 'error',
-        message: 'That does not appear to be a valid challenge submission.'
-      });
-    });
-
-    it('should throw if passed zero errors', () => {
-      expect(() => formatValidationError([] as ErrorObject[])).toThrow(
-        Error(
-          'Bad Argument: the array of errors must have exactly one element.'
-        )
-      );
-    });
-
-    it('should throw if passed more than one error', () => {
-      expect(() => formatValidationError([{}, {}] as ErrorObject[])).toThrow(
-        Error(
-          'Bad Argument: the array of errors must have exactly one element.'
-        )
-      );
-    });
-  });
-});

api/src/utils/error-formatting.ts

@@ -1,30 +1,41 @@
 import { ErrorObject } from 'ajv';
 
-export type FormattedError = {
+type FormattedError = {
   type: 'error';
-  message:
-    | 'You have not provided the valid links for us to inspect your work.'
-    | 'That does not appear to be a valid challenge submission.'
-    // the next isn't generated here, but the type is more general.
-    | 'You have to complete the project before you can submit a URL.';
+  message: string;
 };
 
-/**
- * Format invalid challenge submission errors.
- *
- * @param errors An array of validation errors.
- * @returns Formatted errors that can be used in the response.
- */
-export const formatValidationError = (
-  errors: ErrorObject[]
-): FormattedError => {
-  if (errors.length !== 1) {
+// TODO(Post-MVP): Normalize error responses (either msg or messge, not both)
+type CodeRoadError = {
+  type: 'error';
+  msg: string;
+};
+
+const getError = (errors: ErrorObject[]): ErrorObject => {
+  // This is a guard against accidentally enabling allErrors in ajv and making
+  // the server more vulnerable to DOS.
+  const error = errors[0];
+  if (!error || errors.length !== 1) {
     throw new Error(
       'Bad Argument: the array of errors must have exactly one element.'
     );
   }
+  return error;
+};
 
-  return errors[0]?.params.missingProperty === 'solution'
+/**
+ * Format validation errors for /project-completed.
+ *
+ * @param errors An array of validation errors.
+ * @returns Formatted errors that can be used in the response.
+ */
+export const formatProjectCompletedValidation = (
+  errors: ErrorObject[]
+): FormattedError => {
+  const error = getError(errors);
+
+  return error.instancePath === '' &&
+    error.params.missingProperty === 'solution'
     ? {
         type: 'error',
         message:
@@ -35,3 +46,32 @@ export const formatValidationError = (
         message: 'That does not appear to be a valid challenge submission.'
       };
 };
+
+/**
+ * Format validation errors for /coderoad-challenge-completed.
+ *
+ * @param errors An array of validation errors.
+ * @returns Formatted errors that can be used in the response.
+ */
+export const formatCoderoadChallengeCompletedValidation = (
+  errors: ErrorObject[]
+): CodeRoadError => {
+  const error = getError(errors);
+
+  // TODO(Post-MVP): Return error saying that the body is not an object.
+  if (error.instancePath === '' && error.message === 'must be object')
+    return { type: 'error', msg: `'tutorialId' not found in request body` };
+
+  if (
+    error.instancePath === '' &&
+    error.params.missingProperty === 'coderoad-user-token'
+  ) {
+    return {
+      type: 'error',
+      msg: `'Coderoad-User-Token' not found in request headers`
+    };
+  } else {
+    // by process of elimination:
+    return { type: 'error', msg: `'tutorialId' not found in request body` };
+  }
+};