fix(v2/apierror): fix (*APIError).Error() for unwrapped Status (#351)

fixes: #350
googleapis · Jun 3, 2024 · 22c16e7 · 22c16e7
1 parent 99b8660
commit 22c16e7
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 12 deletions.
diff --git a/v2/apierror/apierror.go b/v2/apierror/apierror.go
@@ -206,8 +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 {
+ } else if a.status != nil && a.err != nil {
  msg = a.err.Error()
+ } else if a.status != nil {
+ 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
@@ -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 {