From ac7d761477fc55b09f758df4fe9dae267ad84864 Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Fri, 31 May 2024 12:56:59 -0600 Subject: [PATCH 1/2] fix(v2/apierror): fix (*APIError).Error() for unwrapped Status fixes: #350 --- v2/apierror/apierror.go | 6 ++++- v2/apierror/apierror_test.go | 48 +++++++++++++++++++++++++++--------- 2 files changed, 42 insertions(+), 12 deletions(-) diff --git a/v2/apierror/apierror.go b/v2/apierror/apierror.go index d785a065..d712fbf9 100644 --- a/v2/apierror/apierror.go +++ b/v2/apierror/apierror.go @@ -207,7 +207,11 @@ func (a *APIError) Error() string { // an ugly way. msg = fmt.Sprintf("googleapi: Error %d: %s", a.httpErr.Code, a.httpErr.Message) } else if a.status != nil { - msg = a.err.Error() + if a.err != nil { + msg = a.err.Error() + } else { + msg = a.status.Message() + } } return strings.TrimSpace(fmt.Sprintf("%s\n%s", msg, a.details)) } diff --git a/v2/apierror/apierror_test.go b/v2/apierror/apierror_test.go index c3f28dc1..b21c206f 100644 --- a/v2/apierror/apierror_test.go +++ b/v2/apierror/apierror_test.go @@ -465,30 +465,56 @@ func TestParseError(t *testing.T) { se := errors.New("standard error") tests := []struct { - source error - apierr *APIError - b bool + name string + source error + wantErr *APIError + wantParsed bool }{ - {hae, &APIError{httpErr: hae, status: haeS, details: ErrDetails{ErrorInfo: httpErrInfo}}, true}, - {se, &APIError{err: se}, false}, + { + name: "with http ErrorInfo details", + source: hae, + wantErr: &APIError{httpErr: hae, status: haeS, details: ErrDetails{ErrorInfo: httpErrInfo}}, + wantParsed: true, + }, + { + name: "standard error no details", + source: se, + wantErr: &APIError{err: se}, + wantParsed: false, + }, + { + name: "grpc status no details", + source: haeS.Err(), + wantErr: &APIError{httpErr: hae, status: haeS, details: ErrDetails{}}, + wantParsed: true, + }, } for _, tc := range tests { // ParseError with wrap = true is covered by TestFromError, above. - got, apiB := ParseError(tc.source, false) - if tc.b != apiB { - t.Errorf("ParseError(%s, false): got %v, want %v", tc.apierr, apiB, tc.b) + got, gotParsed := ParseError(tc.source, false) + if tc.wantParsed != gotParsed { + t.Errorf("ParseError(%s, false): got %v, want %v", tc.wantErr, gotParsed, tc.wantParsed) } - if tc.b { - if diff := cmp.Diff(got.details, tc.apierr.details, cmp.Comparer(proto.Equal)); diff != "" { + if got != nil { + if got.Error() == "" { + t.Errorf("got.Error(): got %q, want non-empty", got.Error()) + } + } + if tc.wantParsed { + if diff := cmp.Diff(got.details, tc.wantErr.details, cmp.Comparer(proto.Equal)); diff != "" { t.Errorf("got(-), want(+),: \n%s", diff) } - if diff := cmp.Diff(got.status, tc.apierr.status, cmp.Comparer(proto.Equal), cmp.AllowUnexported(status.Status{})); diff != "" { + if diff := cmp.Diff(got.status, tc.wantErr.status, cmp.Comparer(proto.Equal), cmp.AllowUnexported(status.Status{})); diff != "" { t.Errorf("got(-), want(+),: \n%s", diff) } if got.err != nil { t.Errorf("got %s, want nil", got.err) } + } else { + if got != nil { + t.Errorf("got %s, want nil", got) + } } } if err, _ := ParseError(nil, false); err != nil { From 75ae2030123102af66f20ef16b0d27523a9bb028 Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Mon, 3 Jun 2024 10:48:07 -0600 Subject: [PATCH 2/2] raise nested if/else --- v2/apierror/apierror.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/v2/apierror/apierror.go b/v2/apierror/apierror.go index d712fbf9..7de60773 100644 --- a/v2/apierror/apierror.go +++ b/v2/apierror/apierror.go @@ -206,12 +206,10 @@ func (a *APIError) Error() string { // Truncate the googleapi.Error message because it dumps the Details in // an ugly way. msg = fmt.Sprintf("googleapi: Error %d: %s", a.httpErr.Code, a.httpErr.Message) + } else if a.status != nil && a.err != nil { + msg = a.err.Error() } else if a.status != nil { - if a.err != nil { - msg = a.err.Error() - } else { - msg = a.status.Message() - } + msg = a.status.Message() } return strings.TrimSpace(fmt.Sprintf("%s\n%s", msg, a.details)) }