Wavelink icon indicating copy to clipboard operation
Wavelink copied to clipboard

Rewrite: Websocket

Open scragly opened this issue 3 years ago • 5 comments

This issue is for discussing and collating the issues and wanted improvements of Wavelink's Websocket for the rewrite project.

If any particular point gets a lot of attention, consider creating a dedicated issue ticket to discuss it in further detail and to collate all the details specific to it there to ensure this issue is kept relatively clean and focused on the feature as a whole.

Current issues

  • re-connection loop issue?

Improvements

  • add graceful closing
  • more descriptive errors
  • look into not waiting until d.py client is ready

scragly avatar Aug 15 '20 16:08 scragly

Reconnection loop could be fixed by a lock in connect method or by awaiting it.

Graceful closing. Wait until bot is ready and then over ride the loop's signal handlers to destroy every node which closes the respective websocket with 1000, and when everything is done, we stop the loop with loop.stop

WizzyGeek avatar Aug 15 '20 17:08 WizzyGeek

A very basic implementation (of closing)

https://github.com/PythonistaGuild/Wavelink/commit/90e296b27acad2deda0888fda4b08f005710de66

WizzyGeek avatar Aug 15 '20 17:08 WizzyGeek

I'd very much like the custom JSON encoders as in #65 to be usable in Wavelink, but I really feel like the implementation I proposed is not ideal because it sets way too many attributes.

I would love to see it refined in the rewrite and seems like Scrag agrees with me.

I've also requested binary websocket traffic in Lavalink, but seeing the comment it won't be added in the near future.

Gelbpunkt avatar Aug 16 '20 15:08 Gelbpunkt

I don't see a major issue with adding support to custom serialisers. If @EvieePy doesn't have any issues with ensuring it's added as a feature, we'll pin it as an feature to add.

Edit: A proper ticket has now been created for custom JSON encoders at #75 .

scragly avatar Aug 16 '20 15:08 scragly

Maybe add a close() method which complements _connect(), so that node doesn't have to get into websocket's internals to close the web socket.

WizzyGeek avatar Sep 23 '20 05:09 WizzyGeek