Skip to content
This repository has been archived by the owner on Jan 8, 2022. It is now read-only.

feat(Errors): show data sent when an error occurs #72

Merged
merged 2 commits into from
Oct 18, 2021

Conversation

ckohen
Copy link
Member

@ckohen ckohen commented Oct 11, 2021

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

Ported from discordjs/discord.js#5701

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
  • This PR changes the library's interface (methods or parameters added)

Ported from discordjs/discord.js#5701

Co-authored-by: Vlad Frangu <kingdgrizzle@gmail.com>
@codecov
Copy link

codecov bot commented Oct 11, 2021

Codecov Report

Merging #72 (b77830d) into main (90c25ad) will increase coverage by 0.34%.
The diff coverage is 95.55%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #72      +/-   ##
==========================================
+ Coverage   89.48%   89.83%   +0.34%     
==========================================
  Files           8        8              
  Lines        1037     1072      +35     
  Branches      107      107              
==========================================
+ Hits          928      963      +35     
  Misses          3        3              
  Partials      106      106              
Impacted Files Coverage Δ
...ackages/rest/src/lib/handlers/SequentialHandler.ts 84.05% <90.90%> (+0.94%) ⬆️
packages/rest/src/lib/RequestManager.ts 93.68% <100.00%> (ø)
packages/rest/src/lib/errors/DiscordAPIError.ts 82.47% <100.00%> (+2.71%) ⬆️
packages/rest/src/lib/errors/HTTPError.ts 96.66% <100.00%> (+1.42%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 90c25ad...b77830d. Read the comment docs.

@kyranet
Copy link
Member

kyranet commented Oct 11, 2021

Perhaps we should make those take objects? 5 parameters is a bit much.

@ckohen
Copy link
Member Author

ckohen commented Oct 11, 2021

I didn't because they are al required. Also, you can't use the shorthand for properties when destructuring, though I know that will probably want to change anyways so that it can be documented properly

iCrawl
iCrawl previously approved these changes Oct 18, 2021
Copy link
Member

@iCrawl iCrawl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

packages/rest/src/lib/errors/DiscordAPIError.ts Outdated Show resolved Hide resolved
packages/rest/src/lib/errors/HTTPError.ts Outdated Show resolved Hide resolved
packages/rest/src/lib/errors/DiscordAPIError.ts Outdated Show resolved Hide resolved
packages/rest/src/lib/errors/HTTPError.ts Outdated Show resolved Hide resolved
@iCrawl iCrawl merged commit 3e2edc8 into discordjs:main Oct 18, 2021
@ckohen ckohen deleted the data-in-errors branch October 18, 2021 23:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants