-
Notifications
You must be signed in to change notification settings - Fork 176
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
Infinite loop if refresh token is invalid #169
Comments
@devchas please try to reproduce this issue. Check if the OAuth library allows us to detect an invalid refresh token. If so implement a check in the builder which rejects the refresh token with a new exception called LibrarySetupException which contains an enum of reasons. |
Apologies for the delayed response here. I was able to reproduce the issue. Interestingly, the issue only exists for search requests. Mutate and searchStream request exhibit normal behavior. I've debugged fairly extensively and believe there is an issue with either the Gax or OAuth2 libraries because the call to I've opened an issue with the gax-java repo to see if they have any insight. I do think it's specific to the Java implementation of Gax because I tried to replicate the issue with the client library for Python, but it returned an error as expected without any infinite loop. @nwbirnie any thoughts here? |
Pinged on the bug, I agree this looks like a bug in GAX. There is a workaround possible here which is to switch the RPC from paging -> streaming (i.e. call |
Looks like we're tracking this in the upcoming microgenerator migration. |
I have come here because I have seen this issue (with OAuth token) before; but also -- unsure if it is related -- I notice an issue with requests hanging when they result in an error sometimes. For instance, unspecified LoginCustomerId when it needed to be specified resulted in the program hanging. I, unfortunately, do not have a pithy case to reproduce at the moment but has there been any more information here? |
Hello,
and I checked logs, the retries are repeating endlessly. |
I can confirm that this looks like a misbehavior of the retries. We appear to infinitely retry failed OAuth requests. I'm guessing that we have the wrong settings for retries on OAuth errors. Will reach out internally to investigate and get back to you. |
Hey guys, |
Hey thanks for the ping. The underlying issue is being investigated here, I've added some more debugging info an reopened since I'm pretty sure that's the root cause. |
Yes, this is currently being worked on, was picked up a couple weeks ago. We should have an update here soon. |
I am also waiting for a solution to this problem, I am pending for when they solve it |
Can do this:
|
I like this as a temporary workaround until we get a fix.
…On Tue, Aug 24, 2021, 11:10 PM Xiang Sun ***@***.***> wrote:
Can do this:
`UserCredentials credentials = UserCredentials.newBuilder()
.setClientId(clientId)
.setClientSecret(clientSecret)
.setRefreshToken(refreshToken)
.build();
AccessToken accessToken = credentials.refreshAccessToken();
return GoogleAdsClient.newBuilder()
.setCredentials(credentials.toBuilder().setAccessToken(accessToken).build())
.setDeveloperToken(developerToken)
.setLoginCustomerId(loginCustomerId)
.build();`
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#169 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABCOJCGKKNJSHBE6W3OD66LT6RNKZANCNFSM4ILIFPWQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email>
.
|
Just updating this thread that the root cause seems to be caused by this issue. It looks like a design was in progress as of September 2021. |
Current status is that the addition of the Internal ref: b/231441733 |
you can manually shutdown after certain time
|
I had the same situation, cancel the multiple threads didn't work。
|
Have you tried explicitly calling
|
Thank you for your answer. I have not tried this method, my problem is because I deliberately modified the token to be wrong, because the token is not permanent, so I want to test this situation. Now that I've solved it, I still have to use google client's own shutdown. |
Still no fix for this? |
This needs a fix in grpc-java for GRPC will keep retrying (until RPC timeout) if a user uses a bad service account key · Issue #6808 · grpc/grpc-java. A suboptimal workaround would be to remove
you could do:
However, this is suboptimal because there will be occasional |
That grpc-java change looks like it is in 1.63 which was pushed to maven central Apr 4. Edit: I tested it and found no change in behavior. In fact, although grpc/grpc-java#6808 was marked closed for the 1.63 milestone, the commit was reverted (but only on the v1.63.x branch, it's still in master). So, one of the workarounds described above is still required. |
@jacob-tolar-bridg , thanks for highlighting and for testing. I'm following up with my gRPC contacts on this. -Josh, Google Ads API Team |
We align the google-ads-java dependencies with those of the libraries-bom project for interoperability between this library and other Google Cloud libraries. The latest release of the libraries-bom project depends on grpc 1.62.2, but I'm checking periodically for updates. Thanks, |
Finally it is fixed. Updating grpc-java dependencies to 1.64.0 worked for me See here: grpc/grpc-java#6808 (comment) |
Thanks for confirming. I'm still waiting for libraries-bom to pick up this change, per my previous comment. |
@jradcliff |
Good news! Version 33.0.0, released earlier today, includes an upgrade to gRPC |
Hi there,
If I use an invalid refresh token, it goes into an infinite loop, not throwing an exception.
It keeps trying requesting itself again and again..
This is so frustrating as refresh token can be broken many reasons(Change password, etc)
If there are some options which can raise an exception, it would be much more easy to handle these situations.
Please let me know your opinion or guide me!
The text was updated successfully, but these errors were encountered: