Merge pull request #270 from twitchtv/resource_exhausted_429

ResourceExhausted error code to HTTP 429 status code
twitchtv · Sep 16, 2020 · 3865fde · 3865fde
2 parents 01dd2f0 + 4bc5632
commit 3865fde
Show file tree

Hide file tree

Showing 23 changed files with 68 additions and 32 deletions.
diff --git a/clientcompat/internal/clientcompat/clientcompat.twirp.go b/clientcompat/internal/clientcompat/clientcompat.twirp.go
diff --git a/docs/errors.md b/docs/errors.md
@@ -14,7 +14,7 @@ Twirp server implementations can return regular errors too, but those
 will be wrapped with `twirp.InternalErrorWith(err)`, so they are also
 `twirp.Error` values when received by the clients.
 
-Check the [Errors Spec](spec_v5.md) for more information on error
+Check the [Errors Spec](spec_v7.md) for more information on error
 codes and the wire protocol.
 
 Also don't be afraid to open the [source code](https://github.com/twitchtv/twirp/blob/master/errors.go)
@@ -52,7 +52,7 @@ Each error code is defined by a constant in the `twirp` package:
 | AlreadyExists      | already_exists      | 409 Conflict
 | PermissionDenied   | permission_denied   | 403 Forbidden
 | Unauthenticated    | unauthenticated     | 401 Unauthorized
-| ResourceExhausted  | resource_exhausted  | 403 Forbidden
+| ResourceExhausted  | resource_exhausted  | 429 Too Many Requests
 | FailedPrecondition | failed_precondition | 412 Precondition Failed
 | Aborted            | aborted             | 409 Conflict
 | OutOfRange         | out_of_range        | 400 Bad Request
@@ -61,7 +61,7 @@ Each error code is defined by a constant in the `twirp` package:
 | Unavailable        | unavailable         | 503 Service Unavailable
 | DataLoss           | dataloss            | 500 Internal Server Error
 
-For more information on each code, see the [Errors Spec](spec_v5.md).
+For more information on each code, see the [Errors Spec](spec_v7.md).
 
 ### HTTP Errors from Intermediary Proxies
 
@@ -80,7 +80,7 @@ depends upon the HTTP status of the invalid response:
 | 401 Unauthorized         | Unauthenticated
 | 403 Forbidden            | PermissionDenied
 | 404 Not Found            | BadRoute
-| 429 Too Many Requests    | Unavailable
+| 429 Too Many Requests    | ResourceExhausted
 | 502 Bad Gateway          | Unavailable
 | 503 Service Unavailable  | Unavailable
 | 504 Gateway Timeout      | Unavailable
@@ -123,8 +123,8 @@ part of the Protobuf messages (add an error field to proto messages).
 
 ### Writing HTTP Errors outside Twirp services
 
-Twirp services can be [muxed with other HTTP services](mux.md). For consistent responses 
-and error codes _outside_ Twirp servers, such as http middlewares, you can call `twirp.WriteError`. 
+Twirp services can be [muxed with other HTTP services](mux.md). For consistent responses
+and error codes _outside_ Twirp servers, such as http middlewares, you can call `twirp.WriteError`.
 
 The error is expected to satisfy a `twirp.Error`, otherwise it is wrapped with `twirp.InternalError`.
 
@@ -134,7 +134,7 @@ Usage:
 rpc.WriteError(w, twirp.NewError(twirp.Unauthenticated, "invalid token"))
 ```
 
-To simplify `twirp.Error` composition, a few constructors are available, such as `NotFoundError` 
+To simplify `twirp.Error` composition, a few constructors are available, such as `NotFoundError`
 and `RequiredArgumentError`. See [docs](https://godoc.org/github.com/twitchtv/twirp#Error).
 
 With constructor:

diff --git a/docs/routing.md b/docs/routing.md
@@ -27,13 +27,13 @@ For example, to make a hat with the [Haberdasher service](example.md):
 POST /twirp/twirp.example.haberdasher.Haberdasher/MakeHat
 ```
 
-More details on the [protocol specification](spec_v5.md).
+More details on the [protocol specification](spec_v7.md).
 
 #### Naming Style
 
 Please follow the [Protocol Buffers Style Guide](https://developers.google.com/protocol-buffers/docs/style#services). In particular, the `<Service>` and `<Method>` names should be CamelCased (with an initial capital).
 
-The [official Go implementation](https://github.com/twitchtv/twirp) differs in behavior from what is described in the [protocol specification](https://twitchtv.github.io/twirp/docs/spec_v5.html). The routes are modified to be CamelCase instead of using the exact from the proto file definition, so URL paths generated by Go clients may differ from paths generated by other languages. This issue is discussed in [#244](https://github.com/twitchtv/twirp/issues/244), and is easily avoided by following the Protocol Buffers Style Guide on your proto files.
+The [official Go implementation](https://github.com/twitchtv/twirp) differs in behavior from what is described in the [protocol specification](spec_v7.md). The routes are modified to be CamelCase instead of using the exact from the proto file definition, so URL paths generated by Go clients may differ from paths generated by other languages. This issue is discussed in [#244](https://github.com/twitchtv/twirp/issues/244), and is easily avoided by following the Protocol Buffers Style Guide on your proto files.
 
 ### Content-Type Header (json or protobuf)
 

diff --git a/docs/spec_v7.md b/docs/spec_v7.md
@@ -208,7 +208,7 @@ corresponding HTTP Status Code for the response.
 | already_exists      | 409 | An attempt to create an entity failed because one already exists.
 | permission_denied   | 403 | The caller does not have permission to execute the specified operation. It must not be used if the caller cannot be identified (use "unauthenticated" instead).
 | unauthenticated     | 401 | The request does not have valid authentication credentials for the operation.
-| resource_exhausted  | 403 | Some resource has been exhausted, perhaps a per-user quota, or perhaps the entire file system is out of space.
+| resource_exhausted  | 429 | Some resource has been exhausted or rate-limited, perhaps a per-user quota, or perhaps the entire file system is out of space.
 | failed_precondition | 412 | The operation was rejected because the system is not in a state required for the operation's execution. For example, doing an rmdir operation on a directory that is non-empty, or on a non-directory object, or when having conflicting read-modify-write on the same resource.
 | aborted             | 409 | The operation was aborted, typically due to a concurrency issue like sequencer check failures, transaction aborts, etc.
 | out_of_range        | 400 | The operation was attempted past the valid range. For example, seeking or reading past end of a paginated collection. Unlike "invalid_argument", this error indicates a problem that may be fixed if the system state changes (i.e. adding more items to the collection). There is a fair bit of overlap between "failed_precondition" and "out_of_range". We recommend using "out_of_range" (the more specific error) when it applies so that callers who are iterating through a space can easily look for an "out_of_range" error to detect when they are done.

diff --git a/errors.go b/errors.go
@@ -188,8 +188,8 @@ const (
 	// credentials for the operation.
 	Unauthenticated ErrorCode = "unauthenticated"
 
-	// ResourceExhausted indicates some resource has been exhausted, perhaps a
-	// per-user quota, or perhaps the entire file system is out of space.
+	// ResourceExhausted indicates some resource has been exhausted or rate-limited,
+	// perhaps a per-user quota, or perhaps the entire file system is out of space.
 	ResourceExhausted ErrorCode = "resource_exhausted"
 
 	// FailedPrecondition indicates operation was rejected because the system is
@@ -264,7 +264,7 @@ func ServerHTTPStatusFromErrorCode(code ErrorCode) int {
 	case Unauthenticated:
 		return 401 // Unauthorized
 	case ResourceExhausted:
-		return 403 // Forbidden
+		return 429 // Too Many Requests
 	case FailedPrecondition:
 		return 412 // Precondition Failed
 	case Aborted:

diff --git a/example/service.twirp.go b/example/service.twirp.go
diff --git a/internal/twirptest/client_test.go b/internal/twirptest/client_test.go
@@ -455,7 +455,7 @@ func TestClientIntermediaryErrors(t *testing.T) {
 		401: twirp.Unauthenticated,
 		403: twirp.PermissionDenied,
 		404: twirp.BadRoute,
-		429: twirp.Unavailable,
+		429: twirp.ResourceExhausted,
 		502: twirp.Unavailable,
 		503: twirp.Unavailable,
 		504: twirp.Unavailable,

diff --git a/internal/twirptest/empty_service/empty_service.twirp.go b/internal/twirptest/empty_service/empty_service.twirp.go
diff --git a/internal/twirptest/gogo_compat/service.twirp.go b/internal/twirptest/gogo_compat/service.twirp.go
diff --git a/internal/twirptest/google_protobuf_imports/service.twirp.go b/internal/twirptest/google_protobuf_imports/service.twirp.go
diff --git a/internal/twirptest/importable/importable.twirp.go b/internal/twirptest/importable/importable.twirp.go
diff --git a/internal/twirptest/importer/importer.twirp.go b/internal/twirptest/importer/importer.twirp.go
diff --git a/internal/twirptest/importer_local/importer_local.twirp.go b/internal/twirptest/importer_local/importer_local.twirp.go
diff --git a/internal/twirptest/importmapping/x/x.twirp.go b/internal/twirptest/importmapping/x/x.twirp.go
diff --git a/internal/twirptest/multiple/multiple1.twirp.go b/internal/twirptest/multiple/multiple1.twirp.go
diff --git a/internal/twirptest/no_package_name/no_package_name.twirp.go b/internal/twirptest/no_package_name/no_package_name.twirp.go
diff --git a/internal/twirptest/no_package_name_importer/no_package_name_importer.twirp.go b/internal/twirptest/no_package_name_importer/no_package_name_importer.twirp.go
diff --git a/internal/twirptest/proto/proto.twirp.go b/internal/twirptest/proto/proto.twirp.go
diff --git a/internal/twirptest/service.twirp.go b/internal/twirptest/service.twirp.go
diff --git a/internal/twirptest/service_method_same_name/service_method_same_name.twirp.go b/internal/twirptest/service_method_same_name/service_method_same_name.twirp.go
diff --git a/internal/twirptest/snake_case_names/snake_case_names.twirp.go b/internal/twirptest/snake_case_names/snake_case_names.twirp.go
diff --git a/internal/twirptest/source_relative/source_relative.twirp.go b/internal/twirptest/source_relative/source_relative.twirp.go
diff --git a/protoc-gen-twirp/generator.go b/protoc-gen-twirp/generator.go
@@ -621,7 +621,9 @@ func (t *twirp) generateUtils() {
 	t.P(`      code = `, t.pkgs["twirp"], `.PermissionDenied`)
 	t.P(`    case 404: // Not Found`)
 	t.P(`      code = `, t.pkgs["twirp"], `.BadRoute`)
-	t.P(`    case 429, 502, 503, 504: // Too Many Requests, Bad Gateway, Service Unavailable, Gateway Timeout`)
+	t.P(`    case 429: // Too Many Requests`)
+	t.P(`      code = `, t.pkgs["twirp"], `.ResourceExhausted`)
+	t.P(`    case 502, 503, 504: // Bad Gateway, Service Unavailable, Gateway Timeout`)
 	t.P(`      code = `, t.pkgs["twirp"], `.Unavailable`)
 	t.P(`    default: // All other codes`)
 	t.P(`      code = `, t.pkgs["twirp"], `.Unknown`)