-
-
Notifications
You must be signed in to change notification settings - Fork 4k
feat(GuildMemberManager): add timeout option to prune() #11013
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
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Skipped Deployments
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a fan of what this means for our public API and where we take it, I don't think "exceptions" like this are ideal, especially when the design reason for it is something mutable like "this route takes a while"
I'm not sure how future majors of discord.js should address this, but at least for now I feel like we ideally covered this usecase via a REST option
Dynamic timeouts would be nice to have in REST. That said, we should definitely discuss this (and retry count) for either the next major or TSR! It feels like a common use case to have certain calls either be retried or timed out differently (I can also think of sending messages while attaching big files, you wouldn't want the timeout to always be set at minutes) |
I agree, but I think we can still support that usecase by taking a callback at the REST level that lets you decide depending on body and whatever else. I'm not 100% against the per-call approach all together, I'm against adding it to one specific method in this format with the current structure of discord.js 😅 |
concur. route-pattern / body based callback sounds much more scalable and widely applicable than having it as an option on every endpoint and adding an option key (and potentially option where there is none) to the entire high level api interface |
Noted that this approach is not suggested. Decided to update typings anyways since I had missed that during the initial PR. 🤷 |
This PR adds support for an optional
timeout
parameter to theGuildMemberManager#prune()
method.In large guilds, the prune operation can take longer than the default REST timeout (15 seconds), causing it to fail unexpectedly. By allowing a
timeout
to be passed per-request, developers can handle long-running prune operations without modifying the global REST timeout setting, reducing risk for unrelated API calls.This change is fully backward-compatible: if
timeout
is not provided, the method behaves exactly as before.Example usage:
Related issue: #10927
Status and versioning classification: