From 05d79ad60c9c05bff361a87e166801d27f91248a Mon Sep 17 00:00:00 2001 From: Yushan Lin Date: Thu, 23 Jul 2026 17:41:28 -0700 Subject: [PATCH 1/2] fix(bazel): classify bazel query timeout as an infra error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: Intent: - A bazel query that exceeds the configured query timeout was returned as a plain error, so it fell through to the default infra classification only incidentally rather than being explicitly and durably classified. Changes: - Add core/bazel.ErrQueryTimeout, a sentinel wrapped into the returned error when the query's context deadline is exceeded. - Add graphrunner.classifyBazelQueryError, following the orchestrator/errors.go pattern, which classifies ErrQueryTimeout as tangoerrors.ErrorInfra. - Wire classifyBazelQueryError into nativeGraphRunner.Compute. --- Generated by the 🪄 [pr-create](https://sg.uberinternal.com/code.uber.internal/uber-code/devexp-agent-marketplace/-/blob/claude-code/plugins/dev/uber-dev/skills/pr-create/SKILL.md) skill in devexp-agent-marketplace --- core/bazel/query.go | 12 ++++++++++++ core/bazel/query_test.go | 3 +-- graphrunner/BUILD.bazel | 8 +++++++- graphrunner/errors.go | 31 +++++++++++++++++++++++++++++ graphrunner/errors_test.go | 40 ++++++++++++++++++++++++++++++++++++++ graphrunner/native.go | 2 +- 6 files changed, 92 insertions(+), 4 deletions(-) create mode 100644 graphrunner/errors.go create mode 100644 graphrunner/errors_test.go diff --git a/core/bazel/query.go b/core/bazel/query.go index 8b94b129..e9b35fe9 100644 --- a/core/bazel/query.go +++ b/core/bazel/query.go @@ -18,6 +18,7 @@ import ( "bytes" "compress/gzip" "context" + "errors" "fmt" "io" "os" @@ -29,6 +30,11 @@ import ( "google.golang.org/protobuf/proto" ) +// ErrQueryTimeout indicates a bazel query did not complete within the +// configured query timeout. Callers can classify this sentinel as an infra +// error (see core/errors and docs/errors/errors.md). +var ErrQueryTimeout = errors.New("bazel query timed out") + func (b *BazelClient) setupCommand(ctx context.Context, query string, startupOptions []string, additionalArgs ...string) commander { // Build command: bazel query --output=streamed_proto args := make([]string, 0, len(startupOptions)+1+len(additionalArgs)+2) @@ -95,9 +101,15 @@ func (b *BazelClient) executeQueryInternal(ctx context.Context, query string, st waitErr := cmd.Wait() streamErr := g.Wait() if waitErr != nil { + if errors.Is(cmdCtx.Err(), context.DeadlineExceeded) { + return queryResults, b.wrapQueryFailure("bazel query failed", fmt.Errorf("%w: %w", ErrQueryTimeout, waitErr), &stderrBuf) + } return queryResults, b.wrapQueryFailure("bazel query failed", waitErr, &stderrBuf) } if streamErr != nil { + if errors.Is(cmdCtx.Err(), context.DeadlineExceeded) { + return nil, b.wrapQueryFailure("stream processing failed", fmt.Errorf("%w: %w", ErrQueryTimeout, streamErr), &stderrBuf) + } return nil, b.wrapQueryFailure("stream processing failed", streamErr, &stderrBuf) } b.logger.Debugw("Parsed targets from bazel query", zap.Int("target_count", len(queryResults.Target))) diff --git a/core/bazel/query_test.go b/core/bazel/query_test.go index c506214f..a9430e71 100644 --- a/core/bazel/query_test.go +++ b/core/bazel/query_test.go @@ -171,8 +171,7 @@ func TestExecuteQueryInternal_ContextTimeout(t *testing.T) { result, err := client.executeQueryInternal(context.Background(), "//...", nil) require.Nil(t, result) require.Error(t, err) - // Should get timeout or deadline exceeded error - assert.Contains(t, err.Error(), "deadline exceeded") + assert.ErrorIs(t, err, ErrQueryTimeout) } func TestExecuteQueryInternal_Failures(t *testing.T) { diff --git a/graphrunner/BUILD.bazel b/graphrunner/BUILD.bazel index 99f8b0a7..3170700d 100644 --- a/graphrunner/BUILD.bazel +++ b/graphrunner/BUILD.bazel @@ -3,6 +3,7 @@ load("@rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "graphrunner", srcs = [ + "errors.go", "graph_runner.go", "metrics.go", "native.go", @@ -13,6 +14,7 @@ go_library( deps = [ "//config", "//core/bazel", + "//core/errors", "//core/git", "//core/targethasher", "//core/workspace", @@ -23,11 +25,15 @@ go_library( go_test( name = "graphrunner_test", - srcs = ["native_test.go"], + srcs = [ + "errors_test.go", + "native_test.go", + ], embed = [":graphrunner"], deps = [ "//core/bazel", "//core/bazel/bazelmock", + "//core/errors", "//core/git/gitmock", "//core/workspace", "@com_github_bazelbuild_buildtools//build_proto", diff --git a/graphrunner/errors.go b/graphrunner/errors.go new file mode 100644 index 00000000..fe488289 --- /dev/null +++ b/graphrunner/errors.go @@ -0,0 +1,31 @@ +// 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 graphrunner + +import ( + "errors" + "fmt" + + "github.com/uber/tango/core/bazel" + tangoerrors "github.com/uber/tango/core/errors" +) + +func classifyBazelQueryError(err error) error { + wrappedErr := fmt.Errorf("bazel query: %w", err) + if errors.Is(err, bazel.ErrQueryTimeout) { + return tangoerrors.NewInfra(wrappedErr) + } + return wrappedErr +} diff --git a/graphrunner/errors_test.go b/graphrunner/errors_test.go new file mode 100644 index 00000000..481e5dfb --- /dev/null +++ b/graphrunner/errors_test.go @@ -0,0 +1,40 @@ +// 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 graphrunner + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/uber/tango/core/bazel" + tangoerrors "github.com/uber/tango/core/errors" +) + +func TestClassifyBazelQueryError(t *testing.T) { + t.Run("query timeout is classified as infra", func(t *testing.T) { + err := classifyBazelQueryError(bazel.ErrQueryTimeout) + assert.Equal(t, tangoerrors.ErrorInfra, tangoerrors.GetErrorCode(err)) + assert.True(t, errors.Is(err, bazel.ErrQueryTimeout)) + }) + + t.Run("other errors are left unclassified", func(t *testing.T) { + otherErr := errors.New("some other bazel failure") + err := classifyBazelQueryError(otherErr) + var te *tangoerrors.TangoError + assert.False(t, errors.As(err, &te)) + assert.True(t, errors.Is(err, otherErr)) + }) +} diff --git a/graphrunner/native.go b/graphrunner/native.go index 50f3a1f2..3b2fad79 100644 --- a/graphrunner/native.go +++ b/graphrunner/native.go @@ -78,7 +78,7 @@ func (g *nativeGraphRunner) Compute(ctx context.Context, ws workspace.Workspace) }) g.emitter.DurationHistogram(_opCompute, "bazel_query_duration", _phaseDurationBuckets).RecordDuration(time.Since(bazelStart)) if err != nil { - return targethasher.EmptyResult(), err + return targethasher.EmptyResult(), classifyBazelQueryError(err) } gitStart := time.Now() From 40dfaa4bf0d891f54fc29e0d6086dd1835f67756 Mon Sep 17 00:00:00 2001 From: Yushan Lin Date: Thu, 23 Jul 2026 20:18:05 -0700 Subject: [PATCH 2/2] fix(bazel): classify bazel client/query failures as infra errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: Intent: - Any bazel query timeout (and other bazel failures) already fell through to the ErrorInfra default in tangoerrors.GetErrorCode, since nothing wrapped them in a TangoError. Classify these failures explicitly at the two call sites so the classification is intentional rather than incidental, and to give future callers a place to hang more specific classification (e.g. retryable) if it's ever needed. Changes: - Drop the ErrQueryTimeout sentinel added in the previous commit; a bazel query timeout isn't distinguishable from any other bazel query failure for classification purposes. - graphrunner.classifyBazelQueryError now unconditionally wraps a bazel query failure as tangoerrors.NewInfra. - Add orchestrator.classifyBazelClientError, wrapping a bazel.NewBazelClient failure as tangoerrors.NewInfra, and wire it into nativeOrchestrator. --- Generated by the 🪄 [pr-create](https://sg.uberinternal.com/code.uber.internal/uber-code/devexp-agent-marketplace/-/blob/claude-code/plugins/dev/uber-dev/skills/pr-create/SKILL.md) skill in devexp-agent-marketplace --- core/bazel/query.go | 12 ------------ core/bazel/query_test.go | 3 ++- graphrunner/errors.go | 10 +++------- graphrunner/errors_test.go | 18 ++++-------------- orchestrator/errors.go | 5 +++++ orchestrator/native_orchestrator.go | 2 +- 6 files changed, 15 insertions(+), 35 deletions(-) diff --git a/core/bazel/query.go b/core/bazel/query.go index e9b35fe9..8b94b129 100644 --- a/core/bazel/query.go +++ b/core/bazel/query.go @@ -18,7 +18,6 @@ import ( "bytes" "compress/gzip" "context" - "errors" "fmt" "io" "os" @@ -30,11 +29,6 @@ import ( "google.golang.org/protobuf/proto" ) -// ErrQueryTimeout indicates a bazel query did not complete within the -// configured query timeout. Callers can classify this sentinel as an infra -// error (see core/errors and docs/errors/errors.md). -var ErrQueryTimeout = errors.New("bazel query timed out") - func (b *BazelClient) setupCommand(ctx context.Context, query string, startupOptions []string, additionalArgs ...string) commander { // Build command: bazel query --output=streamed_proto args := make([]string, 0, len(startupOptions)+1+len(additionalArgs)+2) @@ -101,15 +95,9 @@ func (b *BazelClient) executeQueryInternal(ctx context.Context, query string, st waitErr := cmd.Wait() streamErr := g.Wait() if waitErr != nil { - if errors.Is(cmdCtx.Err(), context.DeadlineExceeded) { - return queryResults, b.wrapQueryFailure("bazel query failed", fmt.Errorf("%w: %w", ErrQueryTimeout, waitErr), &stderrBuf) - } return queryResults, b.wrapQueryFailure("bazel query failed", waitErr, &stderrBuf) } if streamErr != nil { - if errors.Is(cmdCtx.Err(), context.DeadlineExceeded) { - return nil, b.wrapQueryFailure("stream processing failed", fmt.Errorf("%w: %w", ErrQueryTimeout, streamErr), &stderrBuf) - } return nil, b.wrapQueryFailure("stream processing failed", streamErr, &stderrBuf) } b.logger.Debugw("Parsed targets from bazel query", zap.Int("target_count", len(queryResults.Target))) diff --git a/core/bazel/query_test.go b/core/bazel/query_test.go index a9430e71..c506214f 100644 --- a/core/bazel/query_test.go +++ b/core/bazel/query_test.go @@ -171,7 +171,8 @@ func TestExecuteQueryInternal_ContextTimeout(t *testing.T) { result, err := client.executeQueryInternal(context.Background(), "//...", nil) require.Nil(t, result) require.Error(t, err) - assert.ErrorIs(t, err, ErrQueryTimeout) + // Should get timeout or deadline exceeded error + assert.Contains(t, err.Error(), "deadline exceeded") } func TestExecuteQueryInternal_Failures(t *testing.T) { diff --git a/graphrunner/errors.go b/graphrunner/errors.go index fe488289..bc33d140 100644 --- a/graphrunner/errors.go +++ b/graphrunner/errors.go @@ -15,17 +15,13 @@ package graphrunner import ( - "errors" "fmt" - "github.com/uber/tango/core/bazel" tangoerrors "github.com/uber/tango/core/errors" ) +// classifyBazelQueryError classifies any bazel query failure as an infra error. func classifyBazelQueryError(err error) error { - wrappedErr := fmt.Errorf("bazel query: %w", err) - if errors.Is(err, bazel.ErrQueryTimeout) { - return tangoerrors.NewInfra(wrappedErr) - } - return wrappedErr + return tangoerrors.NewInfra(fmt.Errorf("bazel query: %w", err)) } + diff --git a/graphrunner/errors_test.go b/graphrunner/errors_test.go index 481e5dfb..7e7c9ef1 100644 --- a/graphrunner/errors_test.go +++ b/graphrunner/errors_test.go @@ -19,22 +19,12 @@ import ( "testing" "github.com/stretchr/testify/assert" - "github.com/uber/tango/core/bazel" tangoerrors "github.com/uber/tango/core/errors" ) func TestClassifyBazelQueryError(t *testing.T) { - t.Run("query timeout is classified as infra", func(t *testing.T) { - err := classifyBazelQueryError(bazel.ErrQueryTimeout) - assert.Equal(t, tangoerrors.ErrorInfra, tangoerrors.GetErrorCode(err)) - assert.True(t, errors.Is(err, bazel.ErrQueryTimeout)) - }) - - t.Run("other errors are left unclassified", func(t *testing.T) { - otherErr := errors.New("some other bazel failure") - err := classifyBazelQueryError(otherErr) - var te *tangoerrors.TangoError - assert.False(t, errors.As(err, &te)) - assert.True(t, errors.Is(err, otherErr)) - }) + cause := errors.New("some bazel query failure") + err := classifyBazelQueryError(cause) + assert.Equal(t, tangoerrors.ErrorInfra, tangoerrors.GetErrorCode(err)) + assert.ErrorIs(t, err, cause) } diff --git a/orchestrator/errors.go b/orchestrator/errors.go index 1a422946..f41c4940 100644 --- a/orchestrator/errors.go +++ b/orchestrator/errors.go @@ -29,3 +29,8 @@ func classifyLeaseError(err error) error { } return tangoerrors.NewInfra(wrappedErr) } + +// classifyBazelClientError classifies any bazel client creation failure as an infra error. +func classifyBazelClientError(err error) error { + return tangoerrors.NewInfra(fmt.Errorf("create bazel client: %w", err)) +} diff --git a/orchestrator/native_orchestrator.go b/orchestrator/native_orchestrator.go index 26d744c7..3d77bb2f 100644 --- a/orchestrator/native_orchestrator.go +++ b/orchestrator/native_orchestrator.go @@ -196,7 +196,7 @@ func (b *nativeOrchestrator) GetTargetGraph(ctx context.Context, req entity.GetT StreamLogs: repoCfg.StreamBazelLogs, }) if err != nil { - return nil, fmt.Errorf("create bazel client: %w", err) + return nil, classifyBazelClientError(err) } // Use default native graph runner runner = graphrunner.NewNativeGraphRunner(graphrunner.NativeGraphRunnerParams{