add timeout option for `broadcasteval`

[imranbarbhuiya]

Which package is the feature request for?

discord.js

Feature

Add a timeout option in broadcasteval after which if a shard didn't respond, resolve the promise with null value or reject it. My bot uses multiple broadcasteval and waits for one broadcast to finish before starting a new one. This works most of the time but when a shard gets respawned for some reason, it doesn't reply to the broadcast message. So the broadcast eval never resolves and all the next broadcast eval stops working.

Ideal solution or implementation

Add a timeout option to broadcastEval. When using broadcastEval, wait for the response from the shards, and if the timeout is present and the shard didn't reply before the timeout, resolve the promise with null or reject it. I would prefer resolve with null so that we get reply from other shards.

Alternative solutions or implementations

No response

Other context

No response

[Syjalo]

Maybe don't add that, but fix the bug?

[imranbarbhuiya]

Are u sure it's a bug? because I think it's working as expected. the promise is waiting for a reply and since the shard was restarted, it won't reply, and a proper fix will be adding support for a timeout. But if u have a better way to fix it, lemme know

[kyranet]

That sounds like a bad bug indeed — if some shards succeed to reply but one disconnects, we might want to have a clear rejection from it.

This is likely fixed in #8859, which is around the corner supporting timeouts and able to reject messages if they exceed timeout. The new sharder supports any discord.js version so it'll likely replace the ShardingManager from all versions of discord.js.

As a side note, broadcastEval will stop being a thing, we internally planned to deprecate this method as nobody wants to ship a potential attack vector (were channels to be breached somehow to allow an attacker to send messages), and instead, it'll be under the user's responsibility to handle it.

Furthermore, I do not think it's right to use the sharder's channels to broadcast messages all the time, instead, you should look into Redis streams, a message broker (such as RabbitMQ), or similar, since those systems will always be more battletested and robust than the simpler code we ship (plus, we shouldn't reinvent the wheel anyways).

The problem with broadcasting messages all the time is that it clutters the channels with messages, specially if you send them at a high rate and the messages take a lot of bandwidth to be sent, plus you add extra stress on the manager to queue and await for response from shards, which can lead to issues since the same manager is the one in charge of keeping your bot up and fully available.

Of course, you're able (and you'll still be able) to send small messages of small load, but I wouldn't recommend sticking to that forever and eventually move to dedicated messaging systems that can communicate the shards one with another without relying on the managers for it.

Nevertheless, even if the entire sharding system v13/v14 has will be removed in v15 in favour of the new /sharder, this is still a valid bug and should probably be patched.