Skip to content

Commit 12757c8

Browse files
committed
Fix error swallowing behavior
1 parent d7db764 commit 12757c8

File tree

2 files changed

+36
-4
lines changed

2 files changed

+36
-4
lines changed

internal/cmd/backup.go

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,20 @@ const (
4242
// cobraRunEFunc is the signature of a cobra.Command.RunE function.
4343
type cobraRunEFunc = func(cmd *cobra.Command, args []string) (err error)
4444

45+
// TODO: apply these to other cmds as well
46+
var errorHandlers = []func(error) error{
47+
addPermissionDeniedErrInfo,
48+
addSizeErrInfo,
49+
}
50+
4551
// withErrorHandling is a wrapper that centralizes error handling, instead of having to scatter it around the command logic.
4652
func withErrorHandling(f cobraRunEFunc) cobraRunEFunc {
4753
return func(cmd *cobra.Command, args []string) (err error) {
48-
return addSizeErrInfo(f(cmd, args))
54+
cmdErr := f(cmd, args)
55+
for _, handler := range errorHandlers {
56+
cmdErr = handler(cmdErr)
57+
}
58+
return cmdErr
4959
}
5060
}
5161

@@ -398,9 +408,15 @@ func backupCreateCmdFunc(cmd *cobra.Command, args []string) (err error) {
398408
}
399409
return nil
400410
})
411+
if err != nil {
412+
return err
413+
}
401414

402415
backupCompleted = true
403-
return nil
416+
// NOTE: we return err here because there's cleanup being done
417+
// in the `defer` blocks that will modify the `err` if cleanup
418+
// fails
419+
return err
404420
}
405421

406422
func takeBackup(ctx context.Context, spiceClient client.Client, req *v1.ExportBulkRelationshipsRequest, processResponse func(*v1.ExportBulkRelationshipsResponse) error) error {
@@ -902,3 +918,15 @@ func addSizeErrInfo(err error) error {
902918

903919
return fmt.Errorf("%w: set flag --max-message-size=%d to increase the maximum allowable size", err, 2*necessaryByteCount)
904920
}
921+
922+
func addPermissionDeniedErrInfo(err error) error {
923+
if err == nil {
924+
return nil
925+
}
926+
927+
code := status.Code(err)
928+
if code != codes.PermissionDenied {
929+
return err
930+
}
931+
return fmt.Errorf("%w: ensure that the token used for this call has all requisite permissions", err)
932+
}

internal/cmd/backup_test.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,9 @@ func TestBackupCreateCmdFunc(t *testing.T) {
282282
ctx := t.Context()
283283
srv := zedtesting.NewTestServer(ctx, t)
284284
go func() {
285-
require.NoError(t, srv.Run(ctx))
285+
// NOTE: we don't assert anything about the error
286+
// here because there isn't a good time or place to do so.
287+
_ = srv.Run(ctx)
286288
}()
287289
conn, err := srv.GRPCDialContext(ctx)
288290
require.NoError(t, err)
@@ -519,7 +521,9 @@ func TestBackupRestoreCmdFunc(t *testing.T) {
519521
ctx := t.Context()
520522
srv := zedtesting.NewTestServer(ctx, t)
521523
go func() {
522-
require.NoError(t, srv.Run(ctx))
524+
// NOTE: we don't assert anything about the error
525+
// here because there isn't a good time or place to do so.
526+
_ = srv.Run(ctx)
523527
}()
524528
conn, err := srv.GRPCDialContext(ctx)
525529
require.NoError(t, err)

0 commit comments

Comments
 (0)