core icon indicating copy to clipboard operation
core copied to clipboard

RFC-7807 compatible hydra error response

Open nesl247 opened this issue 3 years ago • 1 comments

Description

Finally, the server SHOULD provide error descriptions using an [RFC7807] standard by using an application/problem+json response. When doing so, the server also MUST provide an additional header pointing to either the built-in Hydra http://www.w3.org/ns/hydra/error context or any JSON-LD context that maps the terms type, title, detail, status and instance the same way as the standard one.

The existing response is the Non RFC-7807 compliant error description using raw Hydra JSON-LD representation. as described here https://www.hydra-cg.com/spec/latest/core/#example-32-non-rfc-7807-compliant-error-description-using-raw-hydra-json-ld-representation.

nesl247 avatar Sep 21 '22 16:09 nesl247

Indeed. Compatibility with RFC7807 is new in Hydra and we didn't updated our implementation since it has been added. PR welcome!

dunglas avatar Sep 21 '22 21:09 dunglas

@dunglas how would you want it to be implemented? Would it be a BC breaking change or would it need to be a new error format jsonld+problem? Or would it be more appropriate to be called hydra because it's the format described by hydra itself, and not jsonld?

nesl247 avatar Sep 22 '22 18:09 nesl247

I think that we can make our current implementation of RFC7807 compatible with Hydra without introducing breaking changes (it's only new keys). Then we'll deprecate the old Hydra error format. WDYT?

dunglas avatar Sep 22 '22 22:09 dunglas

I think that would work. Let me take a look and submit a PR for that. If I run into any issues I’ll reply back here.

nesl247 avatar Sep 22 '22 23:09 nesl247

I'm starting to look into it, and the bigger problem that we need to figure out is the type property. This MUST be a URI format, which means we need to have some sort of response if someone follows it.

I have the following questions:

  • What would the URL prefix you would expect to have (e.g. /errors)?
  • What do we want the full URL to look like? Do we use /errors/<ExceptionName>? Or do we reuse ErrorCodeSerializableInterface::getErrorCode() and thus /errors/<errorCode>?
  • What do we serve at that URL?
    • Do we need to expand ErrorCodeSerializableInterface to an ErrorSerializableInterface so we can also have a getErrorTitle() and getErrorDescription()?
      • How do we want to handle translation here?
    • Do we need to support HTML, or just JSON formats?
    • What happens if the exception doesn't implement these interfaces?
  • Do we want to allow for overriding the URL to something custom, in case there is publicly hosted documentation not provided in the API?
  • How do we add the required Link: <http://www.w3.org/ns/hydra/error>; rel="http://www.w3.org/ns/json-ld#context" to the headers? The Normalizer doesn't work on the response, just on the body.

Unfortunately it seems while I intended this to be a rather simple change, it's actually quite complicated. We do have the option of skipping type, but that defeats one of the biggest purposes of 7807. Adding just the new detail field doesn't seem like a worthwhile change IMO.

nesl247 avatar Sep 28 '22 16:09 nesl247

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Nov 27 '22 16:11 stale[bot]

This isn't stale. It's requires feedback.

nesl247 avatar Nov 28 '22 01:11 nesl247

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Jan 27 '23 02:01 stale[bot]

Indeed this is missing feedbacks, @dunglas any idea?

soyuka avatar Jan 27 '23 09:01 soyuka

Hi, we talked about this with @dunglas and we definitely need to be in sync with the specification on this.

Basically this all means that we'll have a new type of resource (ErrorResource). The first step is to add this feature with a default ErrorResource on /errors that'll use the http code as IRI (/errrors/404). Your ErrorResource basically is an ApiResource and it will have a Provider that fetches whatever information we need. I suggest that we have a default ErrorProvider that has indeed the title of the error. Overriding will then be done using a special ErrorResource attribute that one may use on throwables.

How do we add the required Link: http://www.w3.org/ns/hydra/error; rel="http://www.w3.org/ns/json-ld#context" to the headers? The Normalizer doesn't work on the response, just on the body.

You need to add this to the AddLinkHeaderListener.

soyuka avatar Jan 31 '23 10:01 soyuka

@nesl247 if you have some comments please check the PR we started at #5433 would love your insight !

soyuka avatar Mar 06 '23 16:03 soyuka

I realized that PR #5433 also introduced changes on the GraphQL side which are not valid. { "errors": [ { "message": "Names must only contain [_a-zA-Z0-9] but \"hydra:title\" does not.", "extensions": {}, "stack": "GraphQLError: Names must only contain [_a-zA-Z0-9] but \"hydra:title\" does not." } ] }

What would be the desired behaviour for GraphQL? Should the error types also occur there, then we should fix this. If they are not relevant for GraphQL then we should just add graphqlOperations: [] to the #[ErrorResource] s and the schema will be valid again.

jotwea avatar Jul 24 '23 18:07 jotwea