Skip to content

Commit

Permalink
consolidate error guidance to AIP-193 (#977)
Browse files Browse the repository at this point in the history
* consolidate error guidance to AIP-193

AIP-193 is the primary AIP around errors. Some
AIPs (primarily those around resource-oriented operations)
duplicate some of that guidance.

Consolidating that guidance to help remove redundant text,
making it more maintanable.

Also fixing minor formatting incongruences:
  - links not grouped at the bottom.
  - some inconsistent formatting with the changelogs.

* addressing feedback

- adding explicit call-out for permission denied and not found
  errors in CRUD AIPs.

* addressing feedback

- switching back to original wording, but generalizing for
  both parent and resource.
  • Loading branch information
toumorokoshi authored Jan 3, 2023
1 parent 4348abe commit 88d2d62
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 40 deletions.
21 changes: 10 additions & 11 deletions aip/general/0131.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,24 +83,23 @@ variable in the `google.api.http` annotation on the RPC. This causes the `name`
field in the request to be populated based on the value in the URL when the
REST/JSON interface is used.

[aip-121]: ./0121.md
[aip-123]: ./0123.md
[aip-157]: ./0157.md
[aip-203]: ./0203.md

### Errors

If the user does not have permission to access the resource, regardless of
whether or not it exists, the service **must** error with `PERMISSION_DENIED`
(HTTP 403). Permission **must** be checked prior to checking if the resource
exists.
See [errors][], in particular [when to use PERMISSION_DENIED and
NOT_FOUND errors][permission-denied].

If the user does have proper permission, but the requested resource does not
exist, the service **must** error with `NOT_FOUND` (HTTP 404).
[aip-121]: ./0121.md
[aip-123]: ./0123.md
[aip-157]: ./0157.md
[aip-203]: ./0203.md
[errors]: ./0193.md
[permission-denied]: ./0193.md#permission-denied

## Changelog

- **2022-06-02:** Changed suffix descriptions to eliminate superfluous "-".
- **2022-11-04**: Aggregated error guidance to AIP-193.
- **2022-06-02**: Changed suffix descriptions to eliminate superfluous "-".
- **2020-06-08**: Added guidance on returning the full resource.
- **2019-10-18**: Added guidance on annotations.
- **2019-08-12**: Added guidance for error cases.
Expand Down
28 changes: 13 additions & 15 deletions aip/general/0132.md
Original file line number Diff line number Diff line change
Expand Up @@ -181,31 +181,29 @@ requests. APIs with soft deletion of a resource **should** include a
`bool show_deleted` field in the list request that, if set, will cause
soft-deleted resources to be included.

[aip-121]: ./0121.md
[aip-123]: ./0123.md
[aip-157]: ./0157.md
[aip-158]: ./0158.md
[aip-203]: ./0203.md
[soft delete]: ./0135.md#soft-delete

### Errors

If the user does not have permission to access the parent resource, regardless
of whether or not it exists, the service **must** error with
`PERMISSION_DENIED` (HTTP 403). Permission **must** be checked prior to
checking if the parent resource exists.

If the user does have proper permission, but the requested parent resource does
not exist, the service **must** error with `NOT_FOUND` (HTTP 404).
See [errors][], in particular [when to use PERMISSION_DENIED and
NOT_FOUND errors][permission-denied].

## Further reading

- For details on pagination, see [AIP-158](./0158.md).
- For listing across multiple parent collections, see [AIP-159](./0159.md).

[aip-121]: ./0121.md
[aip-123]: ./0123.md
[aip-157]: ./0157.md
[aip-158]: ./0158.md
[aip-203]: ./0203.md
[errors]: ./0193.md
[permission-denied]: ./0193.md#permission-denied
[soft delete]: ./0135.md#soft-delete

## Changelog

- **2022-06-02:** Changed suffix descriptions to eliminate superfluous "-".
- **2022-11-04**: Aggregated error guidance to AIP-193.
- **2022-06-02**: Changed suffix descriptions to eliminate superfluous "-".
- **2020-09-02**: Add link to the filtering AIP.
- **2020-08-14**: Added error guidance for permission denied cases.
- **2020-06-08**: Added guidance on returning the full resource.
Expand Down
14 changes: 13 additions & 1 deletion aip/general/0133.md
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,11 @@ is provided as a query parameters on the request URI.
**Important:** Declarative-friendly resources (AIP-128) **must** support
user-specified IDs.

### Errors

See [errors][], in particular [when to use PERMISSION_DENIED and
NOT_FOUND errors][permission-denied].

## Further reading

- For ensuring idempotency in `Create` methods, see [AIP-155][].
Expand All @@ -195,10 +200,17 @@ user-specified IDs.
[aip-155]: ./0155.md
[aip-203]: ./0203.md
[aip-210]: ./0210.md
[errors]: ./0193.md
[permission-denied]: ./0193.md#permission-denied

## Changelog

- **2022-11-04**: Aggregated error guidance to AIP-193.

## Changelog

- **2022-06-02:** Changed suffix descriptions to eliminate superfluous "-".
- **2022-11-04**: Referencing aggregated error guidance in AIP-193, similar to other CRUDL AIPs.
- **2022-06-02**: Changed suffix descriptions to eliminate superfluous "-".
- **2020-10-06**: Added declarative-friendly guidance.
- **2020-08-14**: Updated error guidance to use permission denied over
forbidden.
Expand Down
27 changes: 14 additions & 13 deletions aip/general/0134.md
Original file line number Diff line number Diff line change
Expand Up @@ -266,28 +266,29 @@ In this situation, an API **may** return back only the fields that were
updated, and omit the rest, and **should** document this behavior if they do
so.

### Errors

See [errors][], in particular [when to use PERMISSION_DENIED and
NOT_FOUND errors][permission-denied].

In addition, if the user does have proper permission, but the requested resource
does not exist, the service **must** error with `NOT_FOUND` (HTTP 404) unless
`allow_missing` is set to `true`.

[aip-121]: ./0121.md
[aip-128]: ./0128.md
[aip-203]: ./0203.md
[create]: ./0133.md
[errors]: ./0193.md
[permission-denied]: ./0193.md#permission-denied
[state fields]: ./0216.md
[aip-128]: ./0128.md
[required]: ./0203.md#required
[optional]: ./0203.md#optional

### Errors

If the user does not have permission to access the resource, regardless of
whether or not it exists, the service **must** error with `PERMISSION_DENIED`
(HTTP 403). Permission **must** be checked prior to checking if the resource
exists.

If the user does have proper permission, but the requested resource does not
exist, the service **must** error with `NOT_FOUND` (HTTP 404) unless
`allow_missing` is set to `true`.

## Changelog

- **2022-06-02:** Changed suffix descriptions to eliminate superfluous "-".
- **2022-11-04**: Aggregated error guidance to AIP-193.
- **2022-06-02**: Changed suffix descriptions to eliminate superfluous "-".
- **2021-11-04**: Changed the permission check if `allow_missing` is set.
- **2021-07-08**: Added error guidance for resource not found case.
- **2021-03-05**: Changed the etag error from `FAILED_PRECONDITION` (which
Expand Down
12 changes: 12 additions & 0 deletions aip/general/0193.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,25 @@ and the method **should** put partial failure information in the metadata
message. The errors themselves **must** still be represented with a
[`google.rpc.Status`][Status] object.

### Permission Denied

If the user does not have permission to access the resource or parent,
regardless of whether or not it exists, the service **must** error with
`PERMISSION_DENIED` (HTTP 403). Permission **must** be checked prior to checking
if the resource or parent exists.

If the user does have proper permission, but the requested resource or parent
does not exist, the service **must** error with `NOT_FOUND` (HTTP 404).

## Further reading

- For which error codes to retry, see [AIP-194](https://aip.dev/194).
- For how to retry errors in client libraries, see [AIP-4221](https://aip.dev/client-libraries/4221).

## Changelog

- **2022-11-04**: Added guidance around PERMISSION_DENIED errors previously
found in other AIPs.
- **2022-08-12**: Reworded/Simplified intro to add clarity to the intent.
- **2020-01-22**: Added a reference to the [`ErrorInfo`][ErrorInfo] message.
- **2019-10-14**: Added guidance restricting error message mutability to if
Expand Down

0 comments on commit 88d2d62

Please sign in to comment.