diff --git a/core/bazel/query.go b/core/bazel/query.go index 8b94b129..1a6ffc26 100644 --- a/core/bazel/query.go +++ b/core/bazel/query.go @@ -95,20 +95,27 @@ func (b *BazelClient) executeQueryInternal(ctx context.Context, query string, st waitErr := cmd.Wait() streamErr := g.Wait() if waitErr != nil { - return queryResults, b.wrapQueryFailure("bazel query failed", waitErr, &stderrBuf) + return queryResults, b.wrapQueryFailure(cmdCtx, "bazel query failed", waitErr, &stderrBuf) } if streamErr != nil { - return nil, b.wrapQueryFailure("stream processing failed", streamErr, &stderrBuf) + return nil, b.wrapQueryFailure(cmdCtx, "stream processing failed", streamErr, &stderrBuf) } b.logger.Debugw("Parsed targets from bazel query", zap.Int("target_count", len(queryResults.Target))) return queryResults, nil } -// wrapQueryFailure logs the failure and returns a wrapped error. When stderr -// was captured (streamLogs off), its contents are appended so the failure is -// self-contained. When streamLogs is on the operator has already seen stderr -// live, so it's omitted. -func (b *BazelClient) wrapQueryFailure(msg string, cause error, stderrBuf *bytes.Buffer) error { +// wrapQueryFailure logs the failure and returns a wrapped error. ctx is the +// query's own timeout context (not a parent cancellation): if it has hit its +// deadline, cause is additionally wrapped with ctx.Err() (context.DeadlineExceeded) +// so callers can identify via errors.Is that this failure was the query's own +// timeout elapsing, as opposed to the caller disconnecting, without a +// dedicated sentinel error. When stderr was captured (streamLogs off), its +// contents are appended so the failure is self-contained. When streamLogs is +// on the operator has already seen stderr live, so it's omitted. +func (b *BazelClient) wrapQueryFailure(ctx context.Context, msg string, cause error, stderrBuf *bytes.Buffer) error { + if ctxErr := ctx.Err(); ctxErr == context.DeadlineExceeded { + cause = fmt.Errorf("%w: %w", ctxErr, cause) + } tail := "" if !b.streamLogs { tail = "\nstderr:\n" + stderrBuf.String() diff --git a/core/bazel/query_test.go b/core/bazel/query_test.go index c506214f..f0b98b67 100644 --- a/core/bazel/query_test.go +++ b/core/bazel/query_test.go @@ -171,8 +171,79 @@ 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, context.DeadlineExceeded) +} + +func TestExecuteQueryInternal_StreamTimeoutWithoutWaitError(t *testing.T) { + defer goleak.VerifyNone(t) + ctrl := gomock.NewController(t) + mockCmd := commandermock.NewMockcommander(ctrl) + + prStdout, pwStdout := io.Pipe() + prStderr, pwStderr := io.Pipe() + + gomock.InOrder( + mockCmd.EXPECT().StdoutPipe().Return(prStdout, nil), + mockCmd.EXPECT().StderrPipe().Return(prStderr, nil), + mockCmd.EXPECT().Start().Return(nil), + // The process itself exits cleanly right at the deadline, but the + // stream-reading goroutines are still blocked mid-read when their + // context is canceled. + mockCmd.EXPECT().Wait().Return(nil), + ) + + client, err := NewBazelClient(context.Background(), Params{ + BazelCommand: "bazel", + WorkspacePath: "/tmp/test", + Logger: zap.NewNop().Sugar(), + EnvVarsMap: map[string]string{}, + QueryTimeout: 10 * time.Millisecond, // Short timeout for test + ExecCommandContext: func(ctx context.Context, name string, arg ...string) commander { + // Simulate process behavior: when context is cancelled, close pipes + // to unblock the stream-reading goroutines. + go func() { + <-ctx.Done() + pwStdout.Close() + pwStderr.Close() + }() + return mockCmd + }, + }) + require.NoError(t, err) + result, err := client.executeQueryInternal(context.Background(), "//...", nil) + require.Nil(t, result) + require.Error(t, err) + assert.ErrorIs(t, err, context.DeadlineExceeded) +} + +func TestExecuteQueryInternal_WaitFailureWithoutTimeout(t *testing.T) { + defer goleak.VerifyNone(t) + ctrl := gomock.NewController(t) + mockCmd := commandermock.NewMockcommander(ctrl) + + gomock.InOrder( + mockCmd.EXPECT().StdoutPipe().Return(io.NopCloser(strings.NewReader("")), nil), + mockCmd.EXPECT().StderrPipe().Return(io.NopCloser(strings.NewReader("")), nil), + mockCmd.EXPECT().Start().Return(nil), + mockCmd.EXPECT().Wait().Return(errors.New("exit status 1")), + ) + + client, err := NewBazelClient(context.Background(), Params{ + BazelCommand: "bazel", + WorkspacePath: "/tmp/test", + Logger: zap.NewNop().Sugar(), + EnvVarsMap: map[string]string{}, + ExecCommandContext: func(ctx context.Context, name string, arg ...string) commander { + return mockCmd + }, + }) + require.NoError(t, err) + _, err = client.executeQueryInternal(context.Background(), "//...", nil) + require.Error(t, err) + // A plain wait failure without the query's own context deadline elapsing + // (e.g. a parent cancellation, or bazel exiting non-zero) is not a + // timeout. + assert.NotErrorIs(t, err, context.DeadlineExceeded) } func TestExecuteQueryInternal_Failures(t *testing.T) {