Fixed bug in Response validator

[ronniel1]

Response validator replaces res.json with a custom function.
In case the user configured passError option, we need to restore the
original res.json function so that the error handling middleware can
update the response.

[itsikbelson-spotlight]

Looks good

[ronniel1]

Unfortunately this is not good enough.
If the request handler had an error, and calls next(err) instead of res.status(x).send(...) then res.json will not be restored, and then the error handling middleware will change the json the response validator will be invoked, and override the json response, which it shouldn't.

Any ideas how this can be resolved?

[ronniel1]

Added another commit that fixes the issue, in a hackish way.
The problem is how to cleanup the res.json patch when the response validator is not being called.
exported a function to cleanup the Response object, which can be called from the error handling middleware.

I don't really like this, but did not find another solution.

[evanshortiss]

Thank you for this PR! Can you take a look at the comments?

[evanshortiss]

I think it'd be best not to expose this. Make the fix-up automatic in all scenarios, including response, if passError is enabled. WDYT?

[evanshortiss]

Ah, I see your last comment. Right, this is a tricky...

[ronniel1]

This is indeed Hacky. Do you have any other suggestion?

[evanshortiss]

I had this idea stewing in my head around providing schemas for certain status code ranges. It's a bit more complex though. Something like:

validation.response({
  // Schemas to run for given status codes
  '2xx': schemaSuccess,
  '4xx': schemaFail
}, {
  // These could be defined globally instead of as part of 
  // the call to response()
  passError: true
  passErrorSchemas: {
    '5xx': errrorHandlerSchema
  }
})

If no schema is provided for 5xx, then perhaps validation is bypassed?

[ronniel1]

This seems like a very good interface.
It seems to me that it will still need the mechanism of overriding the json function on the Response object and require restoring the original json function

[evanshortiss]

This should be a 500 error, yet the test is passing. That's odd. Checking res.statusCode in the end block instead will do the trick.

I'll need to look into why supertest is not asserting it correctly since it seems to be the same for other tests...

[ronniel1]

The errorHandler sets the status code to 422, so I think this works as expected