Skip to content
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

Rate Limit #21

Closed
a7md opened this issue Sep 10, 2021 · 7 comments · Fixed by #35
Closed

Rate Limit #21

a7md opened this issue Sep 10, 2021 · 7 comments · Fixed by #35

Comments

@a7md
Copy link

a7md commented Sep 10, 2021

Hello,

I really like this package and i use it in my applications to get the latest errors and fix them asap, but sometimes the errors spams, and i have jobs, so the discord logger gives an exception and it stops the jobs from working, because of the error exception.

so i hope there is an exception bypass to the discord rate limit, i mean by that if an error happened and discord rate limited the hook, it does not stop the app jobs from working, it pass the error.

Thank you very much again for such good & helpful package like this.

@vpratfr
Copy link
Member

vpratfr commented Sep 10, 2021

I would be open to a pull request if you could have a look. I have little time to get that implemented but I too encountered the same problem on a project of mine.

@CaptainCannabis
Copy link

Same here, loggin in jobs and stuff like that to a devlog discord channel for delightful (yeah i know about sentry and tools like that) exception fixing. But sometimes rate limit kicks in. Would it be possible to avoid that by letting the discord-logger count the send messages and than, if rate limit would be reached log to file instead?

@vpratfr
Copy link
Member

vpratfr commented Nov 10, 2021

Hi,

As I said, I would happily welcome a (well tested) PR.

We could indeed for example keep a hash of the last 10 messages sent, along with the timestamp.

Then, before sending a message, check if the hash is already in our history and if the timestamp is older than 1 minutes for instance.

That way we would avoid sending the same messages too often.

@sammyaxe
Copy link

sammyaxe commented Nov 26, 2022

Yeah, this is a pretty big problem, especially in prod, because it causes the application to crash. Maybe it would make sense to simply add logic inside try-catch if the application is in prod mode to not throw error? or just log it somewhere else?

@Xinecraft
Copy link
Contributor

Xinecraft commented Feb 23, 2023

yea, crashing a job just coz logger fail to communicate with discord seems to be a problem. Apart from 429 there might be other discord sided issue too.
Just ignoring feels better.

Lemme know if u open for a PR to just ignore error instead of throwing Exception.

Also thanks for this helpful package. 🙏

@vpratfr
Copy link
Member

vpratfr commented Feb 23, 2023 via email

@Xinecraft
Copy link
Contributor

Just noticed there is an option in the stack driver in Laravel already to ignore exceptions. 😅
image

But still, if someone needs to disable discord specifically or doesn't use stack driver then here is PR for it. #35

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants