Skip to content

RequestMetadataCallback.onFailure() should provide a type-safe method to retrieve the root cause of the failure #626

@nwbirnie

Description

@nwbirnie

This caused issue: googleapis/gax-java#965 which is being investigated by grpc team here: grpc/grpc-java#6808

Description

The following is what happens.

  1. If refresh token is invalid, then auth lib will throw an HttpResponseException like the following

    com.google.api.client.http.HttpResponseException: 400 Bad Request
    POST https://oauth2.googleapis.com/token
    {
      "error": "invalid_request",
      "error_description": "Could not determine client ID from request."
    }
    
  2. auth lib then passes the exception to a callback

    protected final void blockingGetToCallback(URI uri, RequestMetadataCallback callback) {
        Map<String, List<String>result;
        try {
          result = getRequestMetadata(uri);
        } catch (Throwable e) {
          callback.onFailure(e);   <========= here
          return;
        }
        callback.onSuccess(result);
    }
    
  3. callback is handled by the following code in grpc-auth GoogleAuthLibraryCallCredentials.java

    public void onFailure(Throwable e) {
        if (e instanceof IOException) {         <=============== here
          // Since it's an I/O failure, let the call be retried with UNAVAILABLE.
          applier.fail(Status.UNAVAILABLE
              .withDescription("Credentials failed to obtain metadata")
              .withCause(e));
        } else {
          applier.fail(Status.UNAUTHENTICATED
              .withDescription("Failed computing credential metadata")
              .withCause(e));
        }
    }
    

Note that HttpResponseException is subclass of IOException, but we shouldn't retry on HttpResponseException. So the correct implementation would be:

public void onFailure(Throwable e) {
        if ((e instanceof IOException) && !(e instanceof HttpResponseException)) {         <=============== here
          // Since it's an I/O failure, let the call be retried with UNAVAILABLE.
          applier.fail(Status.UNAVAILABLE
              .withDescription("Credentials failed to obtain metadata")
              .withCause(e));
        } else {
          applier.fail(Status.UNAUTHENTICATED
              .withDescription("Failed computing credential metadata")
              .withCause(e));
        }
      }

Describe the solution you'd like
HttpResponseException is an implementation detail of google-auth-library-java. We'd like a typed exception returned here with properties for the error context.

public class AuthenticationException {
  public AuthErrorCode getError() {
    // returns AuthErrorCode.INVALID_REQUEST for the example above.
  }

  public String getErrorDescription() {
    // returns "Could not determine client ID from request." for the example above.
  }
}

public enum AuthErrorCode {
  UNAVAILABLE,
  UNAUTHENTICATED,
  INVALID_REQUEST
  ...
}

Describe alternatives you've considered
gRPC could modify the exception handling to check if the exception is a HttpResponseException. However, this is an implementation detail of google-auth-library-java and potentially not a reliable indicator that the request has failed permanently.

Metadata

Metadata

Assignees

Labels

priority: p3Desirable enhancement or fix. May not be included in next release.type: bugError or flaw in code with unintended results or allowing sub-optimal usage patterns.

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions