Mirrors/unleash.unleash
1
0
Fork 0
mirror of https://github.com/Unleash/unleash.git synced 2025-01-25 00:07:47 +01:00
Code Issues Projects Releases Wiki Activity

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

Browse Source 
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.
This commit is contained in:
Thomas Heartman 2024-07-17 12:47:32 +02:00 committed by GitHub
parent 6e4e58aee8
commit 949a5f0109
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 94 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
src/lib/error/bad-data-error.ts
View File

@ -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],
);
};

105
src/lib/error/unleash-error.test.ts
View File

@ -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', () => {

2
src/lib/services/openapi-service.ts
View File

@ -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,
);