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

fix(handlers): create burst handler for interaction callbacks #8996

Merged
merged 5 commits into from
Mar 30, 2023

Conversation

ckohen
Copy link
Member

@ckohen ckohen commented Dec 31, 2022

Please describe the changes this PR makes and why it should be merged:

Interaction callbacks currently create a queue per callback, this is unnecessary and consumes excess memory for no reason.
The burst handler ignores all ratelimits (except for hitting 429s) because that's what callbacks do.

What do we think of how the route is handled with this? As it currently is this resolves #9019

Also solves the testing issue on Node 18+

Status and versioning classification:

  • Code changes have been tested against the Discord API, or there are no code changes
  • I know how to update typings and have done so, or typings don't need updating

@vercel
Copy link

vercel bot commented Dec 31, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
discord-js ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Mar 30, 2023 at 5:17PM (UTC)
1 Ignored Deployment
Name Status Preview Comments Updated
discord-js-guide ⬜️ Ignored (Inspect) Visit Preview Mar 30, 2023 at 5:17PM (UTC)

@codecov
Copy link

codecov bot commented Mar 15, 2023

Codecov Report

Merging #8996 (bd4b8fc) into main (3cf4f4b) will increase coverage by 4.33%.
The diff coverage is 89.40%.

❗ Current head bd4b8fc differs from pull request most recent head f74ddc0. Consider uploading reports for the commit f74ddc0 to get more accurate results

@@            Coverage Diff             @@
##             main    #8996      +/-   ##
==========================================
+ Coverage   79.43%   83.76%   +4.33%     
==========================================
  Files         121      102      -19     
  Lines       10070     9812     -258     
  Branches     1124     1131       +7     
==========================================
+ Hits         7999     8219     +220     
+ Misses       2031     1554     -477     
+ Partials       40       39       -1     
Flag Coverage Δ
guide ?
proxy 79.56% <ø> (ø)
rest 91.92% <89.40%> (-0.15%) ⬇️
ws 57.73% <ø> (+0.12%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
packages/rest/src/lib/handlers/IHandler.ts 0.00% <0.00%> (ø)
packages/rest/src/lib/handlers/Shared.ts 88.53% <88.53%> (ø)
packages/rest/src/lib/handlers/BurstHandler.ts 91.09% <91.09%> (ø)
...ackages/rest/src/lib/handlers/SequentialHandler.ts 87.74% <93.75%> (+0.49%) ⬆️
packages/rest/src/lib/utils/utils.ts 96.44% <95.23%> (-0.22%) ⬇️
packages/rest/src/lib/RequestManager.ts 90.16% <100.00%> (+0.33%) ⬆️
packages/rest/src/lib/utils/constants.ts 98.30% <100.00%> (+0.05%) ⬆️

... and 23 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

packages/rest/src/lib/handlers/BurstHandler.ts Outdated Show resolved Hide resolved
packages/rest/src/lib/handlers/BurstHandler.ts Outdated Show resolved Hide resolved
packages/rest/src/lib/handlers/BurstHandler.ts Outdated Show resolved Hide resolved
packages/rest/src/lib/handlers/BurstHandler.ts Outdated Show resolved Hide resolved
packages/rest/src/lib/handlers/BurstHandler.ts Outdated Show resolved Hide resolved
ckohen and others added 3 commits March 20, 2023 23:54
Co-Authored-By: Almeida <almeidx@pm.me>
Co-authored-by: Jiralite <33201955+Jiralite@users.noreply.github.com>
@vercel
Copy link

vercel bot commented Mar 30, 2023

@kyranet is attempting to deploy a commit to the discordjs Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions
Copy link

github-actions bot commented Mar 30, 2023

⚡️ Lighthouse report for the changes in this PR:

Category Score
🟠 Performance 55
🟢 Accessibility 94
🟢 Best practices 100
🟢 SEO 100
🟠 PWA 70

Lighthouse ran on https://discord-js-git-fork-ckohen-fix-interactions-ratelimit-discordjs.vercel.app/

@kodiakhq kodiakhq bot merged commit db8df10 into discordjs:main Mar 30, 2023
@ckohen ckohen deleted the fix/interactions-ratelimit branch March 30, 2023 22:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Request route for interaction responses includes the token
7 participants