From a18236cf031ec3b9a24cd6eb0281119d75474ee9 Mon Sep 17 00:00:00 2001 From: rahul2393 Date: Fri, 3 Dec 2021 02:57:05 +0530 Subject: [PATCH] refactor(spanner): return APIError as wrapped error in spanner.Error struct (#5169) * refactor(spanner): return APIError as wrapped error in spanner.Error struct Co-authored-by: Rahul Yadav Co-authored-by: Hengfeng Li --- spanner/errors.go | 33 ++++++++++++++++++++++++++------- spanner/errors_test.go | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 7 deletions(-) diff --git a/spanner/errors.go b/spanner/errors.go index 98de741662c..f4451812765 100644 --- a/spanner/errors.go +++ b/spanner/errors.go @@ -20,12 +20,22 @@ import ( "context" "fmt" + "github.com/googleapis/gax-go/v2/apierror" "google.golang.org/genproto/googleapis/rpc/errdetails" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" ) // Error is the structured error returned by Cloud Spanner client. +// +// Deprecated: Unwrap any error that is returned by the Spanner client as an APIError +// to access the error details. Do not try to convert the error to the +// spanner.Error struct, as that struct may be removed in a future release. +// +// Example: +// var apiErr *apierror.APIError +// _, err := spanner.NewClient(context.Background()) +// errors.As(err, &apiErr) type Error struct { // Code is the canonical error code for describing the nature of a // particular error. @@ -103,11 +113,11 @@ func (e *Error) decorate(info string) { e.Desc = fmt.Sprintf("%v, %v", info, e.Desc) } -// spannerErrorf generates a *spanner.Error with the given description and a -// status error with the given error code as its wrapped error. +// spannerErrorf generates a *spanner.Error with the given description and an +// APIError error having given error code as its status. func spannerErrorf(code codes.Code, format string, args ...interface{}) error { msg := fmt.Sprintf(format, args...) - wrapped := status.Error(code, msg) + wrapped, _ := apierror.FromError(status.Error(code, msg)) return &Error{ Code: code, err: wrapped, @@ -119,7 +129,7 @@ func spannerErrorf(code codes.Code, format string, args ...interface{}) error { // error is already a *spanner.Error, the original error will be returned. // // Spanner Errors are normally created by the Spanner client library from the -// returned status of a RPC. This method can also be used to create Spanner +// returned APIError of a RPC. This method can also be used to create Spanner // errors for use in tests. The recommended way to create test errors is // calling this method with a status error, e.g. // ToSpannerError(status.New(codes.NotFound, "Table not found").Err()) @@ -127,6 +137,15 @@ func ToSpannerError(err error) error { return toSpannerErrorWithCommitInfo(err, false) } +// toAPIError converts a general Go error to *gax-go.APIError. If the given +// error is not convertible to *gax-go.APIError, the original error will be returned. +func toAPIError(err error) error { + if apiError, ok := apierror.FromError(err); ok { + return apiError + } + return err +} + // toSpannerErrorWithCommitInfo converts general Go error to *spanner.Error // with additional information if the error occurred during a Commit request. // @@ -147,9 +166,9 @@ func toSpannerErrorWithCommitInfo(err error, errorDuringCommit bool) error { desc = fmt.Sprintf("%s, %s", desc, transactionOutcomeUnknownMsg) wrapped = &TransactionOutcomeUnknownError{err: wrapped} } - return &Error{status.FromContextError(err).Code(), wrapped, desc, ""} + return &Error{status.FromContextError(err).Code(), toAPIError(wrapped), desc, ""} case status.Code(err) == codes.Unknown: - return &Error{codes.Unknown, err, err.Error(), ""} + return &Error{codes.Unknown, toAPIError(err), err.Error(), ""} default: statusErr := status.Convert(err) code, desc := statusErr.Code(), statusErr.Message() @@ -158,7 +177,7 @@ func toSpannerErrorWithCommitInfo(err error, errorDuringCommit bool) error { desc = fmt.Sprintf("%s, %s", desc, transactionOutcomeUnknownMsg) wrapped = &TransactionOutcomeUnknownError{err: wrapped} } - return &Error{code, wrapped, desc, ""} + return &Error{code, toAPIError(wrapped), desc, ""} } } diff --git a/spanner/errors_test.go b/spanner/errors_test.go index 42d95aaa98f..9377a7d257a 100644 --- a/spanner/errors_test.go +++ b/spanner/errors_test.go @@ -22,6 +22,7 @@ import ( "strings" "testing" + "github.com/googleapis/gax-go/v2/apierror" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" ) @@ -101,3 +102,35 @@ func TestToSpannerError(t *testing.T) { } } } + +func TestAPIErrorBackwardCompatibility(t *testing.T) { + apiError, _ := apierror.FromError(status.Error(codes.InvalidArgument, "bad")) + testCases := []struct { + name string + clientError error + want *apierror.APIError + }{ + { + name: "when client returns spanner.Error", + clientError: spannerErrorf(codes.InvalidArgument, "bad"), + want: apiError, + }, + { + name: "when client returns apierror.APIError", + clientError: apiError, + want: apiError, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + var got *apierror.APIError + ok := errorAs(tc.clientError, &got) + if !ok { + t.Error("error unwrapping spanner.Error to apierror.APIError") + } + if got.Error() != tc.want.Error() { + t.Errorf("Wrapped error mismatch\nGot: %v\nWant: %v", got, tc.want) + } + }) + } +}