-
Notifications
You must be signed in to change notification settings - Fork 660
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
fix: prevent double callback in _executeRequest #363
Conversation
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. |
Hitting this issue (just on server startup and first request) myself. Any ides on if/when this may be merged? Thanks! |
+1 Lost 2 working days right before demo day because of this issue. Please merge this PR, thank you. |
+1 behind schedule with no way of fixing it, would be greatly appreciated if merged, thanks. |
Guys, this is open source software. Nobody gets paid to merge this.
I promise not to delete this code in the next 100 years. |
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
<#363 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAANQSDZXOQI2W5HBLPAKP3VVL2F5ANCNFSM5ID3CYDA>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
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: jaredhanson/passport-google-oauth2#87) EDIT: And I should have also said thank you, Tom, as well, for this fix! :D |
xD This was fast. Nice one! |
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. |
@ciaranj, you are a golden god. Thank you! |
👏👏 absolute life saver |
This is amazing, thank you @ciaranj 🙌 |
you saved my life |
Mostly for ciaranj/node-oauth#363, which fixes jaredhanson/passport-oauth2#163. I haven't been able to reproduce that issue with passport-twitter, but in theory it should be possible. I'm not aware of any breaking changes in `oauth`. See https://github.com/ciaranj/node-oauth/commits
Mostly for ciaranj/node-oauth#363, which fixes jaredhanson/passport-oauth2#163. I haven't been able to reproduce that issue with passport-oauth1, but in theory it may be possible. I'm not aware of any breaking changes in `oauth`. See https://github.com/ciaranj/node-oauth/commits
www.googleapis.com
doesECONNRESET
just after data is received inpassBackControl
This prevents the callback from being called twice.