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

Error handling is inconsistent and undocumented. #165

Open
tisto opened this issue Nov 26, 2016 · 3 comments

Comments

@tisto
Copy link
Member

commented Nov 26, 2016

We do not document our error handling (possible status codes and messages). In addition our current implementation is inconsistent, e.g. an error on the @login endpoint encapsulates the error type/message within an error attribute:

https://github.com/plone/plone.restapi/pull/164/files#diff-ba94937d2482a112f9b033a9a89deb4bR40

while an unauthorized request on a private document returns a top-level type/message json structure:

https://github.com/plone/plone.restapi/pull/164/files#diff-ba94937d2482a112f9b033a9a89deb4bR117

@tisto tisto added the 01 type: bug label Nov 26, 2016
@tisto tisto referenced this issue Mar 18, 2017
11 of 19 tasks complete
@csenger

This comment has been minimized.

Copy link
Contributor

commented Mar 21, 2017

Do we want to provide a error format suggestion in plone.rest? And if so, should it be different to the common format in plone.restapi?

When implementing it we should keep in mind that plone.restapi still announces providing non JSON responses later.

Beside documenting it and unifying our code we should:

provide an utility function

e.g.

  • in a utility function to create the error json (dict)

      def error_json(type, message=None, extras=None)

    where extras is a dict that updates the default errors dict.

  • and a method of plone.rest.Service and/or plone.restapi.Service to set the status and return the json

    def error(self, type, message=None, extras=None, status=400):

    To make the usage in a Service class easier

provide HTTP Exceptions:

    from plone.rest.exceptions import HTTPError

    if FAILED:
        raise HTTPError('Bad Request', 'missing or invalid Content-Type header')

The signature should look something like

    class HTTPError(type, message, extras=None, status=400):

plone.rest.RestService.__call__() should handle those exceptions.

  • set status code and reason
  • raise an exception to abort the transaction
  • Attach the correct json body to the exception.

For comparision:

@tisto

This comment has been minimized.

Copy link
Member Author

commented May 1, 2017

@tisto

This comment has been minimized.

Copy link
Member Author

commented Jun 20, 2019

RFC 7807 might be a good option:

https://blog.restcase.com/rest-api-error-handling-problem-details-response/
https://tools.ietf.org/html/rfc7807

We will discuss this at the Beethoven Sprint.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.