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

Failed responses should put information in the response payload body instead of HTTP header #71

Open
rikedyp opened this issue Aug 7, 2024 · 3 comments

Comments

@rikedyp
Copy link
Contributor

rikedyp commented Aug 7, 2024

When using Jarvis in REST mode, req.Fail can put error information from ⎕EM or ⎕SI (depending on ErrorInfoLevel) into the HTTP response header as the "reason phrase". However, HTTP clients can ignore the reason phrase, proxies can change it and it was removed in the HTTP/2 spec.

I think the error information should be put as the response body/payload when req.Fail is invoked.

@bpbecker
Copy link
Contributor

bpbecker commented Aug 7, 2024

This is an interesting issue and deserves some discussion. It's not only applicable in REST mode, but also in JSON mode when the endpoint function can accept a left argument (which is the request object itself).

There are three types of errors -

  • ones the user anticipates - here the user can set the response payload to whatever he wants. GitHub and OpenAI provide error information in the response payload.
  • ones Jarvis detects - things like bad HTTP method, endpoint not found, etc. Jarvis sets an appropriate HTTP status code and message
  • unexpected errors - This is when then global error trap catches an error in the user code - Jarvis falls back to a 500 Internal Server Error.

We could decide to put information in the response payload. But what if there's already content there? Do we wipe it out and replace it with the error information? Do we append or prepend? My inclination is to do two things:

  • Provide a SendErrorInPayload setting which will direct Fail to replace any existing payload with error information based on ErrorInfoLevel.
  • Provide a "user hook function" setting OnFailFn which is the name of a function that Fail will call, passing the request object as an argument. Then the developer can go to town and do whatever he wants.

@JasonRivers
Copy link
Member

On your second point, one way I've dealt with some errors that are not "breaking" errors in APIs is having an error section in the payload, so for a JSON payload something like the following:

{
    "data": {
        "stuff": "actual payload"
    },
    "errors": {
        "errorcode": 123,
        "message": "human readable stuff",
        "details": "Bunch more information"
    }
}

I've also seen other API systems do this, where the "errors" section may or may not exist in the payload.

This way you still get the data that you requested, but if the error exists then your data may be truncated or inaccurate, for full errors, (item not found) I've also seen some APIs only issue and "error" section with a http 404 and the details in the JSON format (rather than HTML), too. - this often depends on the "Accept: application/json" header being set on request.

@bpbecker
Copy link
Contributor

bpbecker commented Aug 7, 2024

I've seen same sort of behavior in other APIs (GitHub and OpenAI for instance). But what to include in the payload is really up to the user. Especially in Jarvis' REST mode, the response could have whatever content-type the application decides. So, I think if the user uses SendErrorInPayload, we'll probably send back information in using application/json and document that if you get a non-2XX HTTP status and have SendErrorInPayload set the error information will be JSON. If the user uses OnFailFn, then he can do whatever his heart desires.

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

3 participants