From 59b8d9bea510e913218a8b2efdc1c97953a2b463 Mon Sep 17 00:00:00 2001 From: Justin Won Date: Mon, 29 Jun 2026 16:27:40 -0700 Subject: [PATCH 1/3] Add core/errors package with ClassifiedError and typed error types Introduces a new core/errors package that defines: - ClassifiedError struct wrapping any error with ErrorType and Reason fields for metrics - New(errorType, reason string, err error) constructor - Pre-constructed User and Infra sentinel vars (no-arg messages) - Named struct types for all Handleable-Structured errors (User and Infra) so callers can use errors.As to extract specific fields This package will replace core/common's ClassifiedError interface and WithReason function, and all bare fmt.Errorf/errors.New call sites, in a follow-up change. Co-Authored-By: Claude Sonnet 4.6 (1M context) --- core/errors/errors.go | 334 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 334 insertions(+) create mode 100644 core/errors/errors.go diff --git a/core/errors/errors.go b/core/errors/errors.go new file mode 100644 index 0000000..2fcc9e7 --- /dev/null +++ b/core/errors/errors.go @@ -0,0 +1,334 @@ +// Copyright (c) 2025 Uber Technologies, Inc. +// +// 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 ( + stderrors "errors" + "fmt" +) + +const ( + ErrorTypeUser = "user" + ErrorTypeInfra = "infra" +) + +const ( + FailureReasonCancelled = "cancelled" + FailureReasonDeadlineExceeded = "deadline_exceeded" + FailureReasonUnknown = "unknown" + FailureReasonStorage = "storage" + FailureReasonValidation = "validation" +) + +// ClassifiedError wraps any error with an explicit error type and reason for metrics. +type ClassifiedError struct { + ErrorType string + Reason string + Err error +} + +func (e *ClassifiedError) Error() string { return e.Err.Error() } +func (e *ClassifiedError) Unwrap() error { return e.Err } + +// New constructs a ClassifiedError. +func New(errorType, reason string, err error) *ClassifiedError { + return &ClassifiedError{ErrorType: errorType, Reason: reason, Err: err} +} + +// User Sentinel vars + +var ( + ErrRootDirEmpty = New(ErrorTypeUser, FailureReasonValidation, stderrors.New("root directory cannot be empty")) + ErrBuildDescriptionEmpty = New(ErrorTypeUser, FailureReasonValidation, stderrors.New("build description is empty or invalid")) + ErrRequestNil = New(ErrorTypeUser, FailureReasonValidation, stderrors.New("request cannot be nil")) + ErrFirstRevisionRequired = New(ErrorTypeUser, FailureReasonValidation, stderrors.New("first revision is required")) + ErrSecondRevisionRequired = New(ErrorTypeUser, FailureReasonValidation, stderrors.New("second revision is required")) + ErrFirstRevisionRemoteRequired = New(ErrorTypeUser, FailureReasonValidation, stderrors.New("first revision remote is required")) + ErrFirstRevisionBaseSHARequired = New(ErrorTypeUser, FailureReasonValidation, stderrors.New("first revision base_sha is required")) + ErrSecondRevisionRemoteRequired = New(ErrorTypeUser, FailureReasonValidation, stderrors.New("second revision remote is required")) + ErrSecondRevisionBaseSHARequired = New(ErrorTypeUser, FailureReasonValidation, stderrors.New("second revision base_sha is required")) + ErrRevisionRemoteMismatch = New(ErrorTypeUser, FailureReasonValidation, stderrors.New("first and second revision must have the same remote")) +) + +// Infra Sentinel vars + +var ( + ErrParentPackageNotExist = New(ErrorTypeInfra, FailureReasonUnknown, stderrors.New("parent package does not exist")) + ErrNilReader = New(ErrorTypeInfra, FailureReasonUnknown, stderrors.New("nil reader")) + ErrRepoManagerClonePathRequired = New(ErrorTypeInfra, FailureReasonUnknown, stderrors.New("service.repo_manager_clone_path must be set when worker_root_path is specified")) + ErrNoChunksReturned = New(ErrorTypeInfra, FailureReasonUnknown, stderrors.New("no chunks returned")) +) + +// User Structured types +// Wrap at call site: New(ErrorTypeUser, FailureReasonValidation, &ErrFoo{...}) + +// ErrRegexPatternInvalid is returned when a regex pattern fails to compile. +type ErrRegexPatternInvalid struct { + Pattern string + Cause error +} + +func (e *ErrRegexPatternInvalid) Error() string { + return fmt.Sprintf("invalid pattern %q: %v", e.Pattern, e.Cause) +} +func (e *ErrRegexPatternInvalid) Unwrap() error { return e.Cause } + +// ErrBuildDescriptionMissingFields is returned when required build description fields are absent. +type ErrBuildDescriptionMissingFields struct { + BaseSha string + Remote string +} + +func (e *ErrBuildDescriptionMissingFields) Error() string { + return fmt.Sprintf("build description is missing required fields: base_sha: %s, remote: %s", e.BaseSha, e.Remote) +} + +// Infra Structured types +// Wrap at call site: New(ErrorTypeInfra, FailureReasonUnknown, &ErrFoo{...}) + +// ErrTargetTypeNotHandled is returned when a buildpb.Target proto has an unexpected type. +type ErrTargetTypeNotHandled struct{ TargetType string } + +func (e *ErrTargetTypeNotHandled) Error() string { + return fmt.Sprintf("cannot handle target type %q", e.TargetType) +} + +// ErrExternalRepositoryNotFound is returned when an external repository cannot be resolved. +type ErrExternalRepositoryNotFound struct{ Repo, Target string } + +func (e *ErrExternalRepositoryNotFound) Error() string { + return fmt.Sprintf("cannot find external repository %s from external target %s", e.Repo, e.Target) +} + +// ErrUnexpectedRepository is returned when a target references an unexpected repository. +type ErrUnexpectedRepository struct{ Target string } + +func (e *ErrUnexpectedRepository) Error() string { + return fmt.Sprintf("unexpected repository from target %s", e.Target) +} + +// ErrDownloadGraph is returned when downloading a cached graph fails. +type ErrDownloadGraph struct { + Key string + Cause error +} + +func (e *ErrDownloadGraph) Error() string { return fmt.Sprintf("download graph %s: %v", e.Key, e.Cause) } +func (e *ErrDownloadGraph) Unwrap() error { return e.Cause } + +// ErrDecodeGraph is returned when decoding a cached graph fails. +type ErrDecodeGraph struct { + Key string + Cause error +} + +func (e *ErrDecodeGraph) Error() string { return fmt.Sprintf("decode graph %s: %v", e.Key, e.Cause) } +func (e *ErrDecodeGraph) Unwrap() error { return e.Cause } + +// ErrCachePathInvalid is returned when a cache path does not match the expected format. +type ErrCachePathInvalid struct{ Path string } + +func (e *ErrCachePathInvalid) Error() string { + return fmt.Sprintf("cache path should have form date/TIMESTAMP_SHA: %s", e.Path) +} + +// ErrCacheFilenameInvalid is returned when a cache filename does not match the expected format. +type ErrCacheFilenameInvalid struct{ Filename string } + +func (e *ErrCacheFilenameInvalid) Error() string { + return fmt.Sprintf("cache file name should have form TIMESTAMP_SHA: %s", e.Filename) +} + +// ErrParseTimestamp is returned when parsing a timestamp from a cache entry fails. +type ErrParseTimestamp struct{ Cause error } + +func (e *ErrParseTimestamp) Error() string { return fmt.Sprintf("parse timestamp: %v", e.Cause) } +func (e *ErrParseTimestamp) Unwrap() error { return e.Cause } + +// ErrTargetNotFoundInGraph is returned when an expected target is absent from the graph. +type ErrTargetNotFoundInGraph struct{ ID int } + +func (e *ErrTargetNotFoundInGraph) Error() string { + return fmt.Sprintf("target %d not found in graph", e.ID) +} + +// ErrTargetMissingHash is returned when a source file or package group target lacks a hash. +type ErrTargetMissingHash struct{ Target string } + +func (e *ErrTargetMissingHash) Error() string { + return fmt.Sprintf("source file or package group target %s should already have hash", e.Target) +} + +// ErrRuleTargetMissingHash is returned when a rule target lacks a rule hash. +type ErrRuleTargetMissingHash struct{ Target string } + +func (e *ErrRuleTargetMissingHash) Error() string { + return fmt.Sprintf("rule target %s should already have rule hash", e.Target) +} + +// ErrTargetNotFound is returned when an expected target is absent from the graph. +type ErrTargetNotFound struct{ ID int } + +func (e *ErrTargetNotFound) Error() string { return fmt.Sprintf("target %d not found", e.ID) } + +// ErrDependencyNotFound is returned when a named dependency of a target is not found. +type ErrDependencyNotFound struct{ Dep, Target string } + +func (e *ErrDependencyNotFound) Error() string { + return fmt.Sprintf("dependency %s of target %s not found", e.Dep, e.Target) +} + +// ErrDependencyTargetNotFound is returned when a dependency target is absent from the graph. +type ErrDependencyTargetNotFound struct { + Dep string + ID int +} + +func (e *ErrDependencyTargetNotFound) Error() string { + return fmt.Sprintf("dependency target %s (id=%d) not found in graph", e.Dep, e.ID) +} + +// ErrUnreachableWorkspaceRoot is returned when the workspace root cannot be reached. +type ErrUnreachableWorkspaceRoot struct{ Root string } + +func (e *ErrUnreachableWorkspaceRoot) Error() string { + return fmt.Sprintf("unable to reach workspace root %q", e.Root) +} + +// ErrPRCommitHistory is returned when reading PR commit history fails. +type ErrPRCommitHistory struct{ Cause error } + +func (e *ErrPRCommitHistory) Error() string { + return fmt.Sprintf("failed to read PR commit history: %v", e.Cause) +} +func (e *ErrPRCommitHistory) Unwrap() error { return e.Cause } + +// ErrCommitNotAncestor is returned when a commit is not an ancestor of the given PR. +type ErrCommitNotAncestor struct{ Commit, PR string } + +func (e *ErrCommitNotAncestor) Error() string { + return fmt.Sprintf("commit %q is not an ancestor of PR %s", e.Commit, e.PR) +} + +// ErrBazeliskHTTPFailure is returned when bazelisk download receives a non-success HTTP response. +type ErrBazeliskHTTPFailure struct { + StatusCode int + URL string +} + +func (e *ErrBazeliskHTTPFailure) Error() string { + return fmt.Sprintf("download bazelisk: HTTP %d from %s", e.StatusCode, e.URL) +} + +// ErrBazelQuery is returned when a bazel query fails. +type ErrBazelQuery struct { + Msg string + Tail string + Cause error +} + +func (e *ErrBazelQuery) Error() string { return fmt.Sprintf("%s: %v%s", e.Msg, e.Cause, e.Tail) } +func (e *ErrBazelQuery) Unwrap() error { return e.Cause } + +// ErrCheckRefAncestor is returned when checking git ref ancestry fails. +type ErrCheckRefAncestor struct { + AncestorRef string + DescendantRef string + Cause error +} + +func (e *ErrCheckRefAncestor) Error() string { + return fmt.Sprintf("check if ref %s is ancestor of %s: %v", e.AncestorRef, e.DescendantRef, e.Cause) +} +func (e *ErrCheckRefAncestor) Unwrap() error { return e.Cause } + +// ErrWorkerPoolSizeInvalid is returned when the configured worker pool size is invalid. +type ErrWorkerPoolSizeInvalid struct{ Value int } + +func (e *ErrWorkerPoolSizeInvalid) Error() string { + return fmt.Sprintf("service.worker_pool_size must be > 0, got %d", e.Value) +} + +// ErrRepositoryRemoteEmpty is returned when a repository entry has an empty remote. +type ErrRepositoryRemoteEmpty struct{ Index int } + +func (e *ErrRepositoryRemoteEmpty) Error() string { + return fmt.Sprintf("repository[%d].remote must not be empty", e.Index) +} + +// ErrDuplicateRemote is returned when two repository entries share the same remote. +type ErrDuplicateRemote struct{ Remote string } + +func (e *ErrDuplicateRemote) Error() string { + return fmt.Sprintf("duplicate repository remote %q", e.Remote) +} + +// ErrGetTargetGraph is returned when fetching a numbered target graph fails. +type ErrGetTargetGraph struct { + Order int + Cause error +} + +func (e *ErrGetTargetGraph) Error() string { + return fmt.Sprintf("failed to get target graph #%d: %v", e.Order, e.Cause) +} +func (e *ErrGetTargetGraph) Unwrap() error { return e.Cause } + +// ErrTargetIDNotInMetadata is returned when a target ID is absent from the metadata map. +// Role identifies which target (e.g. "current", "old", "new"). +type ErrTargetIDNotInMetadata struct { + ID uint32 + Role string +} + +func (e *ErrTargetIDNotInMetadata) Error() string { + return fmt.Sprintf("%s target id %d not found in metadata", e.Role, e.ID) +} + +// ErrTargetNamesMismatch is returned when old and new target names disagree. +type ErrTargetNamesMismatch struct{ OldName, NewName string } + +func (e *ErrTargetNamesMismatch) Error() string { + return fmt.Sprintf("target names are different %s != %s", e.OldName, e.NewName) +} + +// ErrTreehashRead is returned when reading a treehash from storage fails. +type ErrTreehashRead struct { + Key string + Cause error +} + +func (e *ErrTreehashRead) Error() string { + return fmt.Sprintf("treehash read failed for key %q: %v", e.Key, e.Cause) +} +func (e *ErrTreehashRead) Unwrap() error { return e.Cause } + +// ErrTreehashBodyRead is returned when reading the body of a treehash response fails. +type ErrTreehashBodyRead struct { + Key string + Cause error +} + +func (e *ErrTreehashBodyRead) Error() string { + return fmt.Sprintf("treehash body read failed for key %q: %v", e.Key, e.Cause) +} +func (e *ErrTreehashBodyRead) Unwrap() error { return e.Cause } + +// ErrNoRepositoryConfig is returned when no repository configuration exists for a remote. +type ErrNoRepositoryConfig struct{ Remote string } + +func (e *ErrNoRepositoryConfig) Error() string { + return fmt.Sprintf("no repository configuration found for remote %q", e.Remote) +} From 75a7350d9fb0d8988d723cd5a3dc849c36b3b3c9 Mon Sep 17 00:00:00 2001 From: Justin Won Date: Mon, 29 Jun 2026 17:34:14 -0700 Subject: [PATCH 2/3] Update core/errors: refine types and add unit tests - Restore ErrParseTimestamp and ErrPRCommitHistory as typed structs with Cause field so callers can type-discriminate via errors.As - Add errors_test.go covering: New constructor, errors.As/Is traversal through ClassifiedError, sentinel identity, and Error() strings for all structured types Co-Authored-By: Claude Sonnet 4.6 (1M context) --- core/errors/errors.go | 8 +- core/errors/errors_test.go | 195 +++++++++++++++++++++++++++++++++++++ 2 files changed, 199 insertions(+), 4 deletions(-) create mode 100644 core/errors/errors_test.go diff --git a/core/errors/errors.go b/core/errors/errors.go index 2fcc9e7..eeae62e 100644 --- a/core/errors/errors.go +++ b/core/errors/errors.go @@ -47,7 +47,7 @@ func New(errorType, reason string, err error) *ClassifiedError { return &ClassifiedError{ErrorType: errorType, Reason: reason, Err: err} } -// User Sentinel vars +// User Sentinel errors var ( ErrRootDirEmpty = New(ErrorTypeUser, FailureReasonValidation, stderrors.New("root directory cannot be empty")) @@ -62,7 +62,7 @@ var ( ErrRevisionRemoteMismatch = New(ErrorTypeUser, FailureReasonValidation, stderrors.New("first and second revision must have the same remote")) ) -// Infra Sentinel vars +// Infra Sentinel errors var ( ErrParentPackageNotExist = New(ErrorTypeInfra, FailureReasonUnknown, stderrors.New("parent package does not exist")) @@ -71,7 +71,7 @@ var ( ErrNoChunksReturned = New(ErrorTypeInfra, FailureReasonUnknown, stderrors.New("no chunks returned")) ) -// User Structured types +// User Structured errors // Wrap at call site: New(ErrorTypeUser, FailureReasonValidation, &ErrFoo{...}) // ErrRegexPatternInvalid is returned when a regex pattern fails to compile. @@ -95,7 +95,7 @@ func (e *ErrBuildDescriptionMissingFields) Error() string { return fmt.Sprintf("build description is missing required fields: base_sha: %s, remote: %s", e.BaseSha, e.Remote) } -// Infra Structured types +// Infra Structured errors // Wrap at call site: New(ErrorTypeInfra, FailureReasonUnknown, &ErrFoo{...}) // ErrTargetTypeNotHandled is returned when a buildpb.Target proto has an unexpected type. diff --git a/core/errors/errors_test.go b/core/errors/errors_test.go new file mode 100644 index 0000000..eb9dcdb --- /dev/null +++ b/core/errors/errors_test.go @@ -0,0 +1,195 @@ +// Copyright (c) 2025 Uber Technologies, Inc. +// +// 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 ( + stderrors "errors" + "fmt" + "testing" +) + +func TestNew(t *testing.T) { + inner := stderrors.New("something went wrong") + ce := New(ErrorTypeInfra, FailureReasonUnknown, inner) + + if ce.ErrorType != ErrorTypeInfra { + t.Errorf("ErrorType = %q, want %q", ce.ErrorType, ErrorTypeInfra) + } + if ce.Reason != FailureReasonUnknown { + t.Errorf("Reason = %q, want %q", ce.Reason, FailureReasonUnknown) + } + if ce.Error() != inner.Error() { + t.Errorf("Error() = %q, want %q", ce.Error(), inner.Error()) + } + if stderrors.Unwrap(ce) != inner { + t.Error("Unwrap() did not return the inner error") + } +} + +func TestClassifiedError_AsTraversal(t *testing.T) { + // errors.As should find *ClassifiedError through a wrapping fmt.Errorf. + inner := stderrors.New("root cause") + ce := New(ErrorTypeUser, FailureReasonValidation, inner) + wrapped := fmt.Errorf("outer: %w", ce) + + var found *ClassifiedError + if !stderrors.As(wrapped, &found) { + t.Fatal("errors.As did not find *ClassifiedError in chain") + } + if found.ErrorType != ErrorTypeUser { + t.Errorf("ErrorType = %q, want %q", found.ErrorType, ErrorTypeUser) + } +} + +func TestClassifiedError_StructuredInnerAsTraversal(t *testing.T) { + // errors.As should traverse ClassifiedError to find the inner structured type. + inner := &ErrDownloadGraph{Key: "itg/abc", Cause: stderrors.New("io error")} + ce := New(ErrorTypeInfra, FailureReasonUnknown, inner) + + var found *ErrDownloadGraph + if !stderrors.As(ce, &found) { + t.Fatal("errors.As did not find *ErrDownloadGraph through *ClassifiedError") + } + if found.Key != "itg/abc" { + t.Errorf("Key = %q, want %q", found.Key, "itg/abc") + } +} + +func TestClassifiedError_IsTraversal(t *testing.T) { + // errors.Is should traverse ClassifiedError and into the structured type's Cause. + root := stderrors.New("root cause") + inner := &ErrDownloadGraph{Key: "k", Cause: root} + ce := New(ErrorTypeInfra, FailureReasonUnknown, inner) + + if !stderrors.Is(ce, root) { + t.Error("errors.Is did not find root cause through *ClassifiedError and *ErrDownloadGraph") + } +} + +func TestSentinelVars(t *testing.T) { + tests := []struct { + name string + sentinel *ClassifiedError + wantType string + wantMsg string + }{ + {"ErrRootDirEmpty", ErrRootDirEmpty, ErrorTypeUser, "root directory cannot be empty"}, + {"ErrRequestNil", ErrRequestNil, ErrorTypeUser, "request cannot be nil"}, + {"ErrNilReader", ErrNilReader, ErrorTypeInfra, "nil reader"}, + {"ErrNoChunksReturned", ErrNoChunksReturned, ErrorTypeInfra, "no chunks returned"}, + {"ErrParentPackageNotExist", ErrParentPackageNotExist, ErrorTypeInfra, "parent package does not exist"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.sentinel.ErrorType != tt.wantType { + t.Errorf("ErrorType = %q, want %q", tt.sentinel.ErrorType, tt.wantType) + } + if tt.sentinel.Error() != tt.wantMsg { + t.Errorf("Error() = %q, want %q", tt.sentinel.Error(), tt.wantMsg) + } + }) + } +} + +func TestSentinelIdentity(t *testing.T) { + // errors.Is on a sentinel should match by pointer identity. + if !stderrors.Is(ErrRootDirEmpty, ErrRootDirEmpty) { + t.Error("errors.Is(ErrRootDirEmpty, ErrRootDirEmpty) should be true") + } + if stderrors.Is(ErrRootDirEmpty, ErrRequestNil) { + t.Error("errors.Is(ErrRootDirEmpty, ErrRequestNil) should be false") + } +} + +func TestStructuredErrorStrings(t *testing.T) { + tests := []struct { + name string + err error + wantMsg string + }{ + { + "ErrTargetTypeNotHandled", + &ErrTargetTypeNotHandled{TargetType: "UNKNOWN"}, + `cannot handle target type "UNKNOWN"`, + }, + { + "ErrExternalRepositoryNotFound", + &ErrExternalRepositoryNotFound{Repo: "myrepo", Target: "//ext:target"}, + "cannot find external repository myrepo from external target //ext:target", + }, + { + "ErrDownloadGraph", + &ErrDownloadGraph{Key: "itg/k", Cause: stderrors.New("eof")}, + "download graph itg/k: eof", + }, + { + "ErrTargetNotFound", + &ErrTargetNotFound{ID: 42}, + "target 42 not found", + }, + { + "ErrTargetNotFoundInGraph", + &ErrTargetNotFoundInGraph{ID: 7}, + "target 7 not found in graph", + }, + { + "ErrDependencyNotFound", + &ErrDependencyNotFound{Dep: "//foo:bar", Target: "//baz:qux"}, + "dependency //foo:bar of target //baz:qux not found", + }, + { + "ErrNoRepositoryConfig", + &ErrNoRepositoryConfig{Remote: "github.com/uber/tango"}, + `no repository configuration found for remote "github.com/uber/tango"`, + }, + { + "ErrTargetIDNotInMetadata_current", + &ErrTargetIDNotInMetadata{ID: 99, Role: "current"}, + "current target id 99 not found in metadata", + }, + { + "ErrBazeliskHTTPFailure", + &ErrBazeliskHTTPFailure{StatusCode: 403, URL: "https://example.com"}, + "download bazelisk: HTTP 403 from https://example.com", + }, + { + "ErrParseTimestamp", + &ErrParseTimestamp{Cause: stderrors.New("invalid syntax")}, + "parse timestamp: invalid syntax", + }, + { + "ErrPRCommitHistory", + &ErrPRCommitHistory{Cause: stderrors.New("network error")}, + "failed to read PR commit history: network error", + }, + { + "ErrCommitNotAncestor", + &ErrCommitNotAncestor{Commit: "abc123", PR: "456"}, + `commit "abc123" is not an ancestor of PR 456`, + }, + { + "ErrRegexPatternInvalid", + &ErrRegexPatternInvalid{Pattern: "[bad", Cause: stderrors.New("missing closing ]")}, + `invalid pattern "[bad": missing closing ]`, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.err.Error(); got != tt.wantMsg { + t.Errorf("Error() = %q, want %q", got, tt.wantMsg) + } + }) + } +} From bc5ac9cd2692df303ebc17c8369323cd09c181ff Mon Sep 17 00:00:00 2001 From: Justin Won Date: Mon, 29 Jun 2026 17:45:22 -0700 Subject: [PATCH 3/3] Switch tests to testify/assert and add BUILD.bazel Co-Authored-By: Claude Sonnet 4.6 (1M context) --- core/errors/BUILD.bazel | 15 ++++ core/errors/errors.go | 4 +- core/errors/errors_test.go | 157 ++++--------------------------------- 3 files changed, 33 insertions(+), 143 deletions(-) create mode 100644 core/errors/BUILD.bazel diff --git a/core/errors/BUILD.bazel b/core/errors/BUILD.bazel new file mode 100644 index 0000000..f92dcb8 --- /dev/null +++ b/core/errors/BUILD.bazel @@ -0,0 +1,15 @@ +load("@rules_go//go:def.bzl", "go_library", "go_test") + +go_library( + name = "errors", + srcs = ["errors.go"], + importpath = "github.com/uber/tango/core/errors", + visibility = ["//visibility:public"], +) + +go_test( + name = "errors_test", + srcs = ["errors_test.go"], + embed = [":errors"], + deps = ["@com_github_stretchr_testify//assert"], +) diff --git a/core/errors/errors.go b/core/errors/errors.go index eeae62e..6ec6ca0 100644 --- a/core/errors/errors.go +++ b/core/errors/errors.go @@ -32,7 +32,7 @@ const ( FailureReasonValidation = "validation" ) -// ClassifiedError wraps any error with an explicit error type and reason for metrics. +// ClassifiedError wraps any error with an explicit error type and reason. type ClassifiedError struct { ErrorType string Reason string @@ -48,7 +48,6 @@ func New(errorType, reason string, err error) *ClassifiedError { } // User Sentinel errors - var ( ErrRootDirEmpty = New(ErrorTypeUser, FailureReasonValidation, stderrors.New("root directory cannot be empty")) ErrBuildDescriptionEmpty = New(ErrorTypeUser, FailureReasonValidation, stderrors.New("build description is empty or invalid")) @@ -63,7 +62,6 @@ var ( ) // Infra Sentinel errors - var ( ErrParentPackageNotExist = New(ErrorTypeInfra, FailureReasonUnknown, stderrors.New("parent package does not exist")) ErrNilReader = New(ErrorTypeInfra, FailureReasonUnknown, stderrors.New("nil reader")) diff --git a/core/errors/errors_test.go b/core/errors/errors_test.go index eb9dcdb..787771b 100644 --- a/core/errors/errors_test.go +++ b/core/errors/errors_test.go @@ -18,24 +18,17 @@ import ( stderrors "errors" "fmt" "testing" + + "github.com/stretchr/testify/assert" ) func TestNew(t *testing.T) { inner := stderrors.New("something went wrong") ce := New(ErrorTypeInfra, FailureReasonUnknown, inner) - if ce.ErrorType != ErrorTypeInfra { - t.Errorf("ErrorType = %q, want %q", ce.ErrorType, ErrorTypeInfra) - } - if ce.Reason != FailureReasonUnknown { - t.Errorf("Reason = %q, want %q", ce.Reason, FailureReasonUnknown) - } - if ce.Error() != inner.Error() { - t.Errorf("Error() = %q, want %q", ce.Error(), inner.Error()) - } - if stderrors.Unwrap(ce) != inner { - t.Error("Unwrap() did not return the inner error") - } + assert.Equal(t, ErrorTypeInfra, ce.ErrorType) + assert.Equal(t, FailureReasonUnknown, ce.Reason) + assert.Equal(t, inner, ce.Err) } func TestClassifiedError_AsTraversal(t *testing.T) { @@ -45,12 +38,10 @@ func TestClassifiedError_AsTraversal(t *testing.T) { wrapped := fmt.Errorf("outer: %w", ce) var found *ClassifiedError - if !stderrors.As(wrapped, &found) { - t.Fatal("errors.As did not find *ClassifiedError in chain") - } - if found.ErrorType != ErrorTypeUser { - t.Errorf("ErrorType = %q, want %q", found.ErrorType, ErrorTypeUser) - } + assert.True(t, stderrors.As(wrapped, &found)) + assert.Equal(t, ErrorTypeUser, found.ErrorType) + assert.Equal(t, FailureReasonValidation, found.Reason) + assert.Equal(t, inner, found.Err) } func TestClassifiedError_StructuredInnerAsTraversal(t *testing.T) { @@ -59,12 +50,8 @@ func TestClassifiedError_StructuredInnerAsTraversal(t *testing.T) { ce := New(ErrorTypeInfra, FailureReasonUnknown, inner) var found *ErrDownloadGraph - if !stderrors.As(ce, &found) { - t.Fatal("errors.As did not find *ErrDownloadGraph through *ClassifiedError") - } - if found.Key != "itg/abc" { - t.Errorf("Key = %q, want %q", found.Key, "itg/abc") - } + assert.True(t, stderrors.As(ce, &found)) + assert.Equal(t, "itg/abc", found.Key) } func TestClassifiedError_IsTraversal(t *testing.T) { @@ -73,123 +60,13 @@ func TestClassifiedError_IsTraversal(t *testing.T) { inner := &ErrDownloadGraph{Key: "k", Cause: root} ce := New(ErrorTypeInfra, FailureReasonUnknown, inner) - if !stderrors.Is(ce, root) { - t.Error("errors.Is did not find root cause through *ClassifiedError and *ErrDownloadGraph") - } -} - -func TestSentinelVars(t *testing.T) { - tests := []struct { - name string - sentinel *ClassifiedError - wantType string - wantMsg string - }{ - {"ErrRootDirEmpty", ErrRootDirEmpty, ErrorTypeUser, "root directory cannot be empty"}, - {"ErrRequestNil", ErrRequestNil, ErrorTypeUser, "request cannot be nil"}, - {"ErrNilReader", ErrNilReader, ErrorTypeInfra, "nil reader"}, - {"ErrNoChunksReturned", ErrNoChunksReturned, ErrorTypeInfra, "no chunks returned"}, - {"ErrParentPackageNotExist", ErrParentPackageNotExist, ErrorTypeInfra, "parent package does not exist"}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if tt.sentinel.ErrorType != tt.wantType { - t.Errorf("ErrorType = %q, want %q", tt.sentinel.ErrorType, tt.wantType) - } - if tt.sentinel.Error() != tt.wantMsg { - t.Errorf("Error() = %q, want %q", tt.sentinel.Error(), tt.wantMsg) - } - }) - } + assert.True(t, stderrors.Is(ce, root)) } -func TestSentinelIdentity(t *testing.T) { - // errors.Is on a sentinel should match by pointer identity. - if !stderrors.Is(ErrRootDirEmpty, ErrRootDirEmpty) { - t.Error("errors.Is(ErrRootDirEmpty, ErrRootDirEmpty) should be true") - } - if stderrors.Is(ErrRootDirEmpty, ErrRequestNil) { - t.Error("errors.Is(ErrRootDirEmpty, ErrRequestNil) should be false") - } -} +func TestClassifiedError_IsTraversalStructured(t *testing.T) { + // errors.Is should match the structured type itself when it is the target. + inner := &ErrDownloadGraph{Key: "k", Cause: stderrors.New("io error")} + ce := New(ErrorTypeInfra, FailureReasonUnknown, inner) -func TestStructuredErrorStrings(t *testing.T) { - tests := []struct { - name string - err error - wantMsg string - }{ - { - "ErrTargetTypeNotHandled", - &ErrTargetTypeNotHandled{TargetType: "UNKNOWN"}, - `cannot handle target type "UNKNOWN"`, - }, - { - "ErrExternalRepositoryNotFound", - &ErrExternalRepositoryNotFound{Repo: "myrepo", Target: "//ext:target"}, - "cannot find external repository myrepo from external target //ext:target", - }, - { - "ErrDownloadGraph", - &ErrDownloadGraph{Key: "itg/k", Cause: stderrors.New("eof")}, - "download graph itg/k: eof", - }, - { - "ErrTargetNotFound", - &ErrTargetNotFound{ID: 42}, - "target 42 not found", - }, - { - "ErrTargetNotFoundInGraph", - &ErrTargetNotFoundInGraph{ID: 7}, - "target 7 not found in graph", - }, - { - "ErrDependencyNotFound", - &ErrDependencyNotFound{Dep: "//foo:bar", Target: "//baz:qux"}, - "dependency //foo:bar of target //baz:qux not found", - }, - { - "ErrNoRepositoryConfig", - &ErrNoRepositoryConfig{Remote: "github.com/uber/tango"}, - `no repository configuration found for remote "github.com/uber/tango"`, - }, - { - "ErrTargetIDNotInMetadata_current", - &ErrTargetIDNotInMetadata{ID: 99, Role: "current"}, - "current target id 99 not found in metadata", - }, - { - "ErrBazeliskHTTPFailure", - &ErrBazeliskHTTPFailure{StatusCode: 403, URL: "https://example.com"}, - "download bazelisk: HTTP 403 from https://example.com", - }, - { - "ErrParseTimestamp", - &ErrParseTimestamp{Cause: stderrors.New("invalid syntax")}, - "parse timestamp: invalid syntax", - }, - { - "ErrPRCommitHistory", - &ErrPRCommitHistory{Cause: stderrors.New("network error")}, - "failed to read PR commit history: network error", - }, - { - "ErrCommitNotAncestor", - &ErrCommitNotAncestor{Commit: "abc123", PR: "456"}, - `commit "abc123" is not an ancestor of PR 456`, - }, - { - "ErrRegexPatternInvalid", - &ErrRegexPatternInvalid{Pattern: "[bad", Cause: stderrors.New("missing closing ]")}, - `invalid pattern "[bad": missing closing ]`, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := tt.err.Error(); got != tt.wantMsg { - t.Errorf("Error() = %q, want %q", got, tt.wantMsg) - } - }) - } + assert.True(t, stderrors.Is(ce, inner)) }