Skip to content
This repository was archived by the owner on May 12, 2026. It is now read-only.
This repository was archived by the owner on May 12, 2026. It is now read-only.

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

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.

Activity

  1. added
    priority: p2Moderately-important priority. Fix may not be included in next release.
    type: bugError or flaw in code with unintended results or allowing sub-optimal usage patterns.
    on Apr 7, 2021
  2. nwbirnie commented on Apr 8, 2021

    @nwbirnie
    Author

    Hey @Neenu1995 just wondering if you could advise whether this is feasible and if so a timeline please? This is causing issues for ads API users and we'd like to make sure that it gets closed out.

  3. Neenu1995 commented on Apr 9, 2021

    @Neenu1995
    Contributor

    Hi,
    The Auth team is currently looking into the issue. This looks like a specific example of a larger problem, which is that errors that might happen during auth flows are not reported with a precision that allows to distinguish errors that are retryable (e.g. server momentarily overloaded) vs error that should not be retried (e.g. the credentials are wrong, and the request should not be retried without a change in the credentials).
    They can start after the next week, since they are OOO next week. A more clear timeline can be provided then.

  4. nwbirnie commented on Apr 20, 2021

    @nwbirnie
    Author

    Friendly ping for @Neenu1995 :)

  5. added
    🚨This issue needs some love.
    and removed
    🚨This issue needs some love.
    on Jul 20, 2021
  6. self-assigned this
    on Jul 23, 2021
  7. TimurSadykov commented on Jul 23, 2021

    @TimurSadykov

    @nwbirnie design in progress :)

  8. nwbirnie commented on Jul 27, 2021

    @nwbirnie
    Author

    That's awesome, thankyou @TimurSadykov !

  9. szleoxu commented on Aug 23, 2021

    @szleoxu
  10. added
    priority: p3Desirable enhancement or fix. May not be included in next release.
    and removed
    priority: p2Moderately-important priority. Fix may not be included in next release.
    on Oct 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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