Swagger Validation middleware fails to parse/process error incase of malformed JSON structure

[gargkeshav]

When I pass an invalid structure/attribute type, swagger validator middleware throws the proper error and pass it down the flow using next which we are catching using custom error handler but when a malformed json object is passed it sends an error response with 200 HTTP status. We don't have any code that is generating this response, I tried to Debug and looks like it's happening somewhere from body-parser from within swagger middleware not sure though. Please note that there is a comma missing after first argument in request body

Request Body

[
  {

    "testArgument": "test123"
    "arg1": "abcd",
    "arg2": "xyz"

    "agr3": [
      {
        "arg4": "gaining",

      }
    ]
  }
]

response

{
    "message": "Unexpected string in JSON at position 48",
    "expose": true,
    "statusCode": 400,
    "status": 400,
    "body": "[\r\n  {\r\n  \t\r\n    \"testArgument\": \"test123\"\r\n    \"arg1\": \"abcd\",\r\n    \"arg2\": \"xyz\"\r\n\r\n    \"agr3\": [\r\n      {\r\n        \"arg4\": \"gaining\",\r\n\r\n      }\r\n    ]\r\n  }\r\n]",
    "type": "entity.parse.failed"
}

server.js

const express = require('express');
const swaggerObject = require('./../swagger/swagger.json');
const swagger =  require('swagger-tools');

const port = process.env.PORT || 3000;
const app = express();

const jsonErrorFormatter = require('./helpers/jsonErrorFormatter');

swagger.initializeMiddleware(swaggerObject, (middleware) => {
    app.use(middleware.swaggerMetadata());
    app.use(jsonErrorFormatter);
    app.use(middleware.swaggerValidator({
        validateResponse: false
    }));
    app.post('/v1/portrequests/', (req, res) => {
     res.send({}).status(201);
  });
    app.use(jsonErrorFormatter);
    app.listen(port, () => console.log(`Server listening on port ${port}`));

});

jsonErrorFormatter

const util = require('util');
const _ = require('lodash');

module.exports = function (err, req, res, next) {
    if (util.isError(err)) {
        Object.defineProperty(err, 'message', { enumerable: true});
        res.json(err).status(444);
    } else {
        if (_.isFunction(next)) {
            next();
        } else if (_.isFunction(res)) {
            res();
        }
    }
  };

Debug Logs

DEBUG=swagger-tools:middleware* node ./v1/server/server.js
  swagger-tools:middleware Initializing middleware +0ms
  swagger-tools:middleware   Identified Swagger version: 2.0 +17ms
  swagger-tools:middleware   Validation: succeeded +402ms
  swagger-tools:middleware:metadata Initializing swagger-metadata middleware +0ms
  swagger-tools:middleware:metadata   Identified Swagger version: 2.0 +4ms
  swagger-tools:middleware:metadata   Found Path: / +8ms
  swagger-tools:middleware:metadata   Found Path: /{id} +2ms
  swagger-tools:middleware:validator Initializing swagger-validator middleware +0ms
  swagger-tools:middleware:validator   Response validation: disabled +4ms
Server listening on port 3000
  swagger-tools:middleware:metadata POST / +13s
  swagger-tools:middleware:metadata   Is a Swagger path: true +4ms
  swagger-tools:middleware:metadata   Is a Swagger operation: true +1ms
  swagger-tools:middleware:metadata   Processing Parameters +2ms
{
  "expose": true,
  "statusCode": 400,
  "status": 400,
  "body": "[\r\n  {\r\n  \t\r\n    \"testArgument\": \"test123\"\r\n    \"arg1\": \"abcd\",\r\n    \"arg2\": \"xyz\"\r\n\r\n    \"agr3\": [\r\n      {\r\n        \"arg4\": \"gaining\",\r\n\r\n      }\r\n    ]\r\n  }\r\n]",
  "type": "entity.parse.failed"
}

[whitlockjc]

swagger-tools never sends a response. In this case, express is sending the response because express has a default error handler that is doing it. You need to add an error handler to handle this yourself.

[gargkeshav]

Thanks @whitlockjc for such a quick response!!!

I understand that swagger-tools never return any response and just leaves/passes it to the next middleware to handle. I believe I misstated the issue/problem statement, Allow me to put it this way.

swagger-tools calls body-parser to parse the request and when body-parser fails to parse it sends an error with body-parser format to swagger-tools, which swagger-tools return to next middleware/express. I have added the custom error handler for Swagger errors but it fails to handle this error since it's of type "entity.parse.failed". Passing the error up as it is isn't a bad thing to do, but an error in swagger format makes more sense since body-parser is used internally by swagger-tools. There are many other dependencies of swagger-tools and handling errors of all libraries in custom error handler will be cumbersome.

your thoughts.?

[whitlockjc]

I'm not sure I understand what the "swagger format" is you speak of. All errors that swagger-tools encounters are sent downstream without modification. Now, all errors that swagger-tools creates, there is a format but it's typically as a result of some validation error where we need to provide specific details about.

I'm all ears on how to make things better but I'm not sure swagger-tools is being inconsistent and I'm not sure how to do things better. But if you have an idea, I'd love to hear it.

[gargkeshav]

Yes, I am referring to the same format which swagger uses to provide specific details around errors encountered during validation. How about having custom errors in swagger-tools with the same format which swagger currently uses and process/modify errors thrown by other modules/dependencies (which are internally used by swagger for ex. body-parser in this case) and mould it in the same format. This way user only needs to parse/handle errors in similar format since all are being passed up by swagger-tools.

[whitlockjc]

I'm just not sure how that would look. I'll give it some thought.