-
Notifications
You must be signed in to change notification settings - Fork 2
docs: Tango Errors RFC #161
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,239 @@ | ||
| # Tango Errors | ||
|
|
||
| ## Overview | ||
|
|
||
| Tango's components all return plain `error` values on their own. Without a shared type, RPCs can't distinguish bad requests from internal failures, and metrics can't tell which component is failing. | ||
|
|
||
| To solve this issue, package `core/errors` defines `TangoError`, wrapped once at the layer that knows an error's classification and preserved unchanged through any later wrapping. | ||
|
|
||
| This document covers the proto contract and the exported API of `core/errors` and `mapper`. | ||
|
|
||
| ## Proto | ||
|
|
||
| `tango/proto/tango.proto` defines the wire-level error type returned by every RPC when it fails. | ||
|
|
||
| ```protobuf | ||
| enum ErrorCode { | ||
| ERROR_UNKNOWN = 0; | ||
| ERROR_CANCELLED = 1; | ||
| ERROR_USER = 2; | ||
| ERROR_INFRA = 3; | ||
| ERROR_INFRA_RETRYABLE = 4; | ||
| } | ||
|
|
||
| message TangoError { | ||
| ErrorCode code = 1; | ||
| String message = 2; | ||
| } | ||
| ``` | ||
|
|
||
| | Error Code | gRPC Code | Definition | | ||
| |---|---|---| | ||
| | `ERROR_UNKNOWN` | `UNKNOWN` | Unexpected errors | | ||
| | `ERROR_CANCELLED` | `CANCELLED` | Client canceled the request | | ||
| | `ERROR_USER` | `INVALID_ARGUMENT` | Request validation errors (missing arg, invalid arg) | | ||
| | `ERROR_INFRA` | `INTERNAL` | Infra failures within Tango APIs not caused by the user | | ||
| | `ERROR_INFRA_RETRYABLE` | `INTERNAL` | A subset of infra errors that can be retried and expected to succeed | | ||
|
|
||
| ## Package `core/errors` | ||
|
|
||
| Package `errors` defines `TangoError`, Tango's internal error type, along with the `FailureSource` classification tag. It provides constructors so callers can wrap their errors with a `TangoError` and a `FailureSource`, while the package itself handles preserving classification through wrapping. | ||
|
|
||
| ### Index | ||
|
|
||
| - [type ErrorCode](#type-errorcode) | ||
| - [func (ErrorCode) String() string](#func-errorcode-string) | ||
| - [type FailureSource](#type-failuresource) | ||
| - [var FailureSourceUnknown, FailureSourceGit, FailureSourceBazel, ...](#failure-source-values) | ||
| - [type TangoError](#type-tangoerror) | ||
| - [func NewInfra(src FailureSource, err error) error](#func-newinfra) | ||
| - [func NewUser(src FailureSource, err error) error](#func-newuser) | ||
| - [func NewInfraRetryable(src FailureSource, err error) error](#func-newinfraretryable) | ||
| - [func (\*TangoError) Error() string](#func-tangoerror-error) | ||
| - [func (\*TangoError) Unwrap() error](#func-tangoerror-unwrap) | ||
| - [func GetErrorCode(err error) ErrorCode](#func-geterrorcode) | ||
| - [func GetFailureSource(err error) FailureSource](#func-getfailuresource) | ||
|
|
||
| ### type ErrorCode | ||
|
|
||
| ```go | ||
| type ErrorCode int | ||
| ``` | ||
|
|
||
| `ErrorCode` mirrors the proto `ErrorCode` enum and classifies a `TangoError` as a user error or an infra error (retryable or not). | ||
|
|
||
| ```go | ||
| const ( | ||
| // ErrorUnknown represents unexpected errors. | ||
| ErrorUnknown ErrorCode = iota | ||
| // ErrorCancelled represents a cancelled request. | ||
| ErrorCancelled | ||
| // ErrorUser represents request validation errors and user failures in builds. | ||
| ErrorUser | ||
| // ErrorInfra represents infra failures within Tango APIs not caused by the user. | ||
| ErrorInfra | ||
| // ErrorInfraRetryable represents a subset of infra errors that can be retried and expected to succeed. | ||
| ErrorInfraRetryable | ||
| ) | ||
| ``` | ||
|
|
||
| #### func (ErrorCode) String | ||
|
|
||
| ```go | ||
| func (code ErrorCode) String() string | ||
| ``` | ||
|
|
||
| String returns the string form of the code: `"cancelled"`, `"user"`, `"infra"`, `"infra_retryable"`, or `"unknown"` as the default. | ||
|
|
||
| ### type FailureSource | ||
|
|
||
| ```go | ||
| type FailureSource interface { | ||
| String() string | ||
| } | ||
| ``` | ||
|
|
||
| `FailureSource` represents the component where an error occurred. This tag provides extra granularity for failures so that if errors spike, it's easy to know which part of Tango they're coming from. The interface is implemented with an unexported method, so outside packages cannot create their own sources and pollute the metric's cardinality. All valid sources are defined in this package. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. in the example above,
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's an additional unexported method not shown here. |
||
|
|
||
| #### Failure Source values | ||
|
|
||
| ```go | ||
| var ( | ||
| // FailureSourceUnknown is used as a fallback when no other source applies. | ||
| FailureSourceUnknown FailureSource = source("unknown") | ||
| // FailureSourceGit represents failures in git operations. | ||
| FailureSourceGit FailureSource = source("git") | ||
| // FailureSourceBazel represents failures in bazel operations. | ||
| FailureSourceBazel FailureSource = source("bazel") | ||
| // FailureSourceITG represents failures in ITG (Incremental Target Graph). | ||
| FailureSourceITG FailureSource = source("itg") | ||
|
justinwon777 marked this conversation as resolved.
|
||
| // FailureSourceStorage represents failures in the storage implementation (disk, in-memory). | ||
| FailureSourceStorage FailureSource = source("storage") | ||
|
justinwon777 marked this conversation as resolved.
|
||
| // FailureSourceConfig represents failures in config parser. | ||
| FailureSourceConfig FailureSource = source("config") | ||
| // FailureSourceController represents failures in controller. | ||
| FailureSourceController FailureSource = source("controller") | ||
| // FailureSourceTargetHasher represents failures in target hasher. | ||
| FailureSourceTargetHasher FailureSource = source("targethasher") | ||
| // FailureSourceOrchestrator represents failures in orchestrator. | ||
| FailureSourceOrchestrator FailureSource = source("orchestrator") | ||
| // FailureSourceRepoManager represents failures in repo manager. | ||
| FailureSourceRepoManager FailureSource = source("repomanager") | ||
| ) | ||
| ``` | ||
|
|
||
| ### type TangoError | ||
|
|
||
| ```go | ||
| type TangoError struct { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you want to implement fmt.Formatter for this type? See https://pkg.go.dev/github.com/pkg/errors#hdr-Formatted_printing_of_errors. If you impl this, the errors will get a Verbose field populated using the formatter when you log it thru Zap. |
||
| failureSource FailureSource | ||
| err error | ||
| errorCode ErrorCode | ||
| } | ||
| ``` | ||
|
|
||
| `TangoError` is Tango's internal error type, carrying the underlying error, its `FailureSource`, and its `ErrorCode`. The `mapper` package uses the error and error code to build the proto `TangoError` for the RPC response, and metrics emitters use the failure source and error code as metric tags. | ||
|
|
||
| #### func (\*TangoError) Error | ||
|
|
||
| ```go | ||
| func (te *TangoError) Error() string | ||
| ``` | ||
|
|
||
| Error returns the underlying error's message. | ||
|
|
||
| #### func (\*TangoError) Unwrap | ||
|
|
||
| ```go | ||
| func (te *TangoError) Unwrap() error | ||
| ``` | ||
|
|
||
| Unwrap returns the underlying error, so `errors.Is` / `errors.As` can traverse a `TangoError`. | ||
|
|
||
| The three constructors below build a `TangoError` from an error and a `FailureSource`, classifying it with a fixed `ErrorCode`. In all three, if `err` already wraps a `TangoError`, the returned error preserves the original's source and code instead of the ones passed in. | ||
|
|
||
| #### func NewInfra | ||
|
|
||
| ```go | ||
| func NewInfra(src FailureSource, err error) error | ||
| ``` | ||
|
|
||
| NewInfra wraps err as a `TangoError` classified [`ERROR_INFRA`](#type-errorcode). | ||
|
|
||
| #### func NewUser | ||
|
|
||
| ```go | ||
| func NewUser(src FailureSource, err error) error | ||
| ``` | ||
|
|
||
| NewUser wraps err as a `TangoError` classified [`ERROR_USER`](#type-errorcode). | ||
|
|
||
| #### func NewInfraRetryable | ||
|
|
||
| ```go | ||
| func NewInfraRetryable(src FailureSource, err error) error | ||
| ``` | ||
|
|
||
| NewInfraRetryable wraps err as a `TangoError` classified [`ERROR_INFRA_RETRYABLE`](#type-errorcode). | ||
|
|
||
| ### func GetErrorCode | ||
|
|
||
| ```go | ||
| func GetErrorCode(err error) ErrorCode | ||
| ``` | ||
|
|
||
| GetErrorCode extracts the `ErrorCode` from err. If err is `context.Canceled`, `ErrorCancelled` is returned. If err wraps a `TangoError`, its code is returned. Otherwise `ErrorUnknown` is returned. | ||
|
|
||
| ### func GetFailureSource | ||
|
|
||
| ```go | ||
| func GetFailureSource(err error) FailureSource | ||
| ``` | ||
|
|
||
| GetFailureSource extracts the `FailureSource` from err. If err wraps a `TangoError`, its source is returned. Otherwise `FailureSourceUnknown` is returned. | ||
|
|
||
| ## Usage | ||
|
|
||
| ### Wrapping errors at the failure site | ||
| Callers should wrap their errors with TangoError using the provided constructors. It is up to the caller to handle classifying an error. In the following example, the bazel package creates a helper func to create a TangoError with `ErrorInfra` classification. This helper is used in `ensureBazelisk` which runs os operations that can fail with infra failures. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. potential counter case
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would expect |
||
|
|
||
| ```go | ||
| func newBazelInfraError(err error) error { | ||
| return tangoerrors.NewInfra(tangoerrors.FailureSourceBazel, err) | ||
| } | ||
|
|
||
| func ensureBazelisk(ctx context.Context) (string, error) { | ||
| cacheDir, err := os.UserCacheDir() | ||
| if err != nil { | ||
| return "", newBazelInfraError(fmt.Errorf("cache dir: %w", err)) | ||
| } | ||
| dir := filepath.Join(cacheDir, "tango", "bin") | ||
| if err := os.MkdirAll(dir, 0o755); err != nil { | ||
| return "", newBazelInfraError(fmt.Errorf("mkdir cache: %w", err)) | ||
| } | ||
| ... | ||
| } | ||
|
|
||
| ``` | ||
|
|
||
| ```go | ||
| func (c *controller) GetChangedTargets(request ..., stream ...) error { | ||
| ... | ||
| if err != nil { | ||
| return mapper.ToProtoError(err) | ||
| } | ||
| return nil | ||
| } | ||
| ``` | ||
| ### Recording failure metrics | ||
|
|
||
| In metrics package | ||
| ```go | ||
| func recordFailure(e Emitter, op string, err error) { | ||
| code := tangoerrors.GetErrorCode(err) | ||
| source := tangoerrors.GetFailureSource(err) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: most likely it will require to traverse error chain twice (once per function) so better to do in one function? |
||
|
|
||
| e.Inc(op, WithTags(failure, code, source)) | ||
| } | ||
|
|
||
| ``` | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if numbers will become a part of the contract than explicit values are better than iota