-
Notifications
You must be signed in to change notification settings - Fork 19
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
http2 GOAWAY not working (error in popsicle as used by client-oauth2 package) #134
Comments
Thanks for the report! Just for a sanity check, can you tell me the versions you’re using? It should be possible with |
Here you go...
|
I also started seeing this systematically after an upgrade from Node 12.16.1 to 12.18.0, with these versions:
I was going to try forcing http1.1 (which can be done when initializing the http transport via the |
not to +1 on this but I ran into this as well, spent a few hours on it today. I am using |
I haven't been able to replicate Found that it's likely related to nodejs/node#32958 (comment). I have a patch locally to workaround this, but it doesn't sound like what this thread run into specifically. |
I've published https://github.com/serviejs/popsicle-transport-http/releases/tag/v1.0.8 which you should get if you reinstall. I'm not sure that it fixes this issue, but may fix an issue with node.js ^12.17 since it looks like a flag was missed in the back port from the latest version. |
Thanks @blakeembrey. I wasn't expecting such a quick response, so I implemented a workaround (using a different request library.- client-oath2 supports using a custom request function. It's kind of a pain to test this (as it was only happening in production), so I might not get to it for a few days now. |
@d1manson @blakeembrey I have a local repro (you can force by using |
I can confirm it's fixed now, @blakeembrey you are my hero today. Since I'm using the client-oauth2 library I forced the version using the following resolution in my package.json
thanks for the incredibly quick resolution on this one. |
Same here, thanks! |
@Metroninja would you mind sharing the repro? Would love to add a test covering it, although catching it on the specific version when it regressed in node.js is a different story. |
To repro you need to be using the latest client-oauth2 package, on node 12.18 and on an oauth return call (aka after a successful auth) when extracting the token. specifically the following method AKA it's not a simple repro case to add test coverage for, at least in my use case. That said you can probably extract what they are doing here https://github.com/mulesoft/js-client-oauth2/blob/master/src/client-oauth2.js#L414 for your purposes |
Closing the issue since it's been resolved, but I'll try and revisit better tests around the HTTP2 connections. |
Hi, for anybody using this library, make sure you create a new instance of ClientOAuth2 every time you are connecting to the OAuth server, even with the aforementioned update to popsicle-transport-http. In my case, I was passing an instance of it in via my constructor to make unit testing easier. The issue is that HTTP/2 sessions are being maintained during the life of the ClientOAuth2 object, so you can get the GOAWAY stream problem in a long running process. |
I'm using
client-oauth2
which in turn usespopsicle
. In particular I tend to make requests to a server once or twice every few hours. For a week or so this seemed to work ok, but now I am getting the below error. Googling was unusually unhelpful on this, but I would have thought this would have been handled within node rather than by popsicle (I am usingNode.js 12 running on 64bit Amazon Linux 2, elastic beanstalk
).Any thoughts?
popsicle
is a dependency ofclient-oauth2
, as shown here - https://github.com/mulesoft/js-client-oauth2/blob/2752e8b3161ed06a6fddb5a6d042ec6a190929b0/package.json#L66.Not sure how useful it will be, but for refernce I was talking to Xero's api, see - https://developer.xero.com/.
In an attempt to work around this, I am simply going to add one retry when the relevant request fails (not sure how popsicle works under the hood but maybe after the error has been caught the http2 connection will be reset or something...?!).
The text was updated successfully, but these errors were encountered: