Refactor Fetchr Tests #143
Conversation
|
CLA is valid! |
|
@redonkulus @mridgway @lingyan this is ready for merge. |
| @@ -335,7 +335,14 @@ Fetcher.middleware = function (options) { | |||
| } | |||
| if (err) { | |||
| var errResponse = getErrorResponse(err); | |||
| res.status(errResponse.statusCode).json(responseFormatter(req, res, errResponse.output)); | |||
| if (req.query && req.query.returnMeta) { | |||
There was a problem hiding this comment.
@Vijar can you explain this part? will this break responseFormatter since output is not passed in directly?
There was a problem hiding this comment.
i think we are fine. We are already doing req.query.returnMeta logic branching on line 348 below
There was a problem hiding this comment.
Yeah, we were doing this for normal requests but not for error requests so its kind of a bug.
| @@ -193,6 +193,10 @@ function io(url, options) { | |||
| err = new Error(errMessage); | |||
| err.statusCode = status; | |||
| err.body = errBody || body; | |||
| if (err.body) { | |||
There was a problem hiding this comment.
should we add a deprecate warning in the next release for body?
There was a problem hiding this comment.
I don't see why we can't keep both.
There was a problem hiding this comment.
Do we need to update docs to make sure only one is mentioned?
There was a problem hiding this comment.
We don't even have docs on handling errors, but I just added some.
|
👍 |
|
👍 |
A preview of whats to come.
Fixes #141