slackapi / node-slack-sdk

Slack Developer Kit for Node.js
https://tools.slack.dev/node-slack-sdk/
MIT License
3.28k stars 662 forks source link

web-api: expose axios interceptors as `WebClient` constructor option #2073

Closed mtjandra closed 1 month ago

mtjandra commented 1 month ago

I'd like to transform outgoing requests from the @slack/web-api package similar to other SDKs such as

This Slack SDK allowa adding additional headers but I require more flexibility. Taking a look at the source it seems it is using axios under the hood (ref). Axios seems to support the concept of interceptors which would support my use case.

I'd like to transform all outgoing requests into POST requests with a body representing the original request, add custom headers, then re-route that to a proxy. Is this possible to configure with the current version @slack/web-api?

If what I'm asking for is not currently possible, is there anything in the works?


Side note: was also thinking we could override the method making the outgoing request (makeRequest), although I'm confused why all outgoing requests are post requests

filmaj commented 1 month ago

It may be possible in JavaScript, though you'd have to reach in to the private axios instance contained within WebClient, which may be tricky - not sure what the state of the art for ignoring class visibility modifiers is in JS 🤷

Alternatively, this could be a new feature request: expose some option on the WebClient constructor to provide the ability to specify interceptors.

although I'm confused why all outgoing requests are post requests

Because this project is aimed at interacting solely with the Slack HTTP API, and it is designed to accept POST requests for any HTTP API method you wish to interact with, this is a convenient approach for the API client. See this brief overview in our docs for details, in particular:

Pass arguments as POST parameters presented as application/x-www-form-urlencoded When sending a HTTP POST, you may present your arguments as either standard POST parameters, or you may use JSON instead.

mtjandra commented 1 month ago

Hi @filmaj thank you for the reply.

you'd have to reach in to the private axios instance contained within WebClient

I would like to avoid overriding and/or modifying private methods/variables as it could lead to unexpected behaviour and breaking changes on version updates.

Alternatively, this could be a new feature request: expose some option on the WebClient constructor to provide the ability to specify interceptors.

Yes it would be amazing if this could be added as a feature. I'm sure other consumers would benefit from this feature as well. It seems fairly common for SDKs to allow consumers to hook into the request life cycle.

filmaj commented 1 month ago

OK, I've set it as a feature / enhancement. We cannot promise any delivery timelines for it. However, if this is something you need, we welcome pull requests!

mtjandra commented 1 month ago

@filmaj I have created a pr: https://github.com/slackapi/node-slack-sdk/pull/2076

I've sign the contributor license agreement, but CI is still failing, you may need to re-run the check on your side.

image

Please advise.

filmaj commented 1 month ago

Closing and re-opening - this sometimes helps reset the CLA robot.

filmaj commented 1 month ago

I meant to do on the PR, woopsie. Anyways, the CLA check is now resolved. Will review your PR shortly. Thank you for sending it!

mtjandra commented 4 weeks ago

@filmaj any idea on when a new release will be published with these changes?

I would love to start making use of it!

filmaj commented 4 weeks ago

Just released web-api 7.7.0 on npm and as a GitHub Release!