Uh oh!
There was an error while loading. Please reload this page.
connection: refetch token if invalid. - #241
Conversation
ryanseys
commented
Sep 27, 2014
So we attempted to do a similar thing in the google-apis-nodejs-client library and always had issues reattaching the media body.We ended up removing refresh functionality on 401 and instead will refresh the token when the expiry_date in the token has past or there's no access_token available but there is a refresh_token available. We do this before the request is attempted for the first time. Not sure how we can support this and piping from streams too. We would have to cache the pipe data to replay it, which is just non-sensical. |
stephenplusplus
commented
Sep 29, 2014
We also do the same validation against our local token, but in the event the token passes our tests, but is somehow deemed invalid upstream, this will force a token re-fetch. The only reasons I can think of that it could be invalid upstream but not locally:
|
stephenplusplus
commented
Sep 29, 2014
Notes: add a max retry limit of 1 |
ryanseys
commented
Sep 29, 2014
Various closed issues filed from the other API client project on a similar effect of a change like this: googleapis/google-api-nodejs-client#139 My "fix" provided to remove refresh functionality: googleapis/google-api-nodejs-client#235 Currently open issues related to this refresh functionality: |
stephenplusplus
commented
Sep 30, 2014
👍 thanks. In our case, we're only using streams in In the case of the stream problem, to solve this just use Duplexify: https://github.com/GoogleCloudPlatform/gcloud-node/blob/master/lib/storage/index.js#L421 - we're using it for the same reason here, which is to control manually when a stream is readable and writable, usually after something asynchronous occurs - for us, after we have a token. |
ryanseys
commented
Sep 30, 2014
Hmm seems reasonable. I say 🚢 it and see what happens. Might actually work out better than expected. Would be nice to port some of this over to the other library too instead of using the poorly implemented sandwich stream. |
stephenplusplus
commented
Sep 30, 2014
Fixing up some last features, adding some tests, and then I will ping for a quick look :) |
6e5a560 to
a8f31ffComparestephenplusplus
commented
Sep 30, 2014
Updated! |
ryanseys
commented
Sep 30, 2014
Can we make MAX_ATTEMPTS not in terms of the number of times to retry an API request, but in terms of the number of times we will attempt to refresh the token given a 401 Unauthorized error (because that's more specifically what we are doing). In this case, MAX_REFRESH_ATTEMPTS = 1 would be the equivalent with a little tweak to the increment & check logic. |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
a8f31ff to
6ff71aeComparestephenplusplus
commented
Sep 30, 2014
Updated with renamed attempt stuff. |
d9115ec to
2b1246eCompare
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
2b1246e to
c2408beCompare
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
c2408be to
797ca1bComparesilvolu
commented
Sep 30, 2014
LGTM, @ryanseys ? |
ryanseys
commented
Sep 30, 2014
Yep LGTM. |
connection: refetch token if invalid.
This PR was generated using Autosynth. 🌈 Synth log will be available here: https://source.cloud.google.com/results/invocations/933ad2fd-a72d-472d-91bb-4b474e01ed77/targets - [ ] To automatically regenerate this PR, check this box. Source-Link: googleapis/synthtool@ba9918c
This PR was generated using Autosynth. 🌈 Synth log will be available here: https://source.cloud.google.com/results/invocations/9f6a8ee0-c33a-40f0-b261-b8a2837448ab/targets - [ ] To automatically regenerate this PR, check this box. (May take up to 24 hours.) Source-Link: googleapis/synthtool@c6706ee Source-Link: googleapis/synthtool@b33b0e2 Source-Link: googleapis/synthtool@898b38a
🤖 I have created a release \*beep\* \*boop\* --- ### [7.1.1](https://www.github.com/googleapis/teeny-request/compare/v7.1.0...v7.1.1) (2021-06-30) ### Bug Fixes * throw error if missing uri or url ([#239](https://www.github.com/googleapis/teeny-request/issues/239)) ([4d770e3](https://www.github.com/googleapis/teeny-request/commit/4d770e3b89254c4cec30c422cdcdad257500c9cc)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
https://developers.google.com/drive/web/handle-errors#401_invalid_credentials
If the API returns a 401, this will nullify the token we have and fetch a new one before trying the request again.