Skip to content

Downgrade gRPC errors to warnings in remote execution logging #3511

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.

Sign up for GitHub

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

Closed
sean- wants to merge 1 commit into from
+5 −3
Closed

Downgrade gRPC errors to warnings in remote execution logging #3511

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions src/remote/remote.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1022,6 +1022,8 @@ func (c *Client) logActionResult(target *core.BuildTarget, run int, message, wor
}

// A grpcLogMabob is an implementation of grpc's logging interface using our backend.
// gRPC errors are downgraded to warnings because they represent remote-side errors
// (e.g. cache misses) that the client handles gracefully by falling back to local execution.
Copy link
Copy Markdown
Contributor

@toastwaffle toastwaffle Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://xkcd.com/285/

Please could you point me to what you've checked to make this assertion about falling back (I'm not totally familiar with this area of Please)?

Copy link
Copy Markdown
Contributor Author

@sean- sean- Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This took way longer for me to run to ground than anticipated. The build succeeded, so I assumed there was a retry, but that wasn't right and there is no fallback to local execution.

The error in the PR description isn't even from the Please client. It's from Elan's server-side logging interceptor in please-servers. The GetTree handler uses multierror.Group for parallel child directory fetches, which wraps the gRPC NotFound status and loses the status code. The logging interceptor then sees Unknown instead of NotFound and logs at ERROR instead of DEBUG.

The elan.log from CI job 61131 shows the blob being uploaded at the exact same millisecond the GetTree raced to read the result:

13:31:33.453   DEBUG: Not found on /build.bazel.remote.execution.v2.ActionCache/GetActionResult: rpc error: code = NotFound desc = Blob f26f33bef5b8b4e4ec103ecf62be682bfba19c2996559a862fe7cd579e007cf0 not found
13:31:33.453   DEBUG: Not found on /build.bazel.remote.execution.v2.ActionCache/GetActionResult: rpc error: code = NotFound desc = Blob bb3121d9b0d0b3d5ad843f73d2533409a455a121225924819cffb0af99a10982 not found
13:31:33.453   DEBUG: Not found on /build.bazel.remote.execution.v2.ActionCache/GetActionResult: rpc error: code = NotFound desc = Blob cbba6b25fe8ef0353e032cc5ec240e83ab801eb2a6f663bfd49f85f279b054da not found
13:31:33.453   DEBUG: Not found on /build.bazel.remote.execution.v2.ActionCache/GetActionResult: rpc error: code = NotFound desc = Blob d786a12428f0a449401db51047ec9bf5a35cdcb6efb4ef13d0322046c9aa472c not found
13:31:33.453   DEBUG: Not found on /build.bazel.remote.execution.v2.ActionCache/GetActionResult: rpc error: code = NotFound desc = Blob ef563e154a2352a3523b4398abaceb853df8cc4373ddf5632c217bc7f5bb05ee not found

The actual fix is thought-machine/please-servers#341. Closing this.

Copy link
Copy Markdown
Contributor Author

@sean- sean- Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The race condition is addressed in thought-machine/please-servers#342

type grpcLogMabob struct{}

func (g *grpcLogMabob) Info(args ...interface{}) { log.Info("%s", args) }
Expand All @@ -1030,9 +1032,9 @@ func (g *grpcLogMabob) Infoln(args ...interface{}) { log.Info("
func (g *grpcLogMabob) Warning(args ...interface{}) { log.Warning("%s", args) }
func (g *grpcLogMabob) Warningf(format string, args ...interface{}) { log.Warning(format, args...) }
func (g *grpcLogMabob) Warningln(args ...interface{}) { log.Warning("%s", args) }
func (g *grpcLogMabob) Error(args ...interface{}) { log.Error("", args...) }
func (g *grpcLogMabob) Errorf(format string, args ...interface{}) { log.Errorf(format, args...) }
func (g *grpcLogMabob) Errorln(args ...interface{}) { log.Error("", args...) }
func (g *grpcLogMabob) Error(args ...interface{}) { log.Warning("%s", args) }
func (g *grpcLogMabob) Errorf(format string, args ...interface{}) { log.Warning(format, args...) }
func (g *grpcLogMabob) Errorln(args ...interface{}) { log.Warning("%s", args) }
func (g *grpcLogMabob) Fatal(args ...interface{}) { log.Fatal(args...) }
func (g *grpcLogMabob) Fatalf(format string, args ...interface{}) { log.Fatalf(format, args...) }
func (g *grpcLogMabob) Fatalln(args ...interface{}) { log.Fatal(args...) }
Expand Down
Loading