Making GoogleAuthException public

[julianSelser]

I'm using the google ads java lib which has this project as a dependency.

its GoogleAdsClient take these project's GoogleCredentials and when trying to authenticate, it may throw com.google.auth.oauth2.GoogleAuthException (or OAuthException)

If so, we would love to catch it like this:

try {
  UserCredentials credentials = ...; //=> GoogleCredentials

  GoogleAdsServiceClient client = GoogleAdsClient.newBuilder()
    .setCredentials(credentials)
    .setDeveloperToken(token)
    .build()
    .getLatestVersion()
    .createGoogleAdsServiceClient();

  doSomethingWith(client);
} catch(GoogleAuthException ex) {
  // handle the specific auth exception
} catch(PossiblyOtherException ex) {
 // handle possibly another exception
}

But we cant because GoogleAuthException is package private, would be nice to have it as public (along with the other exceptions that may bubble outside the package)

[julianSelser]

Hey @TimurSadykov thanks for assigning yourself! let me know if Im missing anything

[TimurSadykov]

Hi, thanks for raising the issue!

Exceptions are package private intentionally. We want to have flexibility to change them in the future without breaking anyone's code.

At the same time, we understand that there could be valid use cases for public exceptions. Could you please describe scenarios, that you think you can handle if Exceptions are public?

One more thing to keep in mind is that this library has internal retry mechanism, so if Exception is retryable - it was already retried 3 times with appropriate backoff.

A proposal: rely on interfaces. It is way easier to maintain public interfaces. You can already catch GoogleAuthException by checking Retryable interface.

How about we do the same for OAuthException? Let's define the OAuthError interface in the credentials package.

[julianSelser]

Thanks for the response Timur! We cant really catch Retryable since its not an Exception, unless you mean checking for the class or something of the sort. I fail to see how to use it otherwise.

I like the proposal if it means to extendException, also maybe instead of OAuthError should be called OAuthException since Error is on the Throwable hierarchy and already has semantics, but then we are back at the beggining having a public OAuthException.

My intention was to convince you of making the exceptions public but I wasnt sure myself tbh, and Im probably not smart enough to make the point anyways, so I asked chat GPT and the results were interesting:

Ok cool, so it depends, but what about this specific case?

[TimurSadykov]

@julianSelser Yes, more specifically I meant catching IOExceptions and checking for instances of Retryable or other interfaces. This makes more sense also because IOException is already a historical de-facto standard exception type and if you want to catch all auth exceptions, you have to catch IOExceptions anyways.

The name OAuthException works better than OAuth2Error, but there is already an exception called OAuthException and java does not allow interface and exception with the same name. So we need something else, ideas are welcome :)

[julianSelser]

@TimurSadykov I'd rather have the exceptions public instead of forcing users to do an instanceof (and maybe a cast) but I guess its better than not being able to catch it.

Is it ok if I redo my PR to accomodate the interface? Im wondering if we can have some reason() method we can have for the user to know what happened, seems like should also implement Retryable, what do you think?

[TimurSadykov]

@julianSelser Ideally, we want to follow the OAuth2 spec for errors. It has a field called errorDescription which is similar to what you are describing?

I would suggest following the OAuthException implementation as a base for you interface. It should have what you need. Then we can make the existing OAuthException to extend the new interface. It already extends the Retryable via GoogleAuthException.

Then we can throw the updated exception from our credential classes.

Let me know if any questions.