Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
* Adds richer error messages for the state api Signed-off-by: Elena Kolevska <elena@kolevska.com> * Adds integration tests Signed-off-by: Elena Kolevska <elena@kolevska.com> * Adds richer error for invalid key names Signed-off-by: Elena Kolevska <elena@kolevska.com> * Corrects imports formatting Signed-off-by: Elena Kolevska <elena@kolevska.com> * Updates to latest kit changes Signed-off-by: Elena Kolevska <elena@kolevska.com> * Adds integration tests for richer errors in http Signed-off-by: Elena Kolevska <elena@kolevska.com> * Removes commented tests Signed-off-by: Elena Kolevska <elena@kolevska.com> * Point to kit fork, temporarily Signed-off-by: Elena Kolevska <elena@kolevska.com> * More integration tests Signed-off-by: Elena Kolevska <elena@kolevska.com> * Adds more integration tests Signed-off-by: Elena Kolevska <elena@kolevska.com> * Reenables test Signed-off-by: Elena Kolevska <elena@kolevska.com> * Adds MultiMaxSize Signed-off-by: Elena Kolevska <elena@kolevska.com> * Adds tests for DAPR_STATE_TOO_MANY_TRANSACTIONS Signed-off-by: Elena Kolevska <elena@kolevska.com> * Adds integration tests for states that don’t support transactions Signed-off-by: Elena Kolevska <elena@kolevska.com> * Cleanup Signed-off-by: Elena Kolevska <elena@kolevska.com> * Removes leftover from merge Signed-off-by: Elena Kolevska <elena@kolevska.com> * Move to error builders Signed-off-by: Elena Kolevska <elena@kolevska.com> * Apply suggestions from code review Co-authored-by: Josh van Leeuwen <me@joshvanl.dev> Signed-off-by: Elena Kolevska <elena-kolevska@users.noreply.github.com> * Changes after review Signed-off-by: Elena Kolevska <elena@kolevska.com> * Caches MultiMaxSize() Signed-off-by: Elena Kolevska <elena@kolevska.com> * Update to latest kit Signed-off-by: Elena Kolevska <elena@kolevska.com> * Adds a readme file - guiide to contributing new errors. Applies comments from reviews. Signed-off-by: Elena Kolevska <elena@kolevska.com> * Fixes linter warnings Signed-off-by: Elena Kolevska <elena@kolevska.com> * Points back to dapr/kit instead of Cassie’s fork Signed-off-by: Elena Kolevska <elena@kolevska.com> * Updates to latest dapr kit main Signed-off-by: Elena Kolevska <elena@kolevska.com> * Adds whitespace Signed-off-by: Elena Kolevska <elena@kolevska.com> * Reverts update Signed-off-by: Elena Kolevska <elena@kolevska.com> * Reverts version update, as per review comment Signed-off-by: Elena Kolevska <elena@kolevska.com> * Runs modtidy-all Signed-off-by: Elena Kolevska <elena@kolevska.com> * Addresses Josh’s comment Signed-off-by: Elena Kolevska <elena@kolevska.com> * Linter Signed-off-by: Elena Kolevska <elena@kolevska.com> * Uses .FromError instead of .As Signed-off-by: Elena Kolevska <elena@kolevska.com> * Apply suggestions from code review Co-authored-by: Cassie Coyle <cassie.i.coyle@gmail.com> Signed-off-by: Elena Kolevska <elena-kolevska@users.noreply.github.com> * Update pluggable.go Co-authored-by: Josh van Leeuwen <me@joshvanl.dev> Signed-off-by: Elena Kolevska <elena-kolevska@users.noreply.github.com> * Move const to string literal Signed-off-by: Elena Kolevska <elena@kolevska.com> * Apply suggestions from code review Co-authored-by: Alessandro (Ale) Segala <43508+ItalyPaleAle@users.noreply.github.com> Signed-off-by: Elena Kolevska <elena-kolevska@users.noreply.github.com> --------- Signed-off-by: Elena Kolevska <elena@kolevska.com> Signed-off-by: Elena Kolevska <elena-kolevska@users.noreply.github.com> Co-authored-by: Artur Souza <asouza.pro@gmail.com> Co-authored-by: Josh van Leeuwen <me@joshvanl.dev> Co-authored-by: Cassie Coyle <cassie.i.coyle@gmail.com> Co-authored-by: Alessandro (Ale) Segala <43508+ItalyPaleAle@users.noreply.github.com>
- Loading branch information
5 people
committed
Jan 9, 2024
1 parent
c9715e4
commit 3ddf46a
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| ### Error codes in Dapr | ||
|
|
||
| #### Introduction | ||
|
|
||
| This guide is intended for developers working on the Dapr code base, specifically for those updating the error handling to align with the gRPC richer error model. The goal is to standardize error responses across Dapr's services. | ||
|
|
||
| #### Prerequisites | ||
|
|
||
| - Familiarity with gRPC and its error model. Refer to the [gRPC Richer Error Model Guide](https://cloud.google.com/apis/design/errors). | ||
|
|
||
| #### Step 1: Understanding the Current Error Handling | ||
| Currently, error handling in Dapr is a mix of predefined errors and dynamically constructed errors within the code. | ||
|
|
||
| - **Predefined Errors**: There are some legacy predefined errors located at [`/pkg/messages/predefined.go`](https://github.com/dapr/dapr/blob/master/pkg/messages/predefined.go), but the new errors are located in [`/pkg/api/errors`](https://github.com/dapr/dapr/blob/master/pkg/api/errors). These errors are standard and reused across various parts of the Dapr codebase. They provide a consistent error handling mechanism for common scenarios. | ||
| - **Dynamically Constructed Errors**: These errors are created on the fly within the code, typically to handle specific situations or errors that are not covered by the predefined set. | ||
|
|
||
| As we move predefined errors to the rich error model and a new location (`pkg/api/errors`), the first step in updating to the new error model is to familiarize yourself with the existing error handling patterns, both predefined and dynamically constructed. This understanding will be crucial when replacing them with the richer error model aligned with gRPC standards. | ||
|
|
||
| #### Step 2: Adding New Errors | ||
|
|
||
| 1. **Check for existing errors**: Before creating a new error, check if a relevant error file exists in `/pkg/api/errors/`. | ||
| 2. **Create a new error file**: If no relevant file exists, create a new file following the pattern `<building-block>.go`. | ||
|
|
||
| #### Step 3: Designing Rich Error Messages | ||
|
|
||
| 1. **Understand Rich Error Construction**: Familiarize yourself with the construction of rich error messages. The definition can be found in [`github.com/dapr/kit/errors`](https://github.com/dapr/kit/blob/main/errors/errors.go). | ||
| 2. **Helper Methods**: Utilize the error builder and the helper methods like `WithErrorInfo`, `WithResourceInfo` to enrich error information. | ||
| 3. **Reference implementation**: You can check out `pkg/api/errors/state.go` for a reference implementation. The following code snippet shows how to create a new error using the helper methods: | ||
|
|
||
| ```go | ||
| func StateStoreInvalidKeyName(storeName string, key string, msg string) error { | ||
| return kiterrors.NewBuilder( | ||
| codes.InvalidArgument, | ||
| http.StatusBadRequest, | ||
| msg, | ||
| "ERR_MALFORMED_REQUEST", | ||
| ). | ||
| WithErrorInfo(kiterrors.CodePrefixStateStore+kiterrors.CodeIllegalKey, nil). | ||
| WithResourceInfo("state", storeName, "", ""). | ||
| WithFieldViolation(key, msg). | ||
| Build() | ||
| } | ||
| ``` | ||
| 4. **Mandatory and Optional Information**: | ||
| - **ErrorInfo**: This is a required field. | ||
| - **ResourceInfo and other Details fields**: These are optional but should be used following best practices as outlined in the [Google Cloud Error Model](https://cloud.google.com/apis/design/errors#error_model). | ||
|
|
||
| #### Step 4: Implementing the New Error Model | ||
|
|
||
| 1. **Consistent Use**: Ensure that the new error model is used consistently throughout the codebase. | ||
| 2. **Refactoring**: Replace existing errors in the code with the newly defined rich errors. | ||
|
|
||
| #### Step 5: Testing and Integration | ||
|
|
||
| 1. **Integration Tests**: Add integration tests for the new error handling mechanism. | ||
| 2. **Follow Existing Patterns**: Use the existing integration tests for the state API as a reference for structure and best practices. For example, these are the integration tests for the errors in state api: `/tests/integration/suite/daprd/state/grpc/errors.go` and `/tests/integration/suite/daprd/state/http/errors.go` |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,114 @@ | ||
| /* | ||
| Copyright 2023 The Dapr Authors | ||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ | ||
|
|
||
| package errors | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "net/http" | ||
| "strconv" | ||
|
|
||
| "google.golang.org/grpc/codes" | ||
|
|
||
| kiterrors "github.com/dapr/kit/errors" | ||
| ) | ||
|
|
||
| func StateStoreNotConfigured() error { | ||
| return kiterrors.NewBuilder( | ||
| codes.FailedPrecondition, | ||
| http.StatusInternalServerError, | ||
| "state store is not configured", | ||
| "ERR_STATE_STORE_NOT_CONFIGURED", | ||
| ). | ||
| WithErrorInfo(kiterrors.CodePrefixStateStore+kiterrors.CodeNotConfigured, nil). | ||
| Build() | ||
| } | ||
|
|
||
| func StateStoreNotFound(storeName string) error { | ||
| return kiterrors.NewBuilder( | ||
| codes.InvalidArgument, // TODO We should change this at some point. It should be codes.NotFound, but it will be a breaking change. | ||
| http.StatusBadRequest, | ||
| fmt.Sprintf("state store %s is not found", storeName), | ||
| "ERR_STATE_STORE_NOT_FOUND", | ||
| ). | ||
| WithErrorInfo(kiterrors.CodePrefixStateStore+kiterrors.CodeNotFound, nil). | ||
| Build() | ||
| } | ||
|
|
||
| func StateStoreInvalidKeyName(storeName string, key string, msg string) error { | ||
| return kiterrors.NewBuilder( | ||
| codes.InvalidArgument, | ||
| http.StatusBadRequest, | ||
| msg, | ||
| "ERR_MALFORMED_REQUEST", | ||
| ).WithErrorInfo(kiterrors.CodePrefixStateStore+kiterrors.CodeIllegalKey, nil). | ||
| WithResourceInfo("state", storeName, "", ""). | ||
| WithFieldViolation(key, msg). | ||
| Build() | ||
| } | ||
|
|
||
| /**** Transactions ****/ | ||
|
|
||
| func StateStoreTransactionsNotSupported(storeName string) error { | ||
| return kiterrors.NewBuilder( | ||
| codes.Unimplemented, | ||
| http.StatusInternalServerError, | ||
| fmt.Sprintf("state store %s doesn't support transactions", storeName), | ||
| "ERR_STATE_STORE_NOT_SUPPORTED", // TODO: @elena-kolevska this code misleading and also used for different things ("query unsupported"); it should be removed in the next major version | ||
| ). | ||
| WithErrorInfo(kiterrors.CodePrefixStateStore+"TRANSACTIONS_NOT_SUPPORTED", nil). | ||
| WithResourceInfo("state", storeName, "", ""). | ||
| WithHelpLink("https://docs.dapr.io/reference/components-reference/supported-state-stores/", "Check the list of state stores and the features they support"). | ||
| Build() | ||
| } | ||
|
|
||
| func StateStoreTooManyTransactionalOps(storeName string, count int, max int) error { | ||
| return kiterrors.NewBuilder( | ||
| codes.InvalidArgument, | ||
| http.StatusBadRequest, | ||
| fmt.Sprintf("the transaction contains %d operations, which is more than what the state store supports: %d", count, max), | ||
| "ERR_STATE_STORE_TOO_MANY_TRANSACTIONS", | ||
| ). | ||
| WithErrorInfo(kiterrors.CodePrefixStateStore+"TOO_MANY_TRANSACTIONS", map[string]string{ | ||
| "currentOpsTransaction": strconv.Itoa(count), | ||
| "maxOpsPerTransaction": strconv.Itoa(max), | ||
| }). | ||
| WithResourceInfo("state", storeName, "", ""). | ||
| Build() | ||
| } | ||
|
|
||
| /**** Query API ****/ | ||
|
|
||
| func StateStoreQueryUnsupported(storeName string) error { | ||
| return kiterrors.NewBuilder( | ||
| codes.Internal, | ||
| http.StatusInternalServerError, | ||
| "state store does not support querying", | ||
| "ERR_STATE_STORE_NOT_SUPPORTED", | ||
| ). | ||
| WithErrorInfo(kiterrors.CodePrefixStateStore+"QUERYING_"+kiterrors.CodeNotSupported, nil). | ||
| WithResourceInfo("state", storeName, "", ""). | ||
| Build() | ||
| } | ||
|
|
||
| func StateStoreQueryFailed(storeName string, detail string) error { | ||
| return kiterrors.NewBuilder( | ||
| codes.Internal, | ||
| http.StatusInternalServerError, | ||
| fmt.Sprintf("state store %s query failed: %s", storeName, detail), | ||
| "ERR_STATE_QUERY", | ||
| ). | ||
| WithErrorInfo(kiterrors.CodePrefixStateStore+kiterrors.CodePostfixQueryFailed, nil). | ||
| WithResourceInfo("state", storeName, "", ""). | ||
| Build() | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
GitHub Actions
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.