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

🐛 Too many permission overrides makes Needle slow #294

Open
MarcusOtter opened this issue Sep 4, 2022 · 5 comments
Open

🐛 Too many permission overrides makes Needle slow #294

MarcusOtter opened this issue Sep 4, 2022 · 5 comments
Labels
confirmed bug 🦋 Something isn't working

Comments

@MarcusOtter
Copy link
Owner

Describe the bug

Having this many channel overrides causes Needle to take more than 3 seconds to reply, which leads to an error on Discord's end.

image

Steps to reproduce the bug

  1. Add about as many channels overrides you see in the image above
  2. Create a thread
  3. Try to close the thread
  4. See "this interaction failed" (error "Unknown interaction" shows up in console a few seconds later)

Expected behavior

It should work under 3 seconds regardless of how many overrides you have.

@MarcusOtter MarcusOtter added the confirmed bug 🦋 Something isn't working label Sep 4, 2022
@MarcusOtter
Copy link
Owner Author

A short term fix is to acknowledge the interaction which gives us more than 3 seconds to respond.
But in the long term it should definitely not take more than 3 seconds for Needle to react to a command like this, so we need to improve the performance of the permission check a lot.

@c43721
Copy link
Contributor

c43721 commented Sep 4, 2022

@MarcusOtter Please assign to me, I can investigate.

@MarcusOtter
Copy link
Owner Author

Sure! The problem should be this method here. I know we can at restructure this and Promise.all, but there may be other performance improvements too (for example, don't fetch the member roles in every iteration for the roles here)

@c43721
Copy link
Contributor

c43721 commented Sep 4, 2022

We cannot parallelize that fetch- we will quickly run into rate limits. I don't think we need that fetch, I will investigate.

@MarcusOtter
Copy link
Owner Author

Any updates @c43721?

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

No branches or pull requests

2 participants