Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add exceptions to ExceptionWrapper #4

Closed
wagenet opened this issue May 1, 2019 · 6 comments
Closed

Add exceptions to ExceptionWrapper #4

wagenet opened this issue May 1, 2019 · 6 comments
Milestone

Comments

@wagenet
Copy link
Collaborator

wagenet commented May 1, 2019

Work has begun here: https://github.com/wagenet/graphiti-rails/blob/master/lib/graphiti/rails/railtie.rb#L6.

For reference, this is the list of registered exceptions in my current app (without graphiti-rails):

{"ActionController::RoutingError"=>:not_found,
 "AbstractController::ActionNotFound"=>:not_found,
 "ActionController::MethodNotAllowed"=>:method_not_allowed,
 "ActionController::UnknownHttpMethod"=>:method_not_allowed,
 "ActionController::NotImplemented"=>:not_implemented,
 "ActionController::UnknownFormat"=>:not_acceptable,
 "ActionController::InvalidAuthenticityToken"=>:unprocessable_entity,
 "ActionController::InvalidCrossOriginRequest"=>:unprocessable_entity,
 "ActionDispatch::Http::Parameters::ParseError"=>:bad_request,
 "ActionController::BadRequest"=>:bad_request,
 "ActionController::ParameterMissing"=>:bad_request,
 "Rack::QueryParser::ParameterTypeError"=>:bad_request,
 "Rack::QueryParser::InvalidParameterError"=>:bad_request,
 "ActiveRecord::RecordNotFound"=>:not_found,
 "ActiveRecord::StaleObjectError"=>:conflict,
 "ActiveRecord::RecordInvalid"=>:unprocessable_entity,
 "ActiveRecord::RecordNotSaved"=>:unprocessable_entity,
 "Survey::NotFound"=>:not_found,
 "Survey::PreconditionNotMet"=>:unprocessable_entity}
@wagenet wagenet added this to the 1.0 milestone May 7, 2019
@wagenet
Copy link
Collaborator Author

wagenet commented May 7, 2019

@wadetandy @richmolj you mentioned that you had an internal mapping of all of this that you probably should open source. Unless you're able to do that soon can you send a PR or share that list with me. We can implement it manually here for now and then de-duplicate later if you open source the other library.

@richmolj
Copy link
Collaborator

richmolj commented May 8, 2019

I think we're going to be disappointing here - less a library and more a small bit of configuration. I think the only significant one is a custom NotAuthorizedError => 403, and a handful of mappings specific to our applications.

I wonder if errors.rb within Graphiti should require each error to have a status code (maybe a superclass sets it). This way we could auto-generate the config in the Railtie, instead of bumping multiple gems when we add a new error class.

@wagenet
Copy link
Collaborator Author

wagenet commented May 8, 2019

I'd definitely like to have a central source instead of defining it in graphiti-rails!

If we're following Rails conventions, I think most Graphiti errors will map to :unprocessable_entity (422) for validation failures, and :bad_request (400) for unreadable data or non-sensical requests. We may also have some :conflict (409).

@wagenet
Copy link
Collaborator Author

wagenet commented May 8, 2019

The relevant JSON:API spec is here https://jsonapi.org/format/.

Some key items:

400

If an endpoint does not support the include parameter, it MUST respond with 400 Bad Request to any requests that include it.

If a server is unable to identify a relationship path or does not support inclusion of resources from a path, it MUST respond with 400 Bad Request.

If the server does not support sorting as specified in the query parameter sort, it MUST return 400 Bad Request.

If a server encounters a query parameter that does not follow the naming conventions above, and the server does not know how to process it as a query parameter from this specification, it MUST return 400 Bad Request.

403

A server MUST return 403 Forbidden in response to an unsupported request to create a resource with a client-generated ID.

A server MAY return 403 Forbidden in response to an unsupported request to create a resource.

A server MAY reject an attempt to do a full replacement of a to-many relationship. In such a case, the server MUST reject the entire update, and return a 403 Forbidden response.

404

A server MUST return 404 Not Found when processing a request to modify a resource that does not exist.

A server MUST return 404 Not Found when processing a request that references a related resource that does not exist.

A server MUST respond with 404 Not Found when processing a request to fetch a single resource that does not exist, except when the request warrants a 200 OK response with null as the primary data (as described above).

A server MUST return 404 Not Found when processing a request to fetch a relationship link URL that does not exist.

If a relationship link URL exists but the relationship is empty, then 200 OK MUST be returned, as described above.

A server SHOULD return a 404 Not Found status code if a deletion request fails due to the resource not existing.

406

Servers MUST respond with a 406 Not Acceptable status code if a request’s Accept header contains the JSON:API media type and all instances of that media type are modified with media type parameters.

409

A server MUST return 409 Conflict when processing a POST request to create a resource with a client-generated ID that already exists.

A server MUST return 409 Conflict when processing a POST request in which the resource object’s type is not among the type(s) that constitute the collection represented by the endpoint.
A server SHOULD include error details and provide enough information to recognize the source of the conflict.

A server MAY return 409 Conflict when processing a PATCH request to update a resource if that update would violate other server-enforced constraints (such as a uniqueness constraint on a property other than id).

A server MUST return 409 Conflict when processing a PATCH request in which the resource object’s type and id do not match the server’s endpoint.

415

Servers MUST respond with a 415 Unsupported Media Type status code if a request specifies the header Content-Type: application/vnd.api+json with any media type parameters.

422

In the responses ... with a 422 status code, 400 Bad Request would also be acceptable. (More details.) JSON:API doesn’t take a position on 400 vs. 422.

I propose sticking with Rails convention and using 422.

Also relevant

When a server encounters multiple problems for a single request, the most generally applicable HTTP error code SHOULD be used in the response. For instance, 400 Bad Request might be appropriate for multiple 4xx errors or 500 Internal Server Error might be appropriate for multiple 5xx errors.

@wagenet
Copy link
Collaborator Author

wagenet commented May 8, 2019

Also, somehow @richmolj has traveled into the future... at least according to his comment time.

@wagenet
Copy link
Collaborator Author

wagenet commented May 8, 2019

I think this is handled as well as we can for now.

@wagenet wagenet closed this as completed May 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants