fix: Update OpenAPI error converter to handle query param errors too (#…

…7609)

This PR updates the OpenAPI error converter to also work for errors with
query parameters.
We previously only sent the body of the request along with the error,
which meant that query parameter errors would show up incorrectly.

For instance given a query param with the date format and the invalid
value `01-2020-01`, you'd previously get the message:
> The `from` value must match format "date". You sent undefined

With this change, you'll get this instead:
> The `from` value must match format "date". You sent "01-2020-01". 

The important changes here are two things:
- passing both request body and query params
- the 3 lines in `fromOpenApiValidationError` that check where we should
get the value you sent from.

The rest of it is primarily updating tests to send the right arguments
and some slight rewording to more accurately reflect that this can be
either request body or query params.

src/lib/error/bad-data-error.ts

@@ -117,10 +117,14 @@ const enumMessage = (
 };
 
 export const fromOpenApiValidationError =
-    (requestBody: object) =>
+    (request: { body: object; query: object }) =>
     (validationError: ErrorObject): ValidationErrorDescription => {
         const { instancePath, params, message } = validationError;
-        const propertyName = instancePath.substring('/body/'.length);
+        const [errorSource, substringOffset] = instancePath.startsWith('/body')
+            ? [request.body, '/body/'.length]
+            : [request.query, '/query/'.length];
+
+        const propertyName = instancePath.substring(substringOffset);
 
         switch (validationError.keyword) {
             case 'required':
@@ -139,28 +143,28 @@ export const fromOpenApiValidationError =
                     message,
                     params.allowedValues,
                     getProp(
-                        requestBody,
-                        instancePath.substring('/body/'.length).split('/'),
+                        errorSource,
+                        instancePath.substring(substringOffset).split('/'),
                     ),
                 );
 
             case 'oneOf':
                 return oneOfMessage(propertyName, validationError.message);
             default:
-                return genericErrorMessage(requestBody, propertyName, message);
+                return genericErrorMessage(errorSource, propertyName, message);
         }
     };
 
 export const fromOpenApiValidationErrors = (
-    requestBody: object,
+    request: { body: object; query: object },
     validationErrors: [ErrorObject, ...ErrorObject[]],
 ): BadDataError => {
     const [firstDetail, ...remainingDetails] = validationErrors.map(
-        fromOpenApiValidationError(requestBody),
+        fromOpenApiValidationError(request),
     );
 
     return new BadDataError(
-        "Request validation failed: the payload you provided doesn't conform to the schema. Check the `details` property for a list of errors that we found.",
+        "Request validation failed: your request doesn't conform to the schema. Check the `details` property for a list of errors that we found.",
         [firstDetail, ...remainingDetails],
     );
 };

src/lib/error/unleash-error.test.ts

@@ -56,7 +56,9 @@ describe('OpenAPI error conversion', () => {
             message: "should have required property 'enabled'",
         };
 
-        const result = fromOpenApiValidationError({})(error);
+        const result = fromOpenApiValidationError({ body: {}, query: {} })(
+            error,
+        );
 
         expect(result).toMatchObject({
             message:
@@ -83,7 +85,10 @@ describe('OpenAPI error conversion', () => {
 
         const parameterValue = [];
         const result = fromOpenApiValidationError({
-            parameters: parameterValue,
+            body: {
+                parameters: parameterValue,
+            },
+            query: {},
         })(error);
 
         expect(result).toMatchObject({
@@ -111,9 +116,12 @@ describe('OpenAPI error conversion', () => {
             };
 
             const result = fromOpenApiValidationError({
-                secret: 'blah',
-                username: 'string2',
-                type: 'admin',
+                body: {
+                    secret: 'blah',
+                    username: 'string2',
+                    type: 'admin',
+                },
+                query: {},
             })(error);
 
             expect(result).toMatchObject({
@@ -148,7 +156,10 @@ describe('OpenAPI error conversion', () => {
 
         const requestDescription = 'A pattern that does not match.';
         const result = fromOpenApiValidationError({
-            description: requestDescription,
+            body: {
+                description: requestDescription,
+            },
+            query: {},
         })(error);
 
         expect(result).toMatchObject({
@@ -167,14 +178,17 @@ describe('OpenAPI error conversion', () => {
             message: 'must be equal to one of the allowed values',
         };
 
-        const request = [
-            {
-                name: 'variant',
-                weight: 500,
-                weightType: 'party',
-                stickiness: 'userId',
-            },
-        ];
+        const request = {
+            body: [
+                {
+                    name: 'variant',
+                    weight: 500,
+                    weightType: 'party',
+                    stickiness: 'userId',
+                },
+            ],
+            query: {},
+        };
 
         const result = fromOpenApiValidationError(request)(error);
 
@@ -202,7 +216,10 @@ describe('OpenAPI error conversion', () => {
 
         const requestDescription = 'Longer than the max length';
         const result = fromOpenApiValidationError({
-            description: requestDescription,
+            body: {
+                description: requestDescription,
+            },
+            query: {},
         })(error);
 
         expect(result).toMatchObject({
@@ -233,7 +250,10 @@ describe('OpenAPI error conversion', () => {
 
         const propertyValue = 6;
         const result = fromOpenApiValidationError({
-            newprop: propertyValue,
+            body: {
+                newprop: propertyValue,
+            },
+            query: {},
         })(error);
 
         expect(result).toMatchObject({
@@ -278,7 +298,10 @@ describe('OpenAPI error conversion', () => {
 
         // create an error and serialize it as it would be shown to the end user.
         const serializedUnleashError: ApiErrorSchema =
-            fromOpenApiValidationErrors({ newprop: 7 }, errors).toJSON();
+            fromOpenApiValidationErrors(
+                { body: { newprop: 7 }, query: {} },
+                errors,
+            ).toJSON();
 
         expect(serializedUnleashError).toMatchObject({
             name: 'BadDataError',
@@ -305,9 +328,10 @@ describe('OpenAPI error conversion', () => {
                 message: 'should NOT have additional properties',
             };
 
-            const error = fromOpenApiValidationError({ bogus: 5 })(
-                openApiError,
-            );
+            const error = fromOpenApiValidationError({
+                body: { bogus: 5 },
+                query: {},
+            })(openApiError);
 
             expect(error).toMatchObject({
                 message: expect.stringContaining(
@@ -322,9 +346,12 @@ describe('OpenAPI error conversion', () => {
 
         it('gives useful messages for nested properties', () => {
             const request2 = {
-                nestedObject: {
-                    nested2: { extraPropertyName: 'illegal property' },
+                body: {
+                    nestedObject: {
+                        nested2: { extraPropertyName: 'illegal property' },
+                    },
                 },
+                query: {},
             };
             const openApiError = {
                 keyword: 'additionalProperties',
@@ -360,7 +387,10 @@ describe('OpenAPI error conversion', () => {
         };
 
         const result = fromOpenApiValidationError({
-            nestedObject: { a: { b: [] } },
+            body: {
+                nestedObject: { a: { b: [] } },
+            },
+            query: {},
         })(error);
 
         expect(result).toMatchObject({
@@ -382,7 +412,10 @@ describe('OpenAPI error conversion', () => {
 
         const illegalValue = 'illegal string';
         const result = fromOpenApiValidationError({
-            nestedObject: { a: { b: illegalValue } },
+            body: {
+                nestedObject: { a: { b: illegalValue } },
+            },
+            query: {},
         })(error);
 
         expect(result).toMatchObject({
@@ -392,6 +425,30 @@ describe('OpenAPI error conversion', () => {
 
         expect(result.message).toMatch(/\bnestedObject\/a\/b\b/);
     });
+
+    it('handles query parameter errors', () => {
+        const error = {
+            instancePath: '/query/from',
+            schemaPath: '#/properties/query/properties/from/format',
+            keyword: 'format',
+            params: { format: 'date' },
+            message: 'must match format "date"',
+        };
+        const query = { from: '01-2020-01' };
+
+        const result = fromOpenApiValidationError({ body: {}, query })(error);
+
+        expect(result).toMatchObject({
+            message:
+                // it tells the user what the format is
+                expect.stringContaining(error.params.format),
+        });
+
+        // it tells the user which property it pertains to
+        expect(result.message).toContain('from');
+        // it tells the user what they provided
+        expect(result.message).toContain(query.from);
+    });
 });
 
 describe('Error serialization special cases', () => {

src/lib/services/openapi-service.ts

@@ -64,7 +64,7 @@ export class OpenApiService {
         app.use((err, req, res, next) => {
             if (err?.status && err.validationErrors) {
                 const apiError = fromOpenApiValidationErrors(
-                    req.body,
+                    req,
                     err.validationErrors,
                 );