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.

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:

• We have a mismatch in expiration
• The token expires between the time we make the request and the time the API gets it

Notes: add a max retry limit of 1

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
googleapis/google-api-nodejs-client#122
googleapis/google-api-nodejs-client#99
googleapis/google-api-nodejs-client#98
googleapis/google-api-nodejs-client#81

My "fix" provided to remove refresh functionality:

googleapis/google-api-nodejs-client#235

Currently open issues related to this refresh functionality:
googleapis/google-api-nodejs-client#260 (cannot stream + refresh)
googleapis/google-api-nodejs-client#261 (wants to bring refresh back)

👍 thanks.

In our case, we're only using streams in Storage.getWritableStream_ (edit: and createReadStream), which circumvents the Connection.req method, and instead creates its own authorized request (with Connection.createAuthorizedReq) and passes it right to Connection.requester (which is the request node module).

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.

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.

Fixing up some last features, adding some tests, and then I will ping for a quick look :)

Updated!

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.

Updated with renamed attempt stuff.

LGTM, @ryanseys ?

Yep LGTM.