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

change deprecated usage of res.send #487

Merged
merged 1 commit into from
Apr 27, 2020

Conversation

dburandt
Copy link
Contributor

Summary

Small fix to replace deprecated usage of res.send(); for status codes.

Currently, a deprecation warning will get propagated down to bolt users if they hit this catch block.

Requirements (place an x in each [ ])

@codecov
Copy link

codecov bot commented Apr 27, 2020

Codecov Report

Merging #487 into master will not change coverage.
The diff coverage is 0.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #487   +/-   ##
=======================================
  Coverage   84.23%   84.23%           
=======================================
  Files           7        7           
  Lines         552      552           
  Branches      163      163           
=======================================
  Hits          465      465           
  Misses         60       60           
  Partials       27       27           
Impacted Files Coverage Δ
src/ExpressReceiver.ts 64.60% <0.00%> (ø)

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 d1ae744...a7fb1ed. Read the comment docs.

@@ -108,7 +108,7 @@ export default class ExpressReceiver implements Receiver {
this.logger.debug('stored response sent');
}
} catch (err) {
res.send(500);
res.status(500).send();
Copy link
Member

Choose a reason for hiding this comment

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

Although the warning message suggests to use res.sendStatus(status), I prefer your change. In this case, a Slack app doesn't need to send any data in response body.

@seratch
Copy link
Member

seratch commented Apr 27, 2020

Your change looks good to me. I will wait for responses from other maintainers for a few business days before merging it. Thanks for fixing this!

@seratch seratch added the enhancement M-T: A feature request for new functionality label Apr 27, 2020
@seratch seratch merged commit 455bf58 into slackapi:master Apr 27, 2020
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants