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

Forbidden: 403 Forbidden (error code: 90001): Reaction blocked #2486

Closed
sentry-io bot opened this issue Mar 24, 2023 · 12 comments
Closed

Forbidden: 403 Forbidden (error code: 90001): Reaction blocked #2486

sentry-io bot opened this issue Mar 24, 2023 · 12 comments
Labels
t: bug Something isn't working

Comments

@sentry-io
Copy link

sentry-io bot commented Mar 24, 2023

Sentry Issue: BOT-3EZ

Forbidden: 403 Forbidden (error code: 90001): Reaction blocked
(2 additional frame(s) were not displayed)
...
  File "bot/decorators.py", line 143, in inner
    ) -> None:
  File "bot/exts/utils/snekbox/_cog.py", line 583, in eval_command
    await self.run_job(ctx, job)
  File "bot/exts/utils/snekbox/_cog.py", line 541, in run_job
    job = await self.continue_job(ctx, response, job.name)
  File "bot/exts/utils/snekbox/_cog.py", line 452, in continue_job
    await ctx.message.add_reaction(REDO_EMOJI)
@wookie184
Copy link
Contributor

Can reproduce by

  1. Sending an eval command
  2. Blocking the bot
  3. Editing the eval message

The bot tries to add the redo emoji but it can't because it's blocked.

There are probably other cases in the bot with this issue. Not sure if there are any other actions that can fail due to being blocked.

@wookie184 wookie184 added the t: bug Something isn't working label Mar 24, 2023
@mbaruh
Copy link
Member

mbaruh commented Mar 24, 2023

Honestly I wonder if we should bother addressing it. It's a very unlikely scenario chosen by the user, we could technically just ignore this

@wookie184
Copy link
Contributor

Since it causes an error I think it's worth handling. It causes a message to be sent in the channel saying that it's unexpected behaviour and to let us know, and sends a sentry alert, neither of which are ideal.

The handling should be simple, possibly just exiting the function if the error is raised.

@mbaruh
Copy link
Member

mbaruh commented Mar 24, 2023

Hmm yeah the message is an issue. The alert can be suppressed

@TizzySaurus
Copy link
Contributor

TizzySaurus commented Mar 30, 2023

Would checking the error.code and ignoring when it's 90001 (reaction blocked) be an adequate solution to this? That way we prevent both the sentry alert and the message saying to contact staff (or can even send a custom message).

If we're concerned about it suppressing other things we can always ensure we create a log message as part of the handling.

@wookie184
Copy link
Contributor

Would checking the error.code and ignoring when it's 90001 (reaction blocked) be an adequate solution to this? That way we prevent both the sentry alert and the message saying to contact staff (or can even send a custom message).

If we're concerned about it suppressing other things we can always ensure we create a log message as part of the handling.

Were you thinking this would go in the global error handler? That sounds good to me. I can't think of another reason the reactions should be blocked so as long as there's an info log or custom message as you say I think it would be fine.

@TizzySaurus
Copy link
Contributor

Were you thinking this would go in the global error handler?

Yeah, that was the idea. If we're happy with this implementation then can I please be assigned?

@TizzySaurus
Copy link
Contributor

Actually, thinking further on this, I suppose this is also an issue in sir-lancebot, right?

I've checked locally and, as an example, the easter/halloween/etc. reactions to keywords have the same issue, so we'd presumably need handling there, which makes me think maybe this should be somewhere in bot-core to prevent duplication across the two bot repos? Although I don't think bot-core currently has a global error handling system (if that's even possible).

Tl;Dr; should this be PRd to both bots separately, or to bot-core in some shape?

@wookie184
Copy link
Contributor

The entire error handler could be migrated over to botcore, which would be the best way of doing that. I was planning on doing that at some point after #2439 and #2436 are merged.

If it's added to bot now it'll be copied across in the migration, which would be fine. We could wait for it to be in botcore to add to lancebot since this is a low priority issue. I don't really mind.

@TizzySaurus
Copy link
Contributor

TizzySaurus commented May 4, 2023

We could wait for it to be in botcore to add to lancebot since this is a low priority issue. I don't really mind.

I'd be happy to PR for both bot and sir-lancebot now, since it's a small and easy fix, and then it can be unified into a global handler in bot-core as part of the migration whenever that happens.

EDIT: Turns out it was actually more complex than I expected (see #2580), so will just leave it on bot for now. It can be added to lancebot when the handler is migrated to botcore.

@hedyhli
Copy link
Member

hedyhli commented Feb 13, 2024

Now that #2580 and python-discord/bot-core#177 is merged, is this issue still relevant, or are we still encountering it?

@wookie184
Copy link
Contributor

Ah yeah, I think this can be closed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants