Alternate AIP-193 recommendation #45

saurabhsahni · 2021-12-06T23:33:16Z

Overview

This PR presents an alternate AIP-193 Error response structure recommendation as below

Error structure

interface Error {
  // A machine-readable code indicating the type of error (like `name_too_long`). This value is parseable for programmatic error handling.
  type: string;

  // A human readable description of the problem. Should not change from occurrence to occurrence.
  message?: string

  // The HTTP status code between 100 and 500
  status?: integer

  // A unique identifier that identifies the specific occurrence of the problem. Can be provided to the API owner for debugging purposes.
  incidentId?: string

  // A map of metadata returning additional error details that can be used programmatically 
  metadata?: dict<string, any>
}

Background

Here's the difference between RFC 7807 and what we've in original AIP-193:

	AIP - 193	RFC 7807 (upcoming draft)
Response properties
The HTTP status code of the problem	code - number (between 200 and 599)	status - ?integer (between 100 and 599)
The type of the error.	type - string This is a constant value that identified the cause of the error, unique within a given domain, that developers write code against.	type - ?string, format: uri-reference A URI reference RFC3986 that identifies the problem type (doesn’t need to resolve)
Human readable error summary	message - A developer-facing error message, in English	title - ?string It should not change from occurrence to occurrence of the problem, except for purposes of localization.
The source of the error	domain: string; like pubsub.google.com usually the registered service address of the tool or product that generates the error
A URI reference that identifies the specific occurrence of the problem.		instance - ?string, format: uri-reference
	details: ?any[] An array of additional error details.	detail - ?string A human-readable explanation specific to this occurrence of the problem
Response Header Content-Type	-	application/problem+json

Additional properties allowed?	No	Yes

Tweaks from RFC 7807 in detail

message instead of title and detail

We do not want to recommend a detail key (A human-readable explanation specific to this occurrence of the problem) in the top-level error structure. If developers only include occurrence-specific information like Your current balance is 30, but that costs 50 as a string message, developers may turn to parse these error strings to dive into the problem.
We prefer to call title field message because it’s a more standard name to describe the error like Exceptions have messages.

type: string instead of type?: string, format: uri-reference

There's a need to return a machine-readable error code beyond HTTP status. While technically the type parameter returns a unique URI reference for a given problem, comparing URI references programmatically doesn't seem an ideal experience.
Now any string would qualify as a uri-reference, it’s confusing to recommend something as a uri-reference, if we think most API providers should just return error codes.
This one field could be set as required.

incidentId?: string instead of instance?: string, format: uri-reference

More on “instance”:
- When the "instance" URI is dereferenceable, the problem details object can be fetched from it. It might also return information about the problem occurrence in other formats through use of proactive content negotiation (see [HTTP], Section 12.5.1).
- When the "instance" URI is not dereferencable, it serves as a unique identifier for the problem occurrence that may be of significance to the server, but is opaque to the client.
Practical use cases of returning additional problem details for a specific occurrence through a URL seem limited.
To represent a unique identifier for an error, while being compliant with RFC 7807, an alternative name could be “occurrenceId”

RFC 7807 recommends Content-Type to be set as application/problem+json. However, it's unclear to me how much value a unique content type would add for the error scenario? Should clients rather refer to HTTP status code in the header to figure out if there was an error or not? Do we need to provide recommendation for Content-Type as part of this AIP?
In lieu of allowing additional properties, we would like to add a new extensible property called metadata that can be used programmatically. This will take following structure: metadata?: dict<string, any>

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.

saurabhsahni · 2021-12-06T23:46:47Z

aip/general/0193/aip.md

+```typescript
+interface Error {
+  //A machine-readable code indicating the type of error. This value is parseable for programmatic error handling.
+  code?: string;


code?: string instead of type?: string, format: uri-reference

There's a need to return a machine-readable error code beyond HTTP status. While technically the type parameter returns a unique URI reference for a given problem, comparing URI references programmatically doesn't seem an ideal experience.

Now any string would qualify as a uri-reference, it’s confusing to recommend something as a uri-reference, if we think most API providers should just return error codes.

To ensure we’re compliant with RFC 7807, an alternative name that’s used frequently by several APIs could be “code” instead of “type”.

Example usage: Apple, Stripe, Github, Microsoft, Facebook

An alternate option here could use type?: string. Though, I'm not sure if that breaks compliance with RFC 7807

saurabhsahni · 2021-12-06T23:48:17Z

aip/general/0193/aip.md

+  detail?: string
+
+  //A unique identifier that identifies the specific occurrence of the problem. Can be provided to the API owner for debugging purposes.
+  id?: string


id?: string instead of instance?: string, format: uri-reference

More on “instance”:

When the "instance" URI is dereferenceable, the problem details object can be fetched from it. It might also return information about the problem occurrence in other formats through use of proactive content negotiation (see [HTTP], Section 12.5.1).

When the "instance" URI is not dereferencable, it serves as a unique identifier for the problem occurrence that may be of significance to the server, but is opaque to the client.

Practical use cases of returning additional problem details for a specific occurrence through a URL seem limited.

To represent a unique identifier for an error, while being compliant with RFC 7807, an alternative name could be “id” (Example usage: Apple, Facebook)

Alternate options:

errorInstanceId

traceId

requestId

occurrenceId

Consensus was to use occurrenceId. id, errorId could be confused as an id unique for this error type instead of the specific occurrence. `

saurabhsahni · 2021-12-07T01:57:29Z

aip/general/0193/aip.md

+
+Services **must** clearly distinguish successful responses from error responses
+by using appropriate HTTP codes:
+- Informational responses **must** use HTTP status codes between 100 and 199.


Allowing 100 to 199 status code is per RFC 7807

I'm not entirely sure what an "informational response" is - and I'd rather not have to go to the HTTP RFCs to find out. Does this range indicate success or failure?

These may not indicate success or failure. These are often used to indicate intermediate status of an HTTP request. For instance, a server that supports HTTP/2 can respond with 101 Switching Protocols upgrading a HTTP/1.1 connection to a HTTP/2 connection: https://datatracker.ietf.org/doc/html/rfc7540#section-3.2

Adding clarification

saurabhsahni · 2021-12-07T01:58:51Z

aip/general/0193/aip.md

+```typescript
+interface Error {
+  //A machine-readable code indicating the type of error (like `name_too_long`). This value is parseable for programmatic error handling.
+  code: string;


code: string instead of type?: string, format: uri-reference

There's a need to return a machine-readable error code beyond HTTP status. While technically the type parameter returns a unique URI reference for a given problem, comparing URI references programmatically doesn't seem an ideal experience.

Now any string would qualify as a uri-reference, it’s confusing to recommend something as a uri-reference, if we think most API providers should just return error codes.

To ensure we’re compliant with RFC 7807, an alternative name that’s used frequently by several APIs could be “code” instead of “type”.

This one field could be set as required.

Example usage: Apple, Stripe, Github, Microsoft, Facebook

We should probably specify that strings should be comparable using ordinal comparisons - not case-insensitively, for example.

Adding clarification below. One thing to note is the recommendation is to use only lowercase letters here.

Per discussion, consensus was to to stick with type?: string without format: uri-reference because comparing URI references programmatically isn't an ideal experience. @dret we were wondering if there has been a discussion around dropping format: uri-reference from RFC 7807?

saurabhsahni · 2021-12-07T02:02:09Z

aip/general/0193/aip.md

+necessary, the service **should** provide a link where a reader can get more
+information or ask questions to help resolve the issue.
+
+Below are some examples of good errors and not so good errors:


Thoughts on including some example good and not so good errors here?

jskeet

The existing Google AIP-193 talks about ErrorInfo, which includes a string-to-string map for additional information, like the extensions in RFC 7807. I think we should have some way of passing additional error-specific-but-machine-readable information.

jskeet · 2021-12-07T15:10:49Z

aip/general/0193/aip.md

+
+Services **must** clearly distinguish successful responses from error responses
+by using appropriate HTTP codes:
+- Informational responses **must** use HTTP status codes between 100 and 199.


I'm not entirely sure what an "informational response" is - and I'd rather not have to go to the HTTP RFCs to find out. Does this range indicate success or failure?

jskeet · 2021-12-07T15:12:10Z

aip/general/0193/aip.md

+Services **must** clearly distinguish successful responses from error responses
+by using appropriate HTTP codes:
+- Informational responses **must** use HTTP status codes between 100 and 199.
+- Successful responses **must** use HTTP status codes between 200 and 399.


Are 3xx responses really successful in the context of APIs? 304 could be considered successful, but anything else really suggests further action.

Good point. 3XX should mean redirection. Updating this

jskeet · 2021-12-07T15:13:31Z

aip/general/0193/aip.md

+```typescript
+interface Error {
+  //A machine-readable code indicating the type of error (like `name_too_long`). This value is parseable for programmatic error handling.
+  code: string;


We should probably specify that strings should be comparable using ordinal comparisons - not case-insensitively, for example.

jskeet · 2021-12-07T15:14:25Z

aip/general/0193/aip.md

+
+```typescript
+interface Error {
+  //A machine-readable code indicating the type of error (like `name_too_long`). This value is parseable for programmatic error handling.


Super-nit: // without a space after it makes me wince :)

dret · 2021-12-07T17:01:27Z

On 2021-12-07 16:20, Jon Skeet wrote: ***@***.**** commented on this pull request. The existing Google AIP-193 talks about ErrorInfo, which includes a string-to-string map for additional information, like the extensions in RFC 7807. I think we should have /some/ way of passing additional error-specific-but-machine-readable information.

that's a good idea. it folds nicely into my proposal to be a more structured with the AIPs by always having why/what/how sections. the "there should be /some/ way of passing additional error-specific-but-machine-readable information" is perfect for the "what" part, and ErrorInfo and RFC 7807 (maybe plus patterns for how to use the type identifier) are two very useful "how" parts.

…

shwoodard · 2021-12-07T18:32:32Z

Another place to look that might add some insight here, https://jsonapi.org/format/#errors

saurabhsahni · 2021-12-14T18:33:14Z

aip/general/0193/aip.md

+  // A unique identifier that identifies the specific occurrence of the problem. Can be provided to the API owner for debugging purposes.
+  id?: string
+
+  // An array of additional error details.                                                                                                                           errors?: any[]


a map is recommended

any json value would be recommended

metadata?: object

saurabhsahni · 2022-01-11T06:28:55Z

aip/general/0193/aip.md

+
+Services **must** clearly distinguish successful responses from error responses
+by using appropriate HTTP codes:
+- Informational responses **must** use HTTP status codes between 100 and 199.


These may not indicate success or failure. These are often used to indicate intermediate status of an HTTP request. For instance, a server that supports HTTP/2 can respond with 101 Switching Protocols upgrading a HTTP/1.1 connection to a HTTP/2 connection: https://datatracker.ietf.org/doc/html/rfc7540#section-3.2

saurabhsahni · 2022-01-25T07:30:39Z

aip/general/0193/aip.yaml

@@ -0,0 +1,7 @@
+---


Do we need to change anything in this file?

it looks like google.aip.dev moves this front-matter to the file itself (which I tend to agree with)

https://raw.githubusercontent.com/aip-dev/google.aip.dev/master/aip/general/0001.md.

Is there an outstanding issue to move this to the more succinct format?

gibson042 · 2022-01-25T16:19:19Z

aip/general/0193/aip.md

+  only lower-case letters, numbers, and the `-` character. These strings should 
+  be comparable using ordinal comparisons.


What does this mean? What kind of "ordinal comparisons", and for what purpose?

Added "ordinal comparison" here per @jskeet's feedback: #45 (comment)

What that would mean here is each a developer should send stable error codes that can always be compared to the exact same numeric character values. For example, an API shouldn't send invalid_auth sometimes and Invalid_Auth other times.

That said, given we're recommending developers to always send lower-case letters here. I'm curious if we need that anymore? @jskeet thoughts?

saurabhsahni · 2022-01-28T02:24:35Z

aip/general/0193/aip.md

+```typescript
+interface Error {
+  // A machine-readable code indicating the type of error (like `name_too_long`). This value is parseable for programmatic error handling.
+  type: string;


Re-posting this comment here because the last one is now buried under the PR updates.

Per discussion, the consensus was to stick with type?: string without format: uri-reference because comparing URI references programmatically isn't an ideal experience. Also, non-resolvable URI references are not valuable. @dret we were wondering what do you think about dropping format: uri-reference from RFC 7807?

aip/general/0193/aip.md

Co-authored-by: Richard Gibson <richard.gibson@gmail.com>

saurabhsahni · 2022-02-01T18:32:21Z

aip/general/0193/aip.md

+  occurenceId?: string
+
+  // A map of metadata returning additional error details that can be used programmatically 
+  metadata?: dict<string, any>


The schema of metadata should be documented and a change in this schema could mean a breaking change.

@shwoodard: The schema of metadata should be fixed per type and a change in this schema could mean a breaking change.

saurabhsahni · 2022-02-05T01:39:58Z

aip/general/0193/aip.md

+  detail?: string
+
+  // A unique identifier that identifies the specific occurrence of the problem. Can be provided to the API owner for debugging purposes.
+  occurenceId?: string


One of the feedback in hangout chat was occurenceID isn't elegant. Do folks have other suggestions here? We previously rejected names like id because we thought they may seem to have 1 to 1 mapping with type.

We considered traceId but were unsure if that may have a different meaning than what trace_id may mean with open telemetry.

@shwoodard incidentID as alternative
@lukesneeringer incidentId sounds 👍

saurabhsahni · 2022-02-08T18:08:31Z

aip/general/0193/aip.md

+  occurenceId?: string
+
+  // A map of metadata returning additional error details that can be used programmatically 
+  metadata?: dict<string, any>


@shwoodard: The schema of metadata should be fixed per type and a change in this schema could mean a breaking change.

saurabhsahni · 2022-02-08T18:10:48Z

aip/general/0193/aip.md

+  status?: integer
+
+  // A human-readable explanation specific to this occurrence of the problem
+  detail?: string


@shwoodard @lukesneeringer message is more standard name for this field. Though RFC 7807 has this as detail.
@lukesneeringer @shwoodard we should drop detail from the recommendation because adding explanation specific to an occurrence of the problem may lead to developers parsing this string.

saurabhsahni · 2022-02-08T18:16:18Z

aip/general/0193/aip.md

+  detail?: string
+
+  // A unique identifier that identifies the specific occurrence of the problem. Can be provided to the API owner for debugging purposes.
+  occurenceId?: string


@shwoodard incidentID as alternative
@lukesneeringer incidentId sounds 👍

shwoodard · 2022-03-15T17:12:09Z

@saurabhsahni would it be possible to update the PR comment that has Overview and, most importantly, Error Structure to match the current state of the files in the PR?

saurabhsahni · 2022-03-15T22:07:21Z

@shwoodard I updated PR to reflect the new structure.

saurabhsahni · 2022-09-07T17:12:59Z

aip/general/0193/aip.md

+
+```typescript
+interface Error {
+  // A machine-readable code indicating the type of error (like `name_too_long`). This value is parseable for programmatic error handling.


Clarify that this "Should not change from occurrence to occurrence"

saurabhsahni · 2022-09-07T17:36:28Z

aip/general/0193/aip.md

+  // A machine-readable code indicating the type of error (like `name_too_long`). This value is parseable for programmatic error handling.
+  type: string;
+
+  // A human readable description of the problem. Should not change from occurrence to occurrence.


A human-readable description of the problem. Messages are likely to be logged in plain text and should not include information about a specific occurrence. Information about specific occurrences should be part of metadata.

toumorokoshi

Hello! I was requested by @makahmad to leave a review.

I think there's already a lot of outstanding issues as-is, so I'm wondering how those will be resolved.

Honestly with a year since the last activity, it's probably worth re-discussing this in the AIP meetings to get some re-review here of the high level approach.

toumorokoshi · 2023-03-31T00:40:35Z

aip/general/0193/aip.md

+
+Error responses **should** conform to the following interface:
+
+```typescript


why typescript when the existing aip.dev is using protobuf / OpenAPI? https://aip-dev.github.io/aip.dev/136.

toumorokoshi · 2023-03-31T00:44:07Z

aip/general/0193/aip.md

+errors, and to reduce boilerplate by having common error-handling logic, rather
+than being expected to constantly add verbose error handling everywhere.
+
+## Guidance


since there already is protobuf-level guidance, why not include protobuf? or does it translate? https://google.aip.dev/193.

I'm thinking about how to reconcile this proposal with existing practices in google.aip.dev (which, as the only public repository up until now, has served as the base of other forks IIUC).

Luke Sneeringer and others added 4 commits September 2, 2020 19:15

Merge branch 'master' into aip-193

0908529

Merge branch 'main' into aip-193

568e0fc

Update aip.md

1519d97

saurabhsahni requested a review from a team as a code owner December 6, 2021 23:33

saurabhsahni commented Dec 6, 2021

View reviewed changes

saurabhsahni added 2 commits December 6, 2021 16:05

Update aip.md

40cff57

Update aip.md

1aa3dee

saurabhsahni commented Dec 7, 2021

View reviewed changes

adding example good errors

9172c51

saurabhsahni commented Dec 7, 2021

View reviewed changes

jskeet reviewed Dec 7, 2021

View reviewed changes

shwoodard mentioned this pull request Dec 14, 2021

AIP 193: Errors #91

Open

Adding an errors array

6233a69

saurabhsahni commented Jan 11, 2022

View reviewed changes

saurabhsahni added 2 commits January 10, 2022 23:02

Update aip.md

421a3fd

Updating errors AIP per discussion

2626d27

saurabhsahni commented Jan 25, 2022

View reviewed changes

gibson042 reviewed Jan 25, 2022

View reviewed changes

saurabhsahni requested review from lukesneeringer and mkistler January 25, 2022 18:27

saurabhsahni commented Jan 28, 2022

View reviewed changes

gibson042 reviewed Jan 28, 2022

View reviewed changes

aip/general/0193/aip.md Outdated Show resolved Hide resolved

Clarifying ordinal comparison

1c083f9

Co-authored-by: Richard Gibson <richard.gibson@gmail.com>

saurabhsahni added 2 commits January 30, 2022 16:02

Merge branch 'main' into saurabh-aip-193

77ee548

clarifying metadata changes can be breaking

e66cf0d

saurabhsahni commented Feb 5, 2022

View reviewed changes

saurabhsahni commented Feb 9, 2022

View reviewed changes

saurabhsahni added 4 commits February 8, 2022 18:03

Minor tweaks per feedback

c499945

Update aip.md

214fac2

adding examples

dcc55d8

Update aip.md

3cf2ce2

Using message instead of title

97eb647

saurabhsahni changed the title ~~Alternate AIP-193 recommendation per RFC 7807~~ Alternate AIP-193 recommendation Mar 15, 2022

saurabhsahni requested a review from shwoodard March 15, 2022 17:27

saurabhsahni commented Sep 7, 2022

View reviewed changes

saurabhsahni added 2 commits September 7, 2022 10:37

Update aip.md

1de2d49

Update aip.md

10c62b6

toumorokoshi reviewed Mar 31, 2023

View reviewed changes

		only lower-case letters, numbers, and the `-` character. These strings should
		be comparable using ordinal comparisons.


		Error responses should conform to the following interface:

		```typescript

Alternate AIP-193 recommendation #45

Are you sure you want to change the base?

Alternate AIP-193 recommendation #45

Conversation

saurabhsahni commented Dec 6, 2021 • edited Loading

Overview

Error structure

Background

Tweaks from RFC 7807 in detail

saurabhsahni Dec 6, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

saurabhsahni Jan 11, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jskeet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dret commented Dec 7, 2021 via email

shwoodard commented Dec 7, 2021

Choose a reason for hiding this comment

saurabhsahni Jan 11, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shwoodard commented Mar 15, 2022

saurabhsahni commented Mar 15, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

toumorokoshi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

saurabhsahni commented Dec 6, 2021 •

edited

Loading

saurabhsahni Dec 6, 2021 •

edited

Loading

saurabhsahni Jan 11, 2022 •

edited

Loading

saurabhsahni Jan 11, 2022 •

edited

Loading