feat: adds support to http problem details - rfc7807 #1786
Conversation
adds supports to handle error objects comforming with the http problem details spec (RFC7807). the spec defines a `status` field instead of `statusCode` and a `application/problem+json` content type.
DonutEspresso
requested review from
hekike,
DonutEspresso,
daviande and
retrohacker
| @@ -1435,7 +1435,7 @@ Server.prototype._routeErrorResponse = function _routeErrorResponse( | |||
| } | |||
|
|
|||
| // only automatically send errors that are known (e.g., restify-errors) | |||
| if (err instanceof Error && _.isNumber(err.statusCode)) { | |||
| if (err instanceof Error && _.isNumber(err.statusCode || err.status)) { | |||
There was a problem hiding this comment.
when is this status? I
restify-errors does statusCode
https://github.com/restify/errors/blob/master/lib/baseClasses/HttpError.js#L54
There was a problem hiding this comment.
exactly, and restify-errors doesn't conform to rfc7807, which defines status. The problem is that it assumes that users are indeed using restify-errors and don't give them the ability to follow the standard.
This PR keeps statusCode for backward compatibility with restify-errors but allows restify to return the correct content type application/problem+json when status is used and no statusCode is present
There was a problem hiding this comment.
class NotFound extends Error {
constructor(detail) {
this.title = 'Not Found';
this.type = 'https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/404';
this.status = 404;
this.detail = detail || 'Resource not found';
}
toJSON() {
return this;
}
}this will conform with rfc7807 and will hit that code with status
There was a problem hiding this comment.
You know what? It doesn't really need to "have a status property instead of a statusCode" to conform to the rfc. I was mainly using status property as a way to identify that was an error message complying with the rfc and sending the correct content-type.
But we can have a statusCode property in the error object and on the toJSON method that I serialize it to status instead, but I would need to find a way to set the application/problem+json content type and still keep backward compatibility. Any thoughts on this?
| @@ -389,7 +389,7 @@ function patch(Response) { | |||
| // If the body is an error object and we were not given a status code, | |||
| // try to derive it from the error object, otherwise default to 500 | |||
| if (!code && body instanceof Error) { | |||
| code = body.statusCode || 500; | |||
| code = body.statusCode || body.status || 500; | |||
Pre-Submission Checklist
make prepushChanges
adds supports to handle error objects conforming with the http
problem details spec (rfc7807).
The spec defines a
statusfield (instead ofstatusCode) and aapplication/problem+jsoncontent type. Code will prefer the currentbehavior of sending
application/jsonifstatusCodeis present.