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

Expose axios timeout parameter for webhooks #1378

Closed
1 of 6 tasks
tk26 opened this issue Nov 18, 2021 · 6 comments
Closed
1 of 6 tasks

Expose axios timeout parameter for webhooks #1378

tk26 opened this issue Nov 18, 2021 · 6 comments
Assignees
Labels
enhancement M-T: A feature request for new functionality good first issue pkg:webhook applies to `@slack/webhook`
Milestone

Comments

@tk26
Copy link

tk26 commented Nov 18, 2021

By default axios has no timeout, this is a bad default for production systems and currently there is no way to override it.
You can override the socket timeout with the agent but it's not the same thing.
I would like to be able to change that via the WebClient options.

Note: This was added for web-api in #1250
As a part of this issue, it'd be great if we could support webhooks.

Packages:

Select all that apply:

  • @slack/web-api
  • @slack/rtm-api
  • @slack/webhooks
  • @slack/oauth
  • @slack/socket-mode
  • I don't know

Requirements

Please read the Contributing guidelines and Code of Conduct before creating this issue or pull request. By submitting, you are agreeing to those rules.

@tk26 tk26 added the untriaged label Nov 18, 2021
@tk26 tk26 changed the title (Set a clear title describing your idea) Expose axios timeout parameter for webhooks Nov 18, 2021
@seratch seratch added this to the webhook@6.x milestone Nov 18, 2021
@seratch seratch added enhancement M-T: A feature request for new functionality pkg:webhook applies to `@slack/webhook` good first issue and removed untriaged labels Nov 18, 2021
@seratch
Copy link
Member

seratch commented Nov 18, 2021

Hi @tk26, thanks for writing in! I agree that this should be a good addition.

@tk26
Copy link
Author

tk26 commented Nov 18, 2021

that was an ultra-fast response! thanks @seratch - mind if I take a stab at it?

@seratch
Copy link
Member

seratch commented Nov 18, 2021

If you are fine to sign our CLA (https://cla-assistant.io/slackapi/node-slack-sdk), a fix with test code (like this way: #1264) would be appreciated!

@tk26
Copy link
Author

tk26 commented Nov 18, 2021

Great! I just signed the CLA. Will create a fix soon. Thanks @seratch

@filmaj
Copy link
Contributor

filmaj commented Dec 6, 2021

This issue was resolved after merging #1394. I will cut a webhook@6.1.0 release shortly which will include this new option.

@filmaj filmaj closed this as completed Dec 6, 2021
@tk26
Copy link
Author

tk26 commented Dec 15, 2021

I totally lost track of this issue. Thanks for fixing it and seeing it through @xuhas @filmaj

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 good first issue pkg:webhook applies to `@slack/webhook`
Projects
None yet
Development

No branches or pull requests

3 participants