feat(slack): accept GET for every Web API method and add api.test - #231
Closed
0xChathurinda wants to merge 2 commits into
Closed
feat(slack): accept GET for every Web API method and add api.test#2310xChathurinda wants to merge 2 commits into
0xChathurinda wants to merge 2 commits into
Conversation
Slack's Web API takes a method's arguments in the query string as well as the body, and its SDKs (python slack_sdk among them) send the read methods as GET: users.info, users.lookupByEmail, users.list, conversations.history, conversations.info, conversations.list, conversations.members, conversations.replies, reactions.get, team.info, bots.info, files.getUploadURLExternal, bookmarks.list, chat.scheduledMessages.list. Those were registered for POST only, so a client built on one of those SDKs got 404s for every read. parseSlackBody now merges query parameters under the body, and the read methods are registered for both verbs. api.test, Slack's no-auth ping that echoes its arguments, is added; setup scripts and SDK smoke checks call it first.
Contributor
|
@0xChathurinda is attempting to deploy a commit to the Vercel Labs Team on Vercel. A member of the Team first needs to authorize it. |
Not only the read methods: Slack's Web API takes any method over GET with the arguments in the query string, and clients written against that (a test harness calling auth.test and conversations.open with GET, say) got 404s here. Every /api/* method is now registered for both verbs; the six that already had a dedicated GET handler keep it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Slack's Web API accepts a method's arguments in the query string as well as the body, and calls any method with
GETas well asPOST. Its SDKs send the read methods asGET: the Pythonslack_sdk, for example, issuesusers.info,users.lookupByEmail,conversations.history,conversations.replies,files.info,files.getUploadURLExternal,reactions.get,team.infoandbots.infothat way, and scripts written against Slack's own leniency callauth.testorconversations.openwithGETtoo. The emulator registered most methods forPOSTonly, so those clients got a 404.parseSlackBodynow starts from the query string and lets body values override it, so the existing handlers work unchanged for both verbs./api/*method is registered forGETandPOSTthrough a smallonGetOrPosthelper. The six methods that already had a dedicatedGEThandler (files.info,files.list,pins.list,users.getPresence,users.profile.get,chat.getPermalink) keep it.api.testis added (GET/POST, no auth), Slack's ping that echoes its arguments and an asked-forerror; setup scripts and SDK smoke checks call it first.Testing
src/__tests__/get-methods.test.tscovers lookups and history/replies overGET, the upload URL reservation overGET, body-over-query precedence, andapi.test. The coverage matrix lists both verbs for every method.pnpm --filter @emulators/slack test,type-checkandlintpass; the package README documents the behaviour.