ciaranj / node-oauth

OAuth wrapper for node.js
MIT License
2.44k stars 660 forks source link

fix: prevent double callback in _executeRequest #363

Closed episage closed 1 year ago

episage commented 2 years ago

www.googleapis.com does ECONNRESET just after data is received in passBackControl

This prevents the callback from being called twice.

8bitDesigner commented 2 years ago

I just lost two days of work troubleshooting this bug. I hate to be the 👍 guy, but it would be great to get this merged.

shayneczyzewski commented 1 year ago

Hitting this issue (just on server startup and first request) myself. Any ides on if/when this may be merged? Thanks!

MithileshHinge commented 1 year ago

+1 Lost 2 working days right before demo day because of this issue. Please merge this PR, thank you.

konotorii commented 1 year ago

+1 behind schedule with no way of fixing it, would be greatly appreciated if merged, thanks.

episage commented 1 year ago

Guys, this is open source software. Nobody gets paid to merge this. If you want to use the fix just install from my branch:

npm install https://github.com/episage/node-oauth.git#db8088793f6a6ce27967068d661c9810cfc076e0

I promise not to delete this code in the next 100 years.

ciaranj commented 1 year ago

I will try to merge this, this weekend. I’ve not reviewed the fix yet tbh.

On Fri, 22 Jul 2022 at 8:39 pm, Tom Ciborski @.***> wrote:

Guys, this is open source software. Nobody gets paid to merge this.

If you want to use the fix just install from my branch:

npm install https://github.com/episage/node-oauth.git#db8088793f6a6ce27967068d661c9810cfc076e0

I promise not to delete this code in the next 100 years.

— Reply to this email directly, view it on GitHub https://github.com/ciaranj/node-oauth/pull/363#issuecomment-1192881426, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAANQSDZXOQI2W5HBLPAKP3VVL2F5ANCNFSM5ID3CYDA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

shayneczyzewski commented 1 year ago

Thank you very much @ciaranj, we really appreciate it!

The problem is a bit more nuanced than that, @episage. I fully agree the burden of package managers is unpaid, thankless and unending. So again, thank you Ciaran! However, this package is a 3rd level dependency for the Google Passport package (and probably others). Therefore, it is not as simple as "fixing it yourself". You have to fork and update the package dependencies for 2 other projects as well, and then stay in line with those as they evolve over time.

One option in the meantime for others on this list, which we are using right now, is https://www.npmjs.com/package/patch-package (which I found this PR and that solution referenced here: https://github.com/jaredhanson/passport-google-oauth2/issues/87)

EDIT: And I should have also said thank you, Tom, as well, for this fix! :D

episage commented 1 year ago

xD This was fast. Nice one!

ciaranj commented 1 year ago

I've just published node-oauth 0.10.0 with this fix in it. I chose to bump the minor version rather than patch as it may affect calling code.

8bitDesigner commented 1 year ago

@ciaranj, you are a golden god. Thank you!

konotorii commented 1 year ago

👏👏 absolute life saver

Rishabh570 commented 1 year ago

This is amazing, thank you @ciaranj 🙌

DevinCLane commented 1 year ago

you saved my life