From 64cae9c8608731897856d2abc54cee391deaa7b9 Mon Sep 17 00:00:00 2001
From: sergeyb <sergeyb@uber.com>
Date: Tue, 30 Jun 2026 20:07:02 +0000
Subject: [PATCH] fix(bazel): surface swallowed Close, remove, and logger-init
 errors
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The bazelisk download swallowed several errors around the temp-file install:

- The success-path tmp.Close() was ignored, so a failed flush (disk full,
  network FS) could leave a truncated binary that was then chmod'd and
  renamed into place — defeating the temp-file-then-rename guard. Its error
  is now checked and propagated.
- The io.Copy error path discarded tmp.Close(); it is now folded into the
  returned error via errors.Join.
- The deferred os.Remove(tmpPath) ignored its error and ran even on success
  (where the file was already renamed away, guaranteeing ENOENT). Cleanup
  now runs only on failure and joins any removal error into the result.

Also handle the discarded zap.NewDevelopment() error in the example server
and client, which would nil-panic at defer zl.Sync() on the (rare) init
failure.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 core/bazel/bazel.go      | 19 ++++++++++++++-----
 example/client/client.go |  6 +++++-
 example/main.go          |  5 ++++-
 3 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/core/bazel/bazel.go b/core/bazel/bazel.go
index 4b13886..217283a 100644
--- a/core/bazel/bazel.go
+++ b/core/bazel/bazel.go
@@ -16,6 +16,7 @@ package bazel
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -113,7 +114,7 @@ func detectBazelExecutable(ctx context.Context, bazelCommand string) (string, er
 }
 
 // ensureBazelisk returns the path to a cached bazelisk binary,
-func ensureBazelisk(ctx context.Context) (string, error) {
+func ensureBazelisk(ctx context.Context) (_ string, retErr error) {
 	cacheDir, err := os.UserCacheDir()
 	if err != nil {
 		return "", fmt.Errorf("cache dir: %w", err)
@@ -148,13 +149,21 @@ func ensureBazelisk(ctx context.Context) (string, error) {
 		return "", fmt.Errorf("create temp file: %w", err)
 	}
 	tmpPath := tmp.Name()
-	defer os.Remove(tmpPath)
+	// On failure, remove the temp file and fold any removal error into the
+	// returned error. On the success path the file has been renamed into
+	// place, so there is nothing to remove.
+	defer func() {
+		if retErr != nil {
+			retErr = errors.Join(retErr, os.Remove(tmpPath))
+		}
+	}()
 
 	if _, err := io.Copy(tmp, resp.Body); err != nil {
-		tmp.Close()
-		return "", fmt.Errorf("write bazelisk: %w", err)
+		return "", fmt.Errorf("write bazelisk: %w", errors.Join(err, tmp.Close()))
+	}
+	if err := tmp.Close(); err != nil {
+		return "", fmt.Errorf("close bazelisk temp: %w", err)
 	}
-	tmp.Close()
 	if err := os.Chmod(tmpPath, 0o755); err != nil {
 		return "", fmt.Errorf("chmod bazelisk: %w", err)
 	}
diff --git a/example/client/client.go b/example/client/client.go
index fa44871..0d2fba5 100644
--- a/example/client/client.go
+++ b/example/client/client.go
@@ -50,7 +50,11 @@ func main() {
 
 	grpcTransport := yarpcgrpc.NewTransport()
 	out := grpcTransport.NewSingleOutbound(*addr)
-	zl, _ := zap.NewDevelopment()
+	zl, err := zap.NewDevelopment()
+	if err != nil {
+		fmt.Fprintf(os.Stderr, "init logger: %v\n", err)
+		os.Exit(1)
+	}
 	defer zl.Sync()
 	logger := zl.Sugar()
 	dispatcher := yarpc.NewDispatcher(yarpc.Config{
diff --git a/example/main.go b/example/main.go
index 2f4077a..991cc31 100644
--- a/example/main.go
+++ b/example/main.go
@@ -45,7 +45,10 @@ func main() {
 }
 
 func run() error {
-	zl, _ := zap.NewDevelopment()
+	zl, err := zap.NewDevelopment()
+	if err != nil {
+		return fmt.Errorf("init logger: %w", err)
+	}
 	defer zl.Sync()
 	logger := zl.Sugar()