laravel-notification-channels / discord

Discord notification channel for Laravel
http://laravel-notification-channels.com
MIT License
228 stars 57 forks source link

Retry sending a notification if you're being rate limited by Discord #61

Open fakeheal opened 2 years ago

fakeheal commented 2 years ago

Hi, all,

First of all great package, thank you!

I'm trying to channel all notifications in my application to our staff discord's channel. Sometimes we send many notifications at once to various text channels we end up being rate limited.

I saw a pull request that returns the underlying GuzzleHttp exception inCouldNotSendNotification. Our notifications are queued and sent "asynchronously", so I was thinking of releasing the job again after the seconds spcified in the response by Discord.

This is my code in theory, because I don't know how to test it without making real requests to Discord (test it in ci/cd pipelines):

public function failed(Exception $exception)
{
    if ($exception instanceof CouldNotSendNotification) {
        if ($exception->getCode() === 429) {
            $json = json_decode($exception->getPrevious()->getResponse());
            $this->release($json->retry_after);
        }
    }
}

Could you please advise on how to proceed?

Thank you, Ivanka

Gummibeer commented 1 year ago

I have overridden the DiscordChannel class and added the Laravel retry() helper to retry the sending if it fails with a 429 status. This works for me - am not sure if it's the bullet-proof solution, primary with the getHeader() logic - but in theory it should be correct.

return retry(
    times: 2,
    callback: fn () => $this->discord->send($channel, [
        'content' => $message->body,
        'embed' => $message->embed,
    ]),
    sleepMilliseconds: function (int $attempts, CouldNotSendNotification $exception): int {
        $previous = $exception->getPrevious();

        if ($previous instanceof ClientException && $previous->hasResponse()) {
            return ($previous->getResponse()->getHeader('x-ratelimit-reset-after') ?: 1) * 1000;
        }

        return 1000;
    },
    when: fn (Exception $exception) => $exception instanceof CouldNotSendNotification 
        && $exception->getCode() === Response::HTTP_TOO_MANY_REQUESTS
);