-
-
Notifications
You must be signed in to change notification settings - Fork 4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add timeout option for broadcasteval
#8872
Comments
Maybe don't add that, but fix the bug? |
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 |
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, 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. |
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 withnull
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 tobroadcastEval
. When usingbroadcastEval
, 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 withnull
or reject it. I would prefer resolve withnull
so that we get reply from other shards.Alternative solutions or implementations
No response
Other context
No response
The text was updated successfully, but these errors were encountered: