Skip to content

Commit 1eaaf05

Browse files
authored
Add more context to errors in remotebazel.go (#11414)
A customer reported a NotFound error and it's hard to tell where it's coming from - the console just shows "rpc error code = NotFound desc = missing digest" with no additional context. Ensure all errors are wrapped so that we can diagnose errors more easily.
1 parent dd70c6f commit 1eaaf05

File tree

1 file changed

+36
-36
lines changed

1 file changed

+36
-36
lines changed

cli/remotebazel/remotebazel.go

Lines changed: 36 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ func determineRemote() (*gitRemote, error) {
216216
Options: remoteNames,
217217
}
218218
if err := survey.AskOne(prompt, &selectedRemoteAndURL); err != nil {
219-
return nil, err
219+
return nil, fmt.Errorf("select git remote: %w", err)
220220
}
221221

222222
selectedRemote := strings.Split(selectedRemoteAndURL, " (")[0]
@@ -295,7 +295,7 @@ func runCommand(name string, args ...string) (string, error) {
295295
func isBinaryFile(path string) (bool, error) {
296296
fileDetails, err := runCommand("file", "--mime", path)
297297
if err != nil {
298-
return false, err
298+
return false, fmt.Errorf("inspect file mime: %w", err)
299299
}
300300
isBinary := strings.Contains(fileDetails, "charset=binary")
301301
return isBinary, nil
@@ -304,7 +304,7 @@ func isBinaryFile(path string) (bool, error) {
304304
func diffUntrackedFile(path string) (string, error) {
305305
isBinary, err := isBinaryFile(path)
306306
if err != nil {
307-
return "", err
307+
return "", fmt.Errorf("check whether %q is binary: %w", path, err)
308308
}
309309

310310
args := []string{"diff", "--no-index", "/dev/null", path}
@@ -316,7 +316,7 @@ func diffUntrackedFile(path string) (string, error) {
316316
// `git diff` returns exit code 1 if there is (valid) diff. Explicitly
317317
// check for this case.
318318
if !strings.Contains(patch, "diff --git") {
319-
return "", err
319+
return "", fmt.Errorf("diff untracked file %q: %w", path, err)
320320
}
321321
}
322322

@@ -385,7 +385,7 @@ func getBaseBranchAndCommit(remoteData string) (branch string, commit string, er
385385

386386
currentBranch, err := getCurrentRef()
387387
if err != nil {
388-
return "", "", err
388+
return "", "", fmt.Errorf("get current ref: %w", err)
389389
}
390390

391391
currentBranchExistsRemotely := branchExistsRemotely(remoteData, currentBranch)
@@ -419,7 +419,7 @@ func getBaseBranchAndCommit(remoteData string) (branch string, commit string, er
419419

420420
defaultBranchCommitHash, err := getHeadCommitForLocalBranch(defaultBranch)
421421
if err != nil {
422-
return "", "", err
422+
return "", "", fmt.Errorf("get head commit for local branch %q: %w", defaultBranch, err)
423423
}
424424
commit = defaultBranchCommitHash
425425
}
@@ -644,7 +644,7 @@ func streamLogs(ctx context.Context, bbClient bbspb.BuildBuddyServiceClient, inv
644644
MinLines: 100,
645645
})
646646
if err != nil {
647-
return err
647+
return status.WrapError(err, "get event log chunk")
648648
}
649649

650650
chunks = append(chunks, l)
@@ -687,7 +687,7 @@ func printLogs(ctx context.Context, bbClient bbspb.BuildBuddyServiceClient, invo
687687
MinLines: 100,
688688
})
689689
if err != nil {
690-
return err
690+
return status.WrapError(err, "get event log chunk")
691691
}
692692

693693
if l.GetLive() {
@@ -706,23 +706,23 @@ func printLogs(ctx context.Context, bbClient bbspb.BuildBuddyServiceClient, invo
706706

707707
func downloadFile(ctx context.Context, bsClient bspb.ByteStreamClient, resourceName *digest.CASResourceName, outFile string) error {
708708
if err := os.MkdirAll(filepath.Dir(outFile), 0755); err != nil {
709-
return err
709+
return fmt.Errorf("create output dir for %q: %w", outFile, err)
710710
}
711711
out, err := os.Create(outFile)
712712
if err != nil {
713-
return err
713+
return fmt.Errorf("create output file %q: %w", outFile, err)
714714
}
715715
defer out.Close()
716716
if err := cachetools.GetBlob(ctx, bsClient, resourceName, out); err != nil {
717-
return err
717+
return status.WrapError(err, "download blob")
718718
}
719719
return nil
720720
}
721721

722722
func lookupBazelInvocationOutputs(ctx context.Context, bbClient bbspb.BuildBuddyServiceClient, invocationID string) ([]*bespb.File, error) {
723723
childInRsp, err := bbClient.GetInvocation(ctx, &inpb.GetInvocationRequest{Lookup: &inpb.InvocationLookup{InvocationId: invocationID}})
724724
if err != nil {
725-
return nil, fmt.Errorf("could not retrieve invocation %q: %s", invocationID, err)
725+
return nil, status.WrapErrorf(err, "get invocation %q", invocationID)
726726
}
727727

728728
if len(childInRsp.GetInvocation()) < 1 {
@@ -748,12 +748,12 @@ func lookupBazelInvocationOutputs(ctx context.Context, bbClient bbspb.BuildBuddy
748748
func bytestreamURIToResourceName(uri string) (*digest.CASResourceName, error) {
749749
u, err := url.Parse(uri)
750750
if err != nil {
751-
return nil, err
751+
return nil, fmt.Errorf("parse bytestream uri %q: %w", uri, err)
752752
}
753753
r := strings.TrimPrefix(u.RequestURI(), "/")
754754
rn, err := digest.ParseDownloadResourceName(r)
755755
if err != nil {
756-
return nil, err
756+
return nil, fmt.Errorf("parse bytestream resource name %q: %w", r, err)
757757
}
758758
return rn, nil
759759
}
@@ -767,46 +767,46 @@ func downloadOutputs(ctx context.Context, env environment.Env, mainOutputs []*be
767767
download := func(f *bespb.File) (string, error) {
768768
r, err := bytestreamURIToResourceName(f.GetUri())
769769
if err != nil {
770-
return "", nil
770+
return "", fmt.Errorf("resolve output uri for %q: %w", f.GetName(), err)
771771
}
772772
outFile := filepath.Join(outputBaseDir, BuildBuddyArtifactDir)
773773
for _, p := range f.GetPathPrefix() {
774774
outFile = filepath.Join(outFile, p)
775775
}
776776
outFile = filepath.Join(outFile, f.GetName())
777777
if err := downloadFile(ctx, bsClient, r, outFile); err != nil {
778-
return "", err
778+
return "", fmt.Errorf("download output %q: %w", f.GetName(), err)
779779
}
780780
return outFile, nil
781781
}
782782
for _, f := range mainOutputs {
783783
outFile, err := download(f)
784784
if err != nil {
785-
return nil, err
785+
return nil, fmt.Errorf("download main output %q: %w", f.GetName(), err)
786786
}
787787
mainLocalArtifacts = append(mainLocalArtifacts, outFile)
788788
}
789789
// Supporting outputs (i.e. runtime files) are downloaded but not displayed to the user.
790790
for _, f := range supportingOutputs {
791791
if _, err := download(f); err != nil {
792-
return nil, err
792+
return nil, fmt.Errorf("download supporting output %q: %w", f.GetName(), err)
793793
}
794794
}
795795
for _, d := range supportingDirs {
796796
rn, err := bytestreamURIToResourceName(d.GetUri())
797797
if err != nil {
798-
return nil, err
798+
return nil, fmt.Errorf("resolve supporting output dir uri %q: %w", d.GetName(), err)
799799
}
800800
tree := &repb.Tree{}
801801
if err := cachetools.GetBlobAsProto(ctx, bsClient, rn, tree); err != nil {
802-
return nil, err
802+
return nil, fmt.Errorf("download supporting output dir metadata %q: %w", d.GetName(), err)
803803
}
804804
outDir := filepath.Join(outputBaseDir, BuildBuddyArtifactDir, d.GetName())
805805
if err := os.MkdirAll(outDir, 0755); err != nil {
806-
return nil, err
806+
return nil, fmt.Errorf("create supporting output dir %q: %w", outDir, err)
807807
}
808808
if _, err := dirtools.DownloadTree(ctx, env, rn.GetInstanceName(), rn.GetDigestFunction(), tree, &dirtools.DownloadTreeOpts{RootDir: outDir}); err != nil {
809-
return nil, err
809+
return nil, fmt.Errorf("download supporting output dir %q: %w", d.GetName(), err)
810810
}
811811
}
812812

@@ -815,7 +815,7 @@ func downloadOutputs(ctx context.Context, env environment.Env, mainOutputs []*be
815815
for _, a := range mainLocalArtifacts {
816816
rp, err := filepath.Rel(outputBaseDir, a)
817817
if err != nil {
818-
return nil, err
818+
return nil, fmt.Errorf("compute relative artifact path for %q: %w", a, err)
819819
}
820820
relArtifacts = append(relArtifacts, " "+rp)
821821
}
@@ -1019,27 +1019,27 @@ func Run(ctx context.Context, opts RunOpts, repoConfig *RepoConfig) (int, error)
10191019
if childIID != "" {
10201020
conn, err := grpc_client.DialSimple(opts.Server)
10211021
if err != nil {
1022-
return 1, fmt.Errorf("could not communicate with sidecar: %s", err)
1022+
return 1, fmt.Errorf("dial sidecar: %w", err)
10231023
}
10241024
env.SetByteStreamClient(bspb.NewByteStreamClient(conn))
10251025
env.SetContentAddressableStorageClient(repb.NewContentAddressableStorageClient(conn))
10261026

10271027
mainOutputs, err := lookupBazelInvocationOutputs(ctx, bbClient, childIID)
10281028
if err != nil {
1029-
return 1, err
1029+
return 1, fmt.Errorf("lookup invocation outputs for %q: %w", childIID, err)
10301030
}
10311031
outputsBaseDir := filepath.Dir(opts.WorkspaceFilePath)
10321032
outputs, err := downloadOutputs(ctx, env, mainOutputs, runfiles, runfileDirectories, outputsBaseDir)
10331033
if err != nil {
1034-
return 1, err
1034+
return 1, fmt.Errorf("download invocation outputs for %q: %w", childIID, err)
10351035
}
10361036
if opts.RunOutputLocally {
10371037
if len(outputs) > 1 {
10381038
return 1, fmt.Errorf("run requested but target produced more than one artifact")
10391039
}
10401040
binPath := outputs[0]
10411041
if err := os.Chmod(binPath, 0755); err != nil {
1042-
return 1, fmt.Errorf("could not prepare binary %q for execution: %s", binPath, err)
1042+
return 1, fmt.Errorf("prepare binary %q for execution: %w", binPath, err)
10431043
}
10441044
execArgs := defaultRunArgs
10451045
// Pass through extra arguments (-- --foo=bar) from the command line.
@@ -1053,7 +1053,7 @@ func Run(ctx context.Context, opts RunOpts, repoConfig *RepoConfig) (int, error)
10531053
if e, ok := err.(*exec.ExitError); ok {
10541054
return e.ExitCode(), nil
10551055
} else if err != nil {
1056-
return 1, err
1056+
return 1, fmt.Errorf("run local output %q: %w", binPath, err)
10571057
}
10581058
return 0, nil
10591059
}
@@ -1071,7 +1071,7 @@ func attemptRun(ctx context.Context, bbClient bbspb.BuildBuddyServiceClient, exe
10711071

10721072
rsp, err := bbClient.Run(ctx, req)
10731073
if err != nil {
1074-
return nil, nil, err
1074+
return nil, nil, status.WrapError(err, "start remote run")
10751075
}
10761076
iid := rsp.GetInvocationId()
10771077

@@ -1109,7 +1109,7 @@ func attemptRun(ctx context.Context, bbClient bbspb.BuildBuddyServiceClient, exe
11091109
var err error
11101110
inRsp, err = bbClient.GetInvocation(ctx, &inpb.GetInvocationRequest{Lookup: &inpb.InvocationLookup{InvocationId: iid}})
11111111
if err != nil {
1112-
return fmt.Errorf("could not retrieve invocation: %w", err)
1112+
return status.WrapErrorf(err, "get invocation %q", iid)
11131113
}
11141114
if len(inRsp.GetInvocation()) == 0 {
11151115
return fmt.Errorf("invocation not found")
@@ -1147,7 +1147,7 @@ func attemptRun(ctx context.Context, bbClient bbspb.BuildBuddyServiceClient, exe
11471147

11481148
err = eg.Wait()
11491149
if err != nil {
1150-
return nil, nil, err
1150+
return nil, nil, fmt.Errorf("wait for run result: %w", err)
11511151
}
11521152

11531153
return inRsp, execRsp, nil
@@ -1161,7 +1161,7 @@ func HandleRemoteBazel(commandLineArgs []string) (int, error) {
11611161

11621162
tempDir, err := os.MkdirTemp("", "buildbuddy-cli-*")
11631163
if err != nil {
1164-
return 1, err
1164+
return 1, fmt.Errorf("create temp dir: %w", err)
11651165
}
11661166
defer func() {
11671167
os.RemoveAll(tempDir)
@@ -1241,7 +1241,7 @@ func HandleRemoteBazel(commandLineArgs []string) (int, error) {
12411241
apiKey, err = login.GetAPIKey()
12421242
if err != nil {
12431243
log.Warnf("Failed to enter login flow. Manually trigger with `bb login` or add an API key to your remote bazel run with `--remote_header=x-buildbuddy-api-key=XXX`.")
1244-
return 1, err
1244+
return 1, fmt.Errorf("get api key: %w", err)
12451245
}
12461246
}
12471247
ctx = metadata.AppendToOutgoingContext(ctx, "x-buildbuddy-api-key", apiKey)
@@ -1266,11 +1266,11 @@ func parseArgs(commandLineArgs []string) (bazelArgs []string, execArgs []string,
12661266

12671267
bazelArgs, err = login.ConfigureAPIKey(bazelArgs)
12681268
if err != nil {
1269-
return nil, nil, err
1269+
return nil, nil, fmt.Errorf("configure api key: %w", err)
12701270
}
12711271
bazelArgs, err = parser.CanonicalizeArgs(bazelArgs)
12721272
if err != nil {
1273-
return nil, nil, err
1273+
return nil, nil, fmt.Errorf("canonicalize bazel args: %w", err)
12741274
}
12751275

12761276
// Ensure all bazel remote runs use the remote cache.
@@ -1349,7 +1349,7 @@ func parseRemoteCliFlags(args []string) ([]string, error) {
13491349
unparsedArgs = unparsedArgs[1:]
13501350
}
13511351
} else {
1352-
return nil, err
1352+
return nil, fmt.Errorf("parse remote flags: %w", err)
13531353
}
13541354
}
13551355
}

0 commit comments

Comments
 (0)