Fix secondary error when handing errors. #59
Conversation
Not all errors are guaranteed to have a code. Setting status to an undefined code is itself another error which masks the original error.
|
@typingduck So what HTTP status ends up being returned to the client after this change is applied? What's an example of cases where no HTTP status code would be found here? |
|
I tested this patch. Before the patch was applied, my server was crashing with: So, a patch is definitely useful here. After applying the patch, the server returned a 200 status code with content like this: The description helpfully points to an error in my code, but the status code of "200" is wrong. In this case, an HTTP status code of 500 would have been appropriate-- this was a server-side error. The question is whether "500" is always a good default status code when the code would otherwise be set to undefined. Defaulting to 500 would at least be an improvement over the current crashing behavior that happens now. |
| @@ -156,7 +156,9 @@ var handleError = function(e, req, res, response, next) { | |||
| res.set(response.headers); | |||
| } | |||
|
|
|||
| res.status(e.code); | |||
| if (e.code) { | |||
There was a problem hiding this comment.
Nice work, could you add some tests for that, since it got fail for @markstos ?
Not all errors are guaranteed to have a code. Setting status to an undefined code is itself another error which masks the original error.