express-oauth-server icon indicating copy to clipboard operation
express-oauth-server copied to clipboard

Fix secondary error when handing errors.

Open typingduck opened this issue 7 years ago • 2 comments

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 avatar Aug 09 '17 20:08 typingduck

@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?

markstos avatar Jun 19 '18 18:06 markstos

I tested this patch. Before the patch was applied, my server was crashing with:

RangeError: Invalid status code: undefined

So, a patch is definitely useful here. After applying the patch, the server returned a 200 status code with content like this:

{
error: "TypeError",
error_description: "config.get(...).find is not a function"
}

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.

markstos avatar Jun 19 '18 21:06 markstos