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

HTTP status codes and responses #206

Closed
wants to merge 7 commits into from
Closed

Conversation

adeinega
Copy link
Contributor

@adeinega adeinega commented Mar 5, 2021

No description provided.

@netlify
Copy link

netlify bot commented Mar 5, 2021

Deploy preview for gnap-core-protocol-editors-draft ready!

Built without sensitive environment variables with commit 7db9ff3

https://deploy-preview-206--gnap-core-protocol-editors-draft.netlify.app

@jricher
Copy link
Collaborator

jricher commented Mar 5, 2021

In general I think we're holding off on defining error codes until the core protocol is more solid, but thanks for this! See #79 for the issue tracking this major hole in the spec.

@adeinega
Copy link
Contributor Author

adeinega commented Mar 5, 2021

I was a bit surprised that the draft says nothing about any HTTP status codes in case of errors. 400 (and 401) are the most important ones in my opinion. These changes just a little step in this direction... without defining detailed error codes.

@adeinega adeinega changed the title An HTTP 400 status code for the error response HTTP status codes and responses Mar 5, 2021
@jricher
Copy link
Collaborator

jricher commented Mar 17, 2021

Not accepted, will address both error codes and response codes in a more comprehensive edit in the future.

@jricher jricher added the Pending Close Issue will be closed unless there are changes to consensus label Mar 17, 2021
@Denisthemalice
Copy link

I concur with adeinega when he says:

I was a bit surprised that the draft says nothing about any HTTP status codes in case of errors.

Common error codes should be mentioned in the draft, in particular 401, 403 and 405.

This will augment the size of the document of about one page, but it is worth to do it, in particular
to make the difference between an authentication error (401) and an authorization error (403).

@jricher
Copy link
Collaborator

jricher commented Mar 17, 2021

The editors have agreed (and stated above) that error codes need to be added to the draft. However, that should be done in a more comprehensive manner than what is described in this PR and so we will address it in a future update. Nobody is saying that we should not add error codes, so please don't imply otherwise.

@adeinega
Copy link
Contributor Author

adeinega commented Mar 19, 2021

GNAP is closely tied with The Hypertext Transfer Protocol (HTTP) and, as a starting point, it is worth specifying that

  1. 200 (OK) indicates that the request succeeded
  2. 400 (BAD REQUEST) indicates that the request hasn't succeeded

before we go any further and identify detailed error codes and this is exactly what I suggested doing in this PR. Although, those are pretty minor changes and if the editors / WG decide to not include them, I'm OK.

This specification could mention lots of other nuances in this area... such as redirect policies (if there are any) for the grant, introspect, continuation, as well as other endpoints. The use and presence of standard response headers like Date, Content-Length and Content-Encoding, an HTTP 429 (Too Many Requests) status code as a way to control the rate of requests, the HTTP Retry-After header, and so forth. This list goes on and on.

@jricher jricher closed this Mar 26, 2021
@jricher jricher removed the Pending Close Issue will be closed unless there are changes to consensus label Mar 26, 2021
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

Successfully merging this pull request may close these issues.

None yet

3 participants