Set this.client to null before calling client.close()

apollographql / subscriptions-transport-ws

:arrows_clockwise: A WebSocket client + server for GraphQL subscriptions

https://www.npmjs.com/package/subscriptions-transport-ws

MIT License

1.52k stars 342 forks source link

Set this.client to null before calling client.close() #829

Closed ibash closed 2 years ago

ibash commented 3 years ago

client.close can sometimes lead to a synchronous call to close. This leads to the error: [TypeError: Cannot set property 'onopen' of null]

This change sets client to null before calling onClose, so that the second synchronous call does not try to cleanup the client.

TODO:

[x] Make sure all of the significant new logic is covered by tests
[x] Rebase your changes on master so that they can be merged easily
[x] Make sure all tests and linter rules pass
[x] Update CHANGELOG.md with your change

ghost commented 3 years ago

Should fix #808

tyagow commented 3 years ago

@ibash sorry to bother you, any estimation on fixing this conflict? I am more than happy to help just not sure how... Facing this issue right now and I don't want to patch the package...

ibash commented 3 years ago

@tyagow Unfortunately there's nothing I can do, it's up to the apollo team to merge it and it seems like they've abandoned this package :(

Your best bet is patching the package, happy to help you set that up if need be. But in general:

Follow setup instructions here
Edit node_modules/subscriptions-transport-ws/dist/client.js with these changes
Run npx patch-package subscriptions-transport-ws

ibash commented 3 years ago

@glasser any chance this can be merged?

tyagow commented 3 years ago

@tyagow Unfortunately there's nothing I can do, it's up to the apollo team to merge it and it seems like they've abandoned this package :(

Your best bet is patching the package, happy to help you set that up if need be. But in general:

Follow setup instructions here

Edit node_modules/subscriptions-transport-ws/dist/client.js with these changes

Run npx patch-package subscriptions-transport-ws

Thanks for the instructions, I am using this approach.

ml242 commented 3 years ago

This package is used by Apollo Client which makes the errors extra irritating as I would have to fork and pin both packages.