search

jaydenwindle / graphene-subscriptions

A plug-and-play GraphQL subscription implementation for Graphene + Django built using Django Channels.
MIT License
116 stars 15 forks source link

Consumer sends after websocket is closed #15

Open msg555 opened 4 years ago

msg555 commented 4 years ago

If you open a websocket and subscribe to a stream, close it, then open another websocket and subscribe, the next time an event arrives the _send_result callback will be invoked on the first expired websocket, attempting to send data on an already closed websocket. I'm not sure if this is ignored on some ASGI servers but on uvicorn this causes the whole connection to be dropped.

The issue appears to be that the stream variable is a global and I can't see logic to unsubscribe a closed websocket from the stream callback.

Aside from that, wouldn't stream_fired be called on the same event for each open websocket and cause the event to be delivered extra times to each consumer?

Additionally, websocket_disconnect should have an explicit group_discard call

jaydenwindle commented 4 years ago

Hey @msg555! Thanks for bringing this issue up.

You're right, the global stream variable does cause some issues currently. I didn't realize that this causes connections to drop on uvicorn, that definitely escalates the severity of this issue.

There is a PR open that should fix most (maybe all) of these issues (#7), but it contains some breaking API changes. I've been waiting to release it until I've been able to write migration docs for it. I'd love any feedback you have on it if you want to take a look :)

I'm going to prioritize these changes and will hopefully deploy a new release this weekend.

FerCalatayud commented 4 years ago

Hi, great library. But I have a question. If the user tries to subscribe but is not authorized. How do I disconnect them from the WebSocket? Or just by not returning the observable is sufficient? Right now I just return an Exception instead of the Observable.