feat: AIP-193 – Errors #3
Conversation
This adds a generic AIP for errors. There is probably a decent bit for us to discuss here. Some high-level notes: - I decided to represent expected JSON interfaces using TypeScript (rather than JSONSchema), which I perceive to be much better for human readability. - We should discuss/debate the proposed Error interface. It is mostly similar to what Google uses but with two fields "promoted" (we have a huge _mea culpa_ here). - I did not discuss any common _headers_ related to error handling (e.g. `Retry-After`). I personally think that `Retry-After` gets covered in AIP-194, and I could not think of any others that warranted inclusion here. I expect this is an area where everyone will need to make changes, but I also notice that entire sections can probably be adopted by everyone (e.g. "Messages"), so I think this should work reasonably well. Looking forward to the discussion on this.
|
Note:
|
| by using appropriate HTTP codes: | ||
|
|
||
| - Successful responses **must** use HTTP status codes between 200 and 399. | ||
| - Errors indicating a problem with the user's request **must** use HTTP status |
There was a problem hiding this comment.
We talked about this briefly in the WG, and I'm posting the question here again.
For parts of the proposal that do not map well with one or more of the "implementations" (say GraphQL in this case). How do we want to represent divergence?
For this specific case, in GraphQL there is a distinction between how to represent before GraphQL execution errors (e.g: malformed HTTP request to the server should yield 400) and say validation/business logic errors (e.g: GraphQL query syntax is invalid would yield a 200 status code with errors populated in the errors block of the body of the HTTP response)
There was a problem hiding this comment.
I would expect a GraphQL AIP to override this section.
There was a problem hiding this comment.
hey @innou is what you're saying how GraphQL actually does it? returning success codes for cases where there are syntax errors in the request? that sounds like a bad idea to me, but if that's how it's done, there's little that can be done. in my mind, that would not be a good pattern to recommend. RFC 6231 is pretty clear on that one, i think:
400 Bad Request:
The 400 (Bad Request) status code indicates that the server cannot or
will not process the request due to something that is perceived to be
a client error (e.g., malformed request syntax, invalid request
message framing, or deceptive request routing).
Raised this in the other PR. I would like your thoughts on what the sample should be and we should try to formalize it so that anybody (if anybody else writes these) could provide a sample and it is consistent with the others. |
|
|
||
| ### Structure | ||
|
|
||
| Error responses **should** conform to the following interface: |
There was a problem hiding this comment.
I'm guessing this is where you were expecting most of the comments :)
My recommendation would be to use RFC 7807
The short of it would be this:
interface Error {
//URL formated identifier. Does not have to be a functioning URL
type: string;
//A human readable description of the problem. Should not change from occurrence to occurrence.
title?: string
//The HTTP status code
status?: number
//A human readable description of the problem that is unique to this occurrence.
detail?: string
//A URI reference unique to the occurrence of the problem. May or may not reveal additional information about the problem.
instance?: string
}Additional properties may be added to the root level of the object to convey information unique to the error.
Note that status is optional. That's because it is expected to already be a part of the response. Adding it here is redundant (and IMHO should be discouraged).
There was a problem hiding this comment.
Although we based our AIP 193 on RFC 7807, you might be interested in seeing how ours differs from the RFC as an extension use case: https://github.com/rhamiltonsf/sf.aip/blob/master/aip/0193.md
There was a problem hiding this comment.
There's a recent draft version of this RFC here: https://datatracker.ietf.org/doc/draft-ietf-httpapi-rfc7807bis/. Most changes look like additional clarifications: https://www.ietf.org/rfcdiff?url1=rfc7807&url2=draft-ietf-httpapi-rfc7807bis-01&difftype=--hwdiff
There was a problem hiding this comment.
one of the reasons why status is in there is to allow RFC 7807 to be used for representing problem reports in a self-contained way. and btw, type is optional, too: everything is optional.
|
|
||
| ### Messages | ||
|
|
||
| Error messages **should** help a reasonably technical user understand and |
There was a problem hiding this comment.
@hudlow Who is the intended audience of these errors?
- @rhamiltonsf discussed whether we should use RFC 7807, which may satisfy both requirements.
| } | ||
| ``` | ||
|
|
||
| ### Partial errors |
There was a problem hiding this comment.
@dhudlow and @saurabhsahni, should we allow for the return of multiple errors?
- @hudlow, @rhamiltonsf Probably not disparate issues (i.e. Authentication AND invalid body), however, this might need to be explored more for bulk and batch operations.
- @saurabhsahni, @hudlow errors that are of the same type can be returned together.
There was a problem hiding this comment.
@rhamiltonsf: We got blocked on the batch/bulk API recommendations last time when we chatted about this
- @hudlow and @rhamiltonsf: In interest of moving this AIP forward, we may want to redact recommendation around batch/bulk APIs for now.
| } | ||
| ``` | ||
|
|
||
| ### Partial errors |
There was a problem hiding this comment.
@rhamiltonsf: We got blocked on the batch/bulk API recommendations last time when we chatted about this
- @hudlow and @rhamiltonsf: In interest of moving this AIP forward, we may want to redact recommendation around batch/bulk APIs for now.
| **should** be provided in a `details` field. If even more information is | ||
| necessary, the service **should** provide a link where a reader can get more | ||
| information or ask questions to help resolve the issue. | ||
|
|
There was a problem hiding this comment.
How about adding some examples like 👇 here?
Below are some examples of good errors and not so good errors:
❌ Invalid Book Name.
✅ Book name must be between 5 and 50 characters.
❌ Access is denied
✅ Only admin users have access to this resource.
❌ Bad input
✅ 'ID' must be provided in the input
|
|
||
| ### Structure | ||
|
|
||
| Error responses **should** conform to the following interface: |
There was a problem hiding this comment.
There's a recent draft version of this RFC here: https://datatracker.ietf.org/doc/draft-ietf-httpapi-rfc7807bis/. Most changes look like additional clarifications: https://www.ietf.org/rfcdiff?url1=rfc7807&url2=draft-ietf-httpapi-rfc7807bis-01&difftype=--hwdiff
| details: ?any[]; | ||
| } | ||
| ``` | ||
|
|
There was a problem hiding this comment.
How about describing code here?
- The
codefield is the HTTP status code value returned as number. It MUST match the status code in the HTTP response.
| ```typescript | ||
| interface Error { | ||
| // The HTTP status code. | ||
| code: number; |
There was a problem hiding this comment.
I would very much appreciate it if this were renamed, both to align with RFC 7807 and also to not conflict with OCI Errors (which use code to communicate the specific error type).
| code: number; | |
| status: number; |
| // The source of the error (usually the registered service address of the | ||
| // tool or product that generates the error). | ||
| // Example: "pubsub.googleapis.com" | ||
| domain: string; |
There was a problem hiding this comment.
This name also seems very strange to me, as opposed to e.g. source or scope or namespace or some suffixed variant thereof.
| // The source of the error (usually the registered service address of the | ||
| // tool or product that generates the error). | ||
| // Example: "pubsub.googleapis.com" | ||
| domain: string; |
There was a problem hiding this comment.
And shouldn't it be optional?
| type: string; | ||
|
|
||
| // An array of additional error details. | ||
| details: ?any[]; |
There was a problem hiding this comment.
Do we care about recommending a structure for details. Example:
details: [
{
message: string;
type: string;
...
}
]
|
I think we want to orient this AIP towards how error handling should leverage |
|
On 2021-12-02 14:09, Sam Woodard wrote:
I think we want to *orient* this AIP towards how error handling should
leverage |ErrorInfo|
<https://github.com/googleapis/googleapis/blob/eecbbee1d2428fcd64b1d816f4419de38c1ec57b/google/rpc/error_details.proto#L88-L136>.
I will fork and suggest edits.
this is very interesting to me. the way i talk to people about
guidelines is to have a structure for each of them.
http://dret.net/lectures/api-days-interface-2020/#anatomy
the point is that for example error handling is generally a good idea,
but obviously looks different in let's say REST and GraphQL and gRPC.
which is why it makes sense to talk about what to do in general, and
then to show how to do this for specific tech stacks.
is that general guidelines structure something that has been discussed
already? i am sorry if that's the case, i am definitely late to the
party and may be missing a lot of context.
…--
erik wilde | ***@***.*** |
| http://dret.net/netdret |
| http://twitter.com/dret |
|
|
Erik,
I reading what you are describing, I initially thought you might be talking
about something like this:
https://google.aip.dev/8
However, they now feel a little different. This might be worth revisiting.
…On Thu, Dec 2, 2021 at 6:09 PM Erik Wilde ***@***.***> wrote:
On 2021-12-02 14:09, Sam Woodard wrote:
> I think we want to *orient* this AIP towards how error handling should
> leverage |ErrorInfo|
> <
https://github.com/googleapis/googleapis/blob/eecbbee1d2428fcd64b1d816f4419de38c1ec57b/google/rpc/error_details.proto#L88-L136>.
> I will fork and suggest edits.
this is very interesting to me. the way i talk to people about
guidelines is to have a structure for each of them.
http://dret.net/lectures/api-days-interface-2020/#anatomy
the point is that for example error handling is generally a good idea,
but obviously looks different in let's say REST and GraphQL and gRPC.
which is why it makes sense to talk about what to do in general, and
then to show how to do this for specific tech stacks.
is that general guidelines structure something that has been discussed
already? i am sorry if that's the case, i am definitely late to the
party and may be missing a lot of context.
--
erik wilde | ***@***.*** |
| http://dret.net/netdret |
| http://twitter.com/dret |
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#3 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AJIXVQK5VTWBWMSZ74OARFTUPAKD5ANCNFSM4QUEMRUA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
|
On 2021-12-06 21:22, rhamiltonsf wrote:
I reading what you are describing, I initially thought you might be talking
about something like this:
https://google.aip.dev/8
However, they now feel a little different. This might be worth revisiting.
it's different. it feels like https://google.aip.dev/8 is about how to
structure your AIPs from a document perspective, and from a document
management perspective.
what i am talking about is more how to structure guidance itself. for
errors, it could be something like:
- why
all APIs should cover failure modes as well so that consumers have a
well-defined way of understanding when and why an API has a problem.
- what
a good way to make this work for consumers is to have error formats that
are coherent across APIs, so that they don't need to learn the general
failure mechanics of each individual API.
- how
depending on the tech, choose a format that works well for the tech and
can be used across APIs using that tech. for HTTP APIs, RFC 7807 might
be a good idea. for other tech (graphql, grpc, event models) you woulod
probably want to suggest other mechanisms.
in a perfect world, devs could then turn AIP into "REST mode" and for
all AIPs would only see the REST specific guidance. that's maybe a bit
far-fetched, but my main point is that it might be good to structure
guidance into why/what/how, and afaict, for each why there can be
multiple what, and for each what there can be multiple how.
…--
erik wilde | ***@***.*** |
| http://dret.net/netdret |
| http://twitter.com/dret |
|
|
Erik,
I like the structure you are talking about, and in some ways think we are
doing it (although there are areas we might-could be doing it more
explicitly).
I would recommend taking an AIP and seeing if you can make this "Why",
"What", "How" structure more explicit. Although, for now, I think it should
still do so within the format defined in AIP 8.
…On Mon, Dec 6, 2021 at 6:24 PM Erik Wilde ***@***.***> wrote:
On 2021-12-06 21:22, rhamiltonsf wrote:
> I reading what you are describing, I initially thought you might be
talking
> about something like this:
> https://google.aip.dev/8
> However, they now feel a little different. This might be worth
revisiting.
it's different. it feels like https://google.aip.dev/8 is about how to
structure your AIPs from a document perspective, and from a document
management perspective.
what i am talking about is more how to structure guidance itself. for
errors, it could be something like:
- why
all APIs should cover failure modes as well so that consumers have a
well-defined way of understanding when and why an API has a problem.
- what
a good way to make this work for consumers is to have error formats that
are coherent across APIs, so that they don't need to learn the general
failure mechanics of each individual API.
- how
depending on the tech, choose a format that works well for the tech and
can be used across APIs using that tech. for HTTP APIs, RFC 7807 might
be a good idea. for other tech (graphql, grpc, event models) you woulod
probably want to suggest other mechanisms.
in a perfect world, devs could then turn AIP into "REST mode" and for
all AIPs would only see the REST specific guidance. that's maybe a bit
far-fetched, but my main point is that it might be good to structure
guidance into why/what/how, and afaict, for each why there can be
multiple what, and for each what there can be multiple how.
--
erik wilde | ***@***.*** |
| http://dret.net/netdret |
| http://twitter.com/dret |
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#3 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AJIXVQIILETAXGUD62NNGO3UPVO6NANCNFSM4QUEMRUA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
|
hello ross.
On 2021-12-17 08:22, rhamiltonsf wrote:
I like the structure you are talking about, and in some ways think we are
doing it (although there are areas we might-could be doing it more
explicitly).
if it were part of the structure it could be processed. maybe this is
too much, but you could, for example, produce a version of the
guidelines that only showed GraphQL guidance. or produce one that only
showed REST guidance. both would who the same "why", but would show
different ways "what" can be done to address individual issues.
API landscapes are becoming more complex and diverse. from what i see,
monocultures will be replaced by more diverse landscapes, and the easier
we can both manage guidance for these, and still make them useful to
those focusing on a specific API style or tech, the better.
I would recommend taking an AIP and seeing if you can make this "Why",
"What", "How" structure more explicit. Although, for now, I think it should
still do so within the format defined in AIP 8.
sounds good. would you have an AIP in mind or can i take any? what's the
group's process ho to create and discuss such a proposal?
thanks and cheers,
dret.
…--
erik wilde | ***@***.*** |
| http://dret.net/netdret |
| http://twitter.com/dret |
|
Anything in the TODO column in this project is fair game. The process is pretty open. Make the desired changes, and then submit for review. We review changes in our weekly meeting, or if you have questions/concerns, you can raise those in that meeting as well. |
There was a problem hiding this comment.
Looks good. 👍
I think the current content is good enough as the initial version of this AIP. It communicates the most essential guidance, which is that you should have a standard error format. The details are likely to vary by company, so we shouldn't dwell to hard on that.
This adds a generic AIP for errors. There is probably a decent bit for us to discuss here.
Some high-level notes:
Retry-After). I personally think thatRetry-Aftergets covered in AIP-194, and I could not think of any others that warranted inclusion here.{% sample %}as I could not think of how a full file would add any context that the AIP should elide (which I think is my standard for using{% sample %}, although this is 100% intuition and we should discuss it).I expect this is an area where everyone will need to make changes, but I also notice that entire sections can probably be adopted by everyone (e.g. "Messages"), so I think this should work reasonably well.
Looking forward to the discussion on this.