paulononato/backstage
1
0
Fork 0
Code Issues Pull Requests Actions 31 Packages Projects Releases Wiki Activity

better error messages

Browse Source 
Signed-off-by: aramissennyeydd <aramis.sennyey@doordash.com>
This commit is contained in:
aramissennyeydd
2024-07-07 15:27:08 -04:00
parent 1b05177ef6
commit 2c7750ca57
10 changed files with 107 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/backend-openapi-utils/src/schema/errors.ts
+30
View File
@@ -15,6 +15,8 @@
*/
import { Operation } from './types';
import { ErrorObject } from 'ajv';
import { humanifyAjvError } from './utils';
export class OperationError extends Error {
constructor(operation: Operation, message: string) {
@@ -35,3 +37,31 @@ export class OperationResponseError extends Error {
);
}
}
export class OperationParsingError extends OperationError {
constructor(operation: Operation, type: string, errors: ErrorObject[]) {
super(
operation,
`${type} validation failed.\n - ${errors
.map(humanifyAjvError)
.join('\n - ')}`,
);
}
}
export class OperationParsingResponseError extends OperationResponseError {
constructor(
operation: Operation,
response: Response,
type: string,
errors: ErrorObject[],
) {
super(
operation,
response,
`${type} validation failed.\n - ${errors
.map(humanifyAjvError)
.join('\n - ')}`,
);
}
}
packages/backend-openapi-utils/src/schema/parameter-validation.test.ts
+10 -10
View File
@@ -91,11 +91,11 @@ describe('query parameters', () => {
const request = {
url: 'http://localhost:8080/api/search?param=hello',
} as Request;
await expect(
parser.parse(request),
).rejects.toThrowErrorMatchingInlineSnapshot(
`"["GET /api/search"] Query parameter validation failed"`,
);
await expect(parser.parse(request)).rejects
.toThrowErrorMatchingInlineSnapshot(`
"["GET /api/search"] Query parameter validation failed.
- Value should be of type number"
`);
});
});
});
@@ -514,11 +514,11 @@ describe('path parameters', () => {
const request = {
url: 'http://localhost:8080/api/item/hello',
} as Request;
await expect(
parser.parse(request),
).rejects.toThrowErrorMatchingInlineSnapshot(
`"["GET /api/item/{id}"] Path parameter validation failed"`,
);
await expect(parser.parse(request)).rejects
.toThrowErrorMatchingInlineSnapshot(`
"["GET /api/item/{id}"] Path parameter validation failed.
- Value should be of type number"
`);
});
});
});
packages/backend-openapi-utils/src/schema/parameter-validation.ts
+10 -7
View File
@@ -23,7 +23,7 @@ import {
ValidatorParams,
} from './types';
import Ajv from 'ajv';
import { OperationError } from './errors';
import { OperationError, OperationParsingError } from './errors';
import { mockttpToFetchRequest } from './utils';
type ReferencelessSchemaObject = SchemaObject & { $ref?: never };
@@ -150,9 +150,10 @@ export class QueryParameterParser
const validate = this.ajv.compile(parameter.schema);
const valid = validate(param);
if (!valid) {
throw new OperationError(
throw new OperationParsingError(
this.operation,
'Query parameter validation failed',
'Query parameter',
validate.errors!,
);
}
queryParameters[name] = param;
@@ -339,9 +340,10 @@ export class HeaderParameterParser
const valid = validate(header);
if (!valid) {
throw new OperationError(
throw new OperationParsingError(
this.operation,
'Header parameter validation failed',
'Header parameter',
validate.errors!,
);
}
headerParameters[name] = header;
@@ -384,9 +386,10 @@ export class PathParameterParser
const valid = validate(param);
if (!valid) {
throw new OperationError(
throw new OperationParsingError(
this.operation,
'Path parameter validation failed',
'Path parameter',
validate.errors!,
);
}
pathParameters[name] = param;
packages/backend-openapi-utils/src/schema/request-body-validation.test.ts
+5 -5
View File
@@ -61,11 +61,11 @@ describe('request body', () => {
const requestBody = {
query: 1,
};
await expect(
parser.parse(toRequest(requestBody)),
).rejects.toThrowErrorMatchingInlineSnapshot(
`"["POST /api/search"] Request body validation failed."`,
);
await expect(parser.parse(toRequest(requestBody))).rejects
.toThrowErrorMatchingInlineSnapshot(`
"["POST /api/search"] Request body validation failed.
- "/query" should be of type string"
`);
});
it('should throw error if request body is required but missing', async () => {
packages/backend-openapi-utils/src/schema/request-body-validation.ts
+4 -3
View File
@@ -17,7 +17,7 @@
import { JsonObject } from '@backstage/types';
import { Operation, ParserOptions, RequestParser } from './types';
import { ValidateFunction } from 'ajv';
import { OperationError } from './errors';
import { OperationError, OperationParsingError } from './errors';
import { RequestBodyObject, SchemaObject } from 'openapi3-ts';
class DisabledRequestBodyParser
@@ -120,9 +120,10 @@ export class RequestBodyParser
const body = (await request.json()) as JsonObject;
const valid = this.validate(body);
if (!valid) {
throw new OperationError(
throw new OperationParsingError(
this.operation,
`Request body validation failed.`,
`Request body`,
this.validate.errors!,
);
}
return body;
packages/backend-openapi-utils/src/schema/response-body-validation.test.ts
+5 -5
View File
@@ -60,11 +60,11 @@ describe('response body', () => {
const responseBody = {
result: 1,
};
await expect(
parser.parse(toResponse(responseBody)),
).rejects.toThrowErrorMatchingInlineSnapshot(
`"["GET /api/search" (200)]: Response body validation failed"`,
);
await expect(parser.parse(toResponse(responseBody))).rejects
.toThrowErrorMatchingInlineSnapshot(`
"["GET /api/search" (200)]: Response body validation failed.
- The "result" property is not allowed"
`);
});
it('should throw error if response body is required but missing', async () => {
packages/backend-openapi-utils/src/schema/response-body-validation.ts
+11 -5
View File
@@ -16,7 +16,11 @@
import { JsonObject } from '@backstage/types';
import { Operation, ParserOptions, ResponseParser } from './types';
import { OperationError, OperationResponseError } from './errors';
import {
OperationError,
OperationParsingResponseError,
OperationResponseError,
} from './errors';
import Ajv from 'ajv';
import { OperationObject, ResponseObject } from 'openapi3-ts';
@@ -58,6 +62,7 @@ export class ResponseBodyParser
const responseSchemas = operation.schema.responses;
for (const [statusCode, schema] of Object.entries(responseSchemas)) {
if (!schema.content) {
// Skip responses without content, eg 204 No Content.
continue;
} else if (!schema.content['application/json']) {
throw new OperationError(
@@ -107,6 +112,7 @@ export class ResponseBodyParser
);
}
const schema = responseSchema.content!['application/json'].schema;
// This is a bit of type laziness. Ideally, this would be a type-narrowing function, but I wasn't able to get the types to work.
if (!schema) {
throw new OperationError(this.operation, 'No schema found in response');
}
@@ -133,10 +139,11 @@ export class ResponseBodyParser
const jsonBody = (await response.json()) as JsonObject;
const valid = validate(jsonBody);
if (!valid) {
throw new OperationResponseError(
throw new OperationParsingResponseError(
this.operation,
response,
'Response body validation failed',
'Response body',
validate.errors!,
);
}
return jsonBody;
@@ -144,9 +151,8 @@ export class ResponseBodyParser
private findResponseSchema(
operationSchema: OperationObject,
response: Response,
{ status }: Response,
): ResponseObject | undefined {
const { status } = response;
return (
operationSchema.responses?.[status] ?? operationSchema.responses?.default
);
packages/backend-openapi-utils/src/schema/utils.ts
+17
View File
@@ -14,6 +14,7 @@
* limitations under the License.
*/
import { CompletedRequest, CompletedResponse } from 'mockttp';
import { ErrorObject } from 'ajv';
export function mockttpToFetchRequest(request: CompletedRequest) {
const headers = new Headers(request.rawHeaders);
@@ -34,3 +35,19 @@ export function mockttpToFetchResponse(response: CompletedResponse) {
text: () => response.body?.getText(),
} as Response;
}
export function humanifyAjvError(error: ErrorObject) {
switch (error.keyword) {
case 'required':
return `The ${error.params.missingProperty} property is required`;
case 'type':
console.log(error);
return `${
error.instancePath ? `"${error.instancePath}"` : 'Value'
} should be of type ${error.params.type}`;
case 'additionalProperties':
return `The "${error.params.additionalProperty}" property is not allowed`;
default:
return error.message;
}
}
packages/backend-openapi-utils/src/schema/validation.test.ts
+5 -5
View File
@@ -164,11 +164,11 @@ describe('OpenApiProxyValidator', () => {
statusCode: 200,
});
await expect(
async () => await validator.validate(request, response),
).rejects.toThrowErrorMatchingInlineSnapshot(
`"["GET /api/search"] Query parameter validation failed"`,
);
await expect(async () => await validator.validate(request, response))
.rejects.toThrowErrorMatchingInlineSnapshot(`
"["GET /api/search"] Query parameter validation failed.
- Value should be of type number"
`);
});
it('accepts valid parameter', async () => {
packages/backend-openapi-utils/src/schema/validation.ts
+10 -1
View File
@@ -24,7 +24,7 @@ import { RequestBodyParser } from './request-body-validation';
import { mockttpToFetchRequest, mockttpToFetchResponse } from './utils';
import { ResponseBodyParser } from './response-body-validation';
const ajv = new Ajv({ allErrors: true }); // options can be passed, e.g. {allErrors: true}
const ajv = new Ajv({ allErrors: true });
class RequestBodyValidator implements Validator {
schema: OpenAPIObject;
@@ -34,6 +34,7 @@ class RequestBodyValidator implements Validator {
async validate({ pair, operation }: ValidatorParams) {
const { request } = pair;
// NOTE: There may be a worthwhile optimization here to cache these results to avoid re-parsing the schema for every request. As is, I don't think this is a big deal.
const parser = RequestBodyParser.fromOperation(operation, { ajv });
const fetchRequest = mockttpToFetchRequest(request);
await parser.parse(fetchRequest);
@@ -48,12 +49,19 @@ class ResponseBodyValidator implements Validator {
async validate({ pair, operation }: ValidatorParams) {
const { response } = pair;
// NOTE: There may be a worthwhile optimization here to cache these results to avoid re-parsing the schema for every request. As is, I don't think this is a big deal.
const parser = ResponseBodyParser.fromOperation(operation, { ajv });
const fetchResponse = mockttpToFetchResponse(response);
await parser.parse(fetchResponse);
}
}
/**
* Find an operation in an OpenAPI schema that matches a request. This is done by comparing the request URL to the paths in the schema.
* @param openApiSchema - The OpenAPI schema to search for the operation in.
* @param request - The request to find the operation for.
* @returns A tuple of the path and the operation object that matches the request.
*/
export function findOperationByRequest(
openApiSchema: OpenAPIObject,
request: CompletedRequest,
@@ -72,6 +80,7 @@ export function findOperationByRequest(
if (pathParts[i] === parts[i]) {
continue;
}
// If the path part is a parameter, we can count it as a match. eg /api/{id} will match /api/1
if (pathParts[i].startsWith('{') && pathParts[i].endsWith('}')) {
continue;
}