Skip to content

Commit

Permalink
fix: Retry gRPC UNAUTHENTICATED errors like 401s. (#2781)
Browse files Browse the repository at this point in the history
HTTP 401s are retried transparently by GCSFuse, because we may have a
delay between when we fetch an OAuth token and when it is validated in
GCS. This applies equivalently to the gRPC error space.
  • Loading branch information
cjc25 authored Jan 8, 2025
1 parent 247da65 commit bc3d55a
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 0 deletions.
13 changes: 13 additions & 0 deletions internal/storage/storageutil/custom_retry.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import (
"cloud.google.com/go/storage"
"github.com/googlecloudplatform/gcsfuse/v2/internal/logger"
"google.golang.org/api/googleapi"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
)

func ShouldRetry(err error) (b bool) {
Expand All @@ -41,5 +43,16 @@ func ShouldRetry(err error) (b bool) {
return
}
}

// This is the same case as above, but for gRPC UNAUTHENTICATED errors. See
// https://github.com/golang/oauth2/issues/623
// TODO: Please incorporate the correct fix post resolution of the above issue.
if status, ok := status.FromError(err); ok {
if status.Code() == codes.Unauthenticated {
b = true
logger.Infof("Retrying for UNAUTHENTICATED error: %v", err)
return
}
}
return
}
28 changes: 28 additions & 0 deletions internal/storage/storageutil/custom_retry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ import (

"github.com/stretchr/testify/assert"
"google.golang.org/api/googleapi"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
)

func TestShouldRetryReturnsTrueWithGoogleApiError(t *testing.T) {
Expand Down Expand Up @@ -118,3 +120,29 @@ func TestShouldRetryReturnsTrueForConnectionRefusedAndResetErrors(t *testing.T)
})
}
}

func TestShouldRetryReturnsTrueForUnauthenticatedGrpcErrors(t *testing.T) {
testCases := []struct {
name string
err error
expectedResult bool
}{
{
name: "UNAUTHENTICATED",
err: status.Error(codes.Unauthenticated, "Request had invalid authentication credentials. Expected OAuth 2 access token, login cookie or other valid authentication credential. See https://developers.google.com/identity/sign-in/web/devconsole-project."),
expectedResult: true,
},
{
name: "PERMISSION_DENIED",
err: status.Error(codes.PermissionDenied, "unauthorized"),
expectedResult: false,
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
actualResult := ShouldRetry(tc.err)
assert.Equal(t, tc.expectedResult, actualResult)
})
}
}

0 comments on commit bc3d55a

Please sign in to comment.