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

Append original stack when WebClient#apiCall throws #1086

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

oleg-codaio
Copy link

Summary

When an API call within @slack/web-api fails, the resulting error's stack loses the call context:

Error: An API error occurred: account_inactive
     at Object.platformErrorFromResult (/project/node_modules/@slack/web-api/src/errors.ts:94:5)
     at WebClient.apiCall (/project/node_modules/@slack/web-api/src/WebClient.ts:188:13)
     at runMicrotasks (<anonymous>)
     at processTicksAndRejections (internal/process/task_queues.js:97:5)

This PR appends the original stack leading up to the call so that developers can more easily trace failing calls.

For more details, see #561 (comment)

Requirements (place an x in each [ ])

@gitwave gitwave bot added the untriaged label Aug 24, 2020
return result;
return result;
} catch (err) {
err.stack += `\n${originalStack}`;
Copy link
Author

Choose a reason for hiding this comment

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

This currently looks as follows:

Error: An API error occurred: account_inactive
     at Object.platformErrorFromResult (/project/node_modules/@slack/web-api/src/errors.ts:94:5)
     at WebClient.apiCall (/project/node_modules/@slack/web-api/src/WebClient.ts:188:13)
     at runMicrotasks (<anonymous>)
     at processTicksAndRejections (internal/process/task_queues.js:97:5)
Error: thrown by
    at WebClient.apiCall (/project/node_modules/@slack/web-api/dist/WebClient.js:120:40)
    at /project/my_file.ts:288:19

Happy to tweak as needed.

@misscoded misscoded added enhancement M-T: A feature request for new functionality and removed untriaged labels Aug 24, 2020
@codecov
Copy link

codecov bot commented Aug 25, 2020

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.37%. Comparing base (caa1fd0) to head (c272377).
Report is 651 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1086      +/-   ##
==========================================
+ Coverage   94.34%   94.37%   +0.02%     
==========================================
  Files          12       12              
  Lines         796      800       +4     
  Branches      177      177              
==========================================
+ Hits          751      755       +4     
  Misses         15       15              
  Partials       30       30              
Flag Coverage Δ
eventsapi 90.74% <ø> (ø)
interactivemessages 94.83% <ø> (ø)
webapi 95.63% <ø> (+0.05%) ⬆️
webhook 87.50% <ø> (ø)

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

@stevengill
Copy link
Member

stevengill commented Sep 3, 2020

Hey @vaskevich!

Thanks for sending this in.

Ended up commenting on the issue to continue discussion

@stevengill stevengill added discussion M-T: An issue where more input is needed to reach a decision semver:patch pkg:web-api applies to `@slack/web-api` and removed discussion M-T: An issue where more input is needed to reach a decision labels Sep 3, 2020
@seratch seratch added this to the [email protected] milestone Mar 25, 2021
@jaywhiletwo
Copy link

This would really improve the developer experience. Currently, errors thrown by the web client are only referencing the stack as they know it when the promise is being resolved. This means we often cannot determine the source of the error without painstaking debugging and isolation steps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement M-T: A feature request for new functionality pkg:web-api applies to `@slack/web-api` semver:patch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants