handle error transform via middleware

README.md

@@ -24,25 +24,8 @@ new OpenApiMiddleware({
 
 ```javascript
 new OpenApiMiddleware({
-
   // required: path to an openapi 3 spec
   apiSpecPath: './openapi.yaml',
-
-  // default true: validates the openapi spec, throws if invalid
-  validateApiDoc: true,
-
-  // default: trueattempts to coerce a value's type to that defined in the openapi spec
-  enableObjectCoercion: true,
-
-  // optional: provide a custom error transform to customize how errors are shaped
-  errorTransform: validationResult => ({
-    // the http status code to return
-    statusCode: validationResult.status,
-    // the custom error object to return
-    error: {
-      code: validationResult.status,
-      message: validationResult.errors[0].message,
-    },
   }),
 }).install(app);
 ```
@@ -99,7 +82,7 @@ console.log('Listening on port 3000');
 module.exports = app;
 ```
 
-## [Example API Server (Full Project Source)](https://github.com/cdimascio/express-middleware-openapi-example) 
+## [Example API Server (Full Project Source)](https://github.com/cdimascio/express-middleware-openapi-example)
 
 A fully working example lives [here](https://github.com/cdimascio/express-middleware-openapi-example)
 

package.json

@@ -14,6 +14,7 @@
   "dependencies": {
     "js-yaml": "^3.12.2",
     "lodash": "^4.17.11",
+    "ono": "^5.0.1",
     "openapi-request-coercer": "^2.3.0",
     "openapi-request-validator": "^3.7.0",
     "openapi-schema-validator": "^3.0.3",

src/errors/index.ts

@@ -1,19 +1,13 @@
-export const methodNotAllowed = (path, method) => ({
-  status: 405,
+export const validationError = (
+  status: number,
+  path: string,
+  message: string
+) => ({
+  status,
   errors: [
     {
       path,
-      message: `${method} method not allowed`,
-    },
-  ],
-});
-
-export const notFoundError = path => ({
-  status: 404,
-  errors: [
-    {
-      path,
-      message: 'Not found',
+      message,
     },
   ],
 });

src/index.ts

@@ -5,14 +5,13 @@ import { OpenApiContext } from './openapi.context';
 import * as middlewares from './middlewares';
 
 export interface ErrorResponse {
-  statusCode: number;
+  status: number;
   error: any;
 }
 
 export interface OpenApiMiddlewareOpts extends OpenAPIFrameworkArgs {
   name: string;
   apiSpecPath: string;
-  errorTransformer?: (validationResult: any) => ErrorResponse;
 }
 
 export function OpenApiMiddleware(opts: OpenApiMiddlewareOpts) {
@@ -54,7 +53,6 @@ OpenApiMiddleware.prototype.install = function(app: ExpressApp) {
       apiDoc: this.apiDoc,
       loggingKey: this.opts.name,
       enableObjectCoercion: this.opts.enableObjectCoercion,
-      errorTransformer: this.opts.errorTransformer,
     })
   );
 };

src/middlewares/openapi.metadata.ts

@@ -5,12 +5,10 @@ import { OpenApiContext } from '../openapi.context';
 export function applyOpenApiMetadata(openApiContext: OpenApiContext) {
   return (req, res, next) => {
     req.openapi = {};
-    console.log('applyOpenApiMetadata: applying metadata to request', req.path);
-    const matched = matchRoute(req);
+    const matched = lookupRoute(req);
 
     if (matched) {
       const { expressRoute, openApiRoute, pathParams, schema } = matched;
-      console.log('applyOpenApiMetadata', expressRoute, openApiRoute);
       req.openapi.expressRoute = expressRoute;
       req.openapi.openApiRoute = openApiRoute;
       req.openapi.pathParams = pathParams;
@@ -20,12 +18,11 @@ export function applyOpenApiMetadata(openApiContext: OpenApiContext) {
     next();
   };
 
-  function matchRoute(req) {
+  function lookupRoute(req) {
     const path = req.path;
     const method = req.method;
-    for (const [expressRoute, methods] of Object.entries(
-      openApiContext.expressRouteMap
-    )) {
+    const routeEntries = Object.entries(openApiContext.expressRouteMap);
+    for (const [expressRoute, methods] of routeEntries) {
       const schema = methods[method];
       const routePair = openApiContext.routePair(expressRoute);
       const openApiRoute = routePair.openApiRoute;
@@ -35,15 +32,14 @@ export function applyOpenApiMetadata(openApiContext: OpenApiContext) {
       const matchedRoute = regexp.exec(path);
 
       if (matchedRoute) {
-        console.log('core_mw: matchRoute', matchedRoute);
-        console.log('core_mw: matchRoute:keys', keys);
         const paramKeys = keys.map(k => k.name);
         const paramsVals = matchedRoute.slice(1);
         const pathParams = _.zipObject(paramKeys, paramsVals);
-        console.log('core_mw: create params', pathParams);
+
         return {
           schema,
-          // schema may or may not contain express and openApi routes, so include them here
+          // schema may or may not contain express and openApi routes,
+          // thus we include them here
           expressRoute,
           openApiRoute,
           pathParams,

src/middlewares/openapi.request.validator.ts

@@ -1,30 +1,22 @@
 import OpenAPIRequestValidator from 'openapi-request-validator';
 import OpenAPIRequestCoercer from 'openapi-request-coercer';
-import { methodNotAllowed, notFoundError } from '../errors';
-
-export function validateRequest({
-  apiDoc,
-  loggingKey,
-  enableObjectCoercion,
-  errorTransformer,
-}) {
+import { validationError } from '../errors';
+import ono from 'ono';
+export function validateRequest({ apiDoc, loggingKey, enableObjectCoercion }) {
   return (req, res, next) => {
-    console.log(
-      'validateRequest_mw: ',
-      req.openapi.openApiRoute,
-      req.openapi.expressRoute
-    );
     const path = req.openapi.expressRoute;
     if (!path) {
-      const err = notFoundError(req.path);
-      return sendValidationError(res, err, errorTransformer);
+      const message = 'not found';
+      const err = validationError(404, req.path, message);
+      throw ono(err, message);
     }
     const schema = req.openapi.schema;
     if (!schema) {
       // add openapi metadata to make this case more clear
       // its not obvious that missig schema means methodNotAllowed
-      const err = methodNotAllowed(path, req.method);
-      return sendValidationError(res, err, errorTransformer);
+      const message = `${req.method} method not allowed`;
+      const err = validationError(405, req.path, message);
+      throw ono(err, message);
     }
 
     if (!schema.parameters) {
@@ -49,7 +41,8 @@ export function validateRequest({
     }
 
     const validationResult = new OpenAPIRequestValidator({
-      //   errorTransformer, // TODO create custom error transformere here as there are a lot of props we can utilize
+      // TODO create custom error transformere here as there are a lot of props we can utilize
+      // errorTransformer,
       parameters: schema.parameters || [],
       requestBody: schema.requestBody,
       // schemas: this.apiDoc.definitions, // v2
@@ -59,33 +52,9 @@ export function validateRequest({
     }).validate(req);
 
     if (validationResult && validationResult.errors.length > 0) {
-      return sendValidationError(res, validationResult, errorTransformer);
+      const message = validationResult.errors[0].message;
+      throw ono(validationResult, message);
     }
     next();
   };
 }
-
-function sendValidationError(res, validationResult, transformer) {
-  console.log(
-    'validateRequest_mw: validation error',
-    validationResult,
-    transformer
-  );
-  if (!validationResult) throw Error('validationResult missing');
-
-  const transform =
-    transformer ||
-    (v => ({
-      statusCode: v.status,
-      // TODO content-type shoudl be set and retuned
-      error: { errors: v.errors },
-    }));
-  const x = transform(validationResult);
-  if (!x || !x.statusCode || !x.error) {
-    throw Error(
-      'invalid error transform. must return an object with shape { statusCode, error}'
-    );
-  }
-  // TODO throw rather than returning a result
-  return res.status(x.statusCode).json(x.error);
-}

src/openapi.spec.loader.ts

@@ -29,7 +29,6 @@ export class OpenApiSpecLoader {
       ...(args as OpenAPIFrameworkArgs),
     };
 
-    console.log(frameworkArgs);
     const framework = new OpenAPIFramework(frameworkArgs);
     return framework;
   }

test/app.common.ts

@@ -43,7 +43,7 @@ export function routes(app) {
 
   app.post('/v1/pets', function(req, res, next) {
     res.json({
-      test: 'hi',
+      id: 'new-id',
     });
   });
 
@@ -61,7 +61,8 @@ export function routes(app) {
 
   app.get('/v1/pets/:id/attributes/:attribute_id', function(req, res, next) {
     res.json({
-      id: req.params.attribute_id,
+      id: req.params.id,
+      attribute_id: req.params.attribute_id,
     });
   });
 

test/app.ts

@@ -18,17 +18,17 @@ app.use(express.static(path.join(__dirname, 'public')));
 
 new OpenApiMiddleware({
   apiSpecPath: './openapi.yaml',
-  // validateApiDoc: true, // the default
-  // enableObjectCoercion: true, // the default
-  // errorTransformer: (a, b) => {
-  //   console.log('---error trans---', a, b);
-
-  //   return a;
-  // },
 }).install(app);
 
 routes(app);
 
+// Register error handler
+app.use((err, req, res, next) => {
+  res.status(err.status).json({
+    errors: err.errors,
+  });
+});
+
 startServer(app, 3000);
 
 export default app;

test/app.with.transform.ts

@@ -19,21 +19,19 @@ app.use(express.static(path.join(__dirname, 'public')));
 
 new OpenApiMiddleware({
   apiSpecPath: './openapi.yaml',
-  validateApiDoc: true, // is the default
-  enableObjectCoercion: true, // should be default
-  errorTransformer: e => {
-    return {
-      statusCode: e.status,
-      error: {
-        code: e.status,
-        message: e.errors[0].message,
-      },
-    };
-  },
 }).install(app);
 
 routes(app);
 
+// Register error handler
+app.use((err, req, res, next) => {
+  const firstError = err.errors[0];
+  res.status(err.status).json({
+    code: err.status,
+    message: firstError.message,
+  });
+});
+
 startServer(app, 3001);
 
 export default app;

test/error.transform.spec.ts

@@ -10,7 +10,6 @@ describe('custom error transform', () => {
       .expect(400)
       .then(r => {
         const e = r.body;
-
         expect(e.code).equals(400);
         expect(e.message).equals('should be integer');
       });

test/router1.spec.ts

@@ -4,13 +4,14 @@ import app from './app';
 const packageJson = require('../package.json');
 
 describe(packageJson.name, () => {
-  it('should throw 200 on route from new router', async () =>
+  it('should throw 404 on a route defined in express, but not documented in the openapi spec', async () =>
     request(app)
       .get('/v1/router1/10')
       .set('Accept', 'application/json')
       .expect(404)
       .then(r => {
-        const e = r.body;
-        console.log('--ERROR BODY--', e);
+        const e = r.body.errors[0];
+        expect(e.message).to.equal('not found')
+        expect(e.path).to.equal('/v1/router1/10')
       }));
 });

test/routes.spec.ts

@@ -93,7 +93,7 @@ describe(packageJson.name, () => {
         })
         .expect(200)
         .then(r => {
-          console.log(r.body);
+          expect(r.body.id).to.equal('new-id');
         }));
   });
 
@@ -139,7 +139,7 @@ describe(packageJson.name, () => {
         .expect(404)
         .then(r => {
           const e = r.body.errors;
-          expect(e[0].message).to.equal('Not found');
+          expect(e[0].message).to.equal('not found');
           expect(e[0].path).to.equal('/v1/unknown_route');
         }));
 
@@ -152,7 +152,7 @@ describe(packageJson.name, () => {
         .expect(404)
         .then(r => {
           const e = r.body.errors;
-          expect(e[0].message).to.equal('Not found');
+          expect(e[0].message).to.equal('not found');
           expect(e[0].path).to.equal(
             '/v1/route_defined_in_express_not_openapi'
           );
@@ -196,15 +196,15 @@ describe(packageJson.name, () => {
         });
     });
 
-    it('should return 400 when path param should be int, but instead is string', async () => {
-      const id = 10;
-      const attributeId = 12;
+    it('should handle multiple path params with coereion', async () => {
+      const id = '10';
+      const attributeId = '12';
       return request(app)
         .get(`/v1/pets/${id}/attributes/${attributeId}`)
         .expect(200)
         .then(r => {
-          const e = r.body.errors;
-          // TODO add assert
+          expect(r.body.id).equals(Number.parseInt(id));
+          expect(r.body.attribute_id).equals(Number.parseInt(attributeId));
         });
     });