Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Accept response validation error handler #456

Merged
merged 4 commits into from
Nov 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 15 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -569,6 +569,21 @@ Determines whether the validator should validate responses. Also accepts respons
}
```

**onError:**

A function that will be invoked on response validation error, instead of the default handling. Useful if you want to log an error or emit a metric, but don't want to actually fail the request. Receives the validation error and offending response body.

For example:

```
validateResponses: {
onError: (error, body) => {
console.log(`Response body fails validation: `, error);
console.debug(body);
}
}
```

### ▪️ validateSecurity (optional)

Determines whether the validator should validate securities e.g. apikey, basic, oauth2, openid, etc
Expand Down
1 change: 1 addition & 0 deletions src/framework/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export type ValidateRequestOpts = {
export type ValidateResponseOpts = {
removeAdditional?: 'failing' | boolean;
coerceTypes?: boolean | 'array';
onError?: (err: InternalServerError, json: any) => void;
};

export type ValidateSecurityOpts = {
Expand Down
35 changes: 27 additions & 8 deletions src/middlewares/openapi.response.validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
OpenApiRequest,
OpenApiRequestMetadata,
InternalServerError,
ValidateResponseOpts,
} from '../framework/types';
import * as mediaTypeParser from 'media-typer';
import * as contentTypeParser from 'content-type';
Expand All @@ -30,11 +31,19 @@ export class ResponseValidator {
private validatorsCache: {
[key: string]: { [key: string]: ajv.ValidateFunction };
} = {};
private eovOptions: ValidateResponseOpts

constructor(openApiSpec: OpenAPIV3.Document, options: ajv.Options = {}) {
constructor(
openApiSpec: OpenAPIV3.Document,
options: ajv.Options = {},
eovOptions: ValidateResponseOpts = {}
) {
this.spec = openApiSpec;
this.ajvBody = createResponseAjv(openApiSpec, options);
this.eovOptions = eovOptions;

// This is a pseudo-middleware function. It doesn't get registered with
// express via `use`
(<any>mung).onError = (err, req, res, next) => {
return next(err);
};
Expand All @@ -60,13 +69,23 @@ export class ResponseValidator {
? accept.split(',').map((h) => h.trim())
: [];

return this._validate({
validators,
body,
statusCode,
path,
accepts, // return 406 if not acceptable
});
try {
return this._validate({
validators,
body,
statusCode,
path,
accepts, // return 406 if not acceptable
});
} catch (err) {
// If a custom error handler was provided, we call that
if (err instanceof InternalServerError && this.eovOptions.onError) {
this.eovOptions.onError(err, body)
} else {
// No custom error handler, or something unexpected happen.
throw err;
}
}
}
return body;
});
Expand Down
3 changes: 3 additions & 0 deletions src/openapi.validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ export class OpenApiValidator {
options.validateResponses = {
removeAdditional: false,
coerceTypes: false,
onError: null
};
}

Expand Down Expand Up @@ -272,6 +273,8 @@ export class OpenApiValidator {
return new middlewares.ResponseValidator(
apiDoc,
this.ajvOpts.response,
// This has already been converted from boolean if required
this.options.validateResponses as ValidateResponseOpts
).validate();
}

Expand Down
93 changes: 93 additions & 0 deletions test/response.validation.on.error.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
import * as path from 'path';
import { expect } from 'chai';
import * as request from 'supertest';
import { createApp } from './common/app';
import * as packageJson from '../package.json';

const apiSpecPath = path.join('test', 'resources', 'response.validation.yaml');

describe(packageJson.name, () => {
let app = null;

let onErrorArgs = null;
before(async () => {
// set up express app
app = await createApp(
{
apiSpec: apiSpecPath,
validateResponses: {
onError: function(_err, body) {
onErrorArgs = Array.from(arguments);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you also add a test, where you re-throw the error and/or create a new error. looks like should all work out, but having a test will be good.

if (body[0].id === 'bad_id_throw') {
throw new Error('error in onError handler');
}
}
},
},
3005,
app => {
app.get(`${app.basePath}/users`, (_req, res) => {
const json = ['user1', 'user2', 'user3'];
return res.json(json);
});
app.get(`${app.basePath}/pets`, (req, res) => {
let json = {};
if (req.query.mode === 'bad_type') {
json = [{ id: 'bad_id', name: 'name', tag: 'tag' }];
} else if (req.query.mode === 'bad_type_throw') {
json = [{ id: 'bad_id_throw', name: 'name', tag: 'tag' }];
}
return res.json(json);
});
app.use((err, _req, res, _next) => {
res.status(err.status ?? 500).json({
message: err.message,
code: err.status ?? 500,
});
});
},
false,
);
});

afterEach(() => {
onErrorArgs = null;
})

after(() => {
app.server.close();
});

it('custom error handler invoked if response field has a value of incorrect type', async () =>
request(app)
.get(`${app.basePath}/pets?mode=bad_type`)
.expect(200)
.then((r: any) => {
const data = [{ id: 'bad_id', name: 'name', tag: 'tag' }];
expect(r.body).to.eql(data);
expect(onErrorArgs.length).to.equal(2);
expect(onErrorArgs[0].message).to.equal('.response[0].id should be integer');
expect(onErrorArgs[1]).to.eql(data);
}));

it('custom error handler not invoked on valid response', async () =>
request(app)
.get(`${app.basePath}/users`)
.expect(200)
.then((r: any) => {
expect(r.body).is.an('array').with.length(3);
expect(onErrorArgs).to.equal(null);
}));

it('returns error if custom error handler throws', async () =>
request(app)
.get(`${app.basePath}/pets?mode=bad_type_throw`)
.expect(500)
.then((r: any) => {
const data = [{ id: 'bad_id_throw', name: 'name', tag: 'tag' }];
expect(r.body.message).to.equal('error in onError handler');
expect(onErrorArgs.length).to.equal(2);
expect(onErrorArgs[0].message).to.equal('.response[0].id should be integer');
expect(onErrorArgs[1]).to.eql(data);
}));
});