fix(WebSocketManager): await WebSocket destroy

[Mogyuchi]

Please describe the changes this PR makes and why it should be merged:
Fixed to wait for WebSocket destroy to complete.
This is because '@discordjs/ws' destroy returns Awaitable.

Status and versioning classification:

• I know how to update typings and have done so, or typings don't need updating
• This PR changes the library's interface (methods or parameters added)
• This PR includes breaking changes (methods removed or renamed, parameters moved or removed)

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Ignored Deployments

Name	Status	Preview	Comments	Updated (UTC)
discord-js	⬜️ Ignored (Inspect)			Jun 9, 2023 9:56am
discord-js-guide	⬜️ Ignored (Inspect)	Visit Preview		Jun 9, 2023 9:56am

[TetieWasTaken]

This looks like a breaking change to me, could you add the correct versioning classification? Although technically a fix it could lead to unexpected race conditions.

[Syjalo]

Although technically a fix it could lead to unexpected race conditions.

Well, it's already not awaited in the library

[Qjuh]

Awaiting the WebsocketManager#destroy() here isn’t needed. Before the switch to using @discordjs/ws the disconnect of the Websocket connection wasn’t awaited too, it just wasn’t that apparent because there were no async functions involved.

However since a destroyed Client should not be used for anything else afterwards (since it‘s destroyed) I don’t see what race conditions would be solved by awaiting this. Especially since this is a breaking change.

[didinele]

What exactly does this fix? Did you actually run into a case where this caused issues?

[femshima]

A possible usecase here.

In order to ensure that the bot goes offline and also the process stops after sending SIGINT, it is necessary to handle SIGINT and call process.exit()(in case uncancelled setTimeout or something like that prevents the process from stopping).

However, in the current implementation, calling process.exit() immediately after calling client.destroy() leaves the bot shown online on the client.

process.on('SIGINT', () => {
  client.destroy();
  process.exit();
})

If client.destroy() returns Promise that waits until the disconnection completes, it will be possible to ensure process stops after bot's websocket is normally closed.

process.on('SIGINT', async () => {
  await client.destroy();
  process.exit();
})

[Mogyuchi]

My bot did exactly that and worked as expected until 14.9. However, when I updated to 14.10.2, the bot would not go offline or exit VoiceChannel instantly.

[github-actions]

⚡️ Lighthouse report for the changes in this PR:

Category	Score
🟠 Performance	70
🟢 Accessibility	98
🟢 Best practices	100
🟢 SEO	94
🟠 PWA	70

Lighthouse ran on https://discord-js-guide-git-fork-mogyuchi-fix-ws-destroy-discordjs.vercel.app/guide/home/introduction