Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BearerToken error handling fail #117

Open
hporchet opened this issue Nov 5, 2024 · 7 comments
Open

BearerToken error handling fail #117

hporchet opened this issue Nov 5, 2024 · 7 comments

Comments

@hporchet
Copy link

hporchet commented Nov 5, 2024

When the application of the metadata is in error, the fail method crash : apply() or fail() already called and hide the exception of the apply method.

public class BearerToken extends CallCredentials {
    ...

    @Override
    public void applyRequestMetadata(RequestInfo requestInfo, Executor executor, MetadataApplier applier) {
        executor.execute(() -> {
            try {
                Metadata headers = new Metadata();
                headers.put(META_DATA_KEY, header);
                applier.apply(headers); // crash
            } catch (Throwable e) {
                applier.fail(Status.UNAUTHENTICATED.withCause(e)); // can't be called after crashed, error : apply() or fail() already called
            }
        });
    }
}
@tstirrat15
Copy link
Contributor

@hporchet do you have a reproducible example? I'd like to write a test around this if possible.

It seems as though this could happen if there were nested/sequential withCallCredentials calls or some other piece of code that's making an apply call. Otherwise the apply seems to be just going and doing a hashmap merge, which I wouldn't expect to error in normal operation.

Where are you seeing the problem manifest?

@hporchet
Copy link
Author

I have seen this problem when i was on windows and the server return NOT_IMPLEMENTED to all grpc call. The request was correctly send but the not implemented raise a STATUS_EXCEPTION at the apply level.

@tstirrat15
Copy link
Contributor

Hmm... That makes sense. We probably shouldn't be catching all Throwables there either. Lemme see if I can narrow that sufficiently.

@hporchet
Copy link
Author

I think a simple log of the error before call fail() on debug mode seems enough, it just need to be easily find when a problem occur.

@kylelutze
Copy link

kylelutze commented Jan 13, 2025

The cause of this, at least in my situation, is because I have other dependencies that also use protobufs, specifically firebase-admin. Google hasn't updated their clients yet to use protobufs v4. See grpc/grpc-java#11015. Because of that, the authzed client was hitting the protobufs-java v3 library pulled in by firebase-admin. I made a copy of the BearerToken object and added some logging and found this

BearerToken error:
java.lang.NoSuchMethodError: 'boolean com.google.protobuf.GeneratedMessage.isStringEmpty(java.lang.Object)'
    at com.authzed.api.v1.WriteSchemaRequest.getSerializedSize(WriteSchemaRequest.java:126)

isStringEmpty exists in GeneratedMessageV3 but wasn't added in GeneratedMessage until v4.

The solution can be found on https://cloud.google.com/java/docs/bom#gradle, but the short version is to add the bom

implementation platform('com.google.cloud:libraries-bom:26.52.0')

to force Google cloud api dependencies to use protobufs v4. So far I'm not seeing any issues in firebase and the BearerToken is working again.

@linear linear bot removed the project/clients label Jan 14, 2025
@vroldanbet
Copy link
Contributor

Closing as a dependency issue unrelated to authzed-java

@tstirrat15
Copy link
Contributor

I'm gonna reopen - @kylelutze's issue was related to a dependency, but @hporchet's issue was related to something else, and the behavior of catching all throwables in this code obscures the actual issue. I'm still not entirely sure what we should be catching there and I need to do some research.

@tstirrat15 tstirrat15 reopened this Jan 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants