Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
…nteractive payloads can cause unhandled promise rejections that crash the Node.js process.
This commit fixes the issue reported at packages/adapter-slack/src/index.ts:1152
**Bug Analysis:**
In `routeSocketEvent` (line 1150), which is a synchronous `void` method, two async operations produce floating promises:
1. `this.handleSlashCommand(params)` (line 1165) - `handleSlashCommand` is `async` and always returns a `Promise<Response>`. It calls `await this.lookupUser(userId)` which internally calls `await this.chat.getState().get()` (before the try/catch around the API call), and `this.chat.processSlashCommand()`. Any of these could throw.
2. `this.dispatchInteractivePayload(payload)` (line 1172) - Returns `Response | Promise<Response>`. When the payload type is `view_submission`, it delegates to `async handleViewSubmission()`, which calls `await this.chat.processModalSubmit()` and accesses `payload.view.state.values` (which could throw on malformed payloads).
Since `routeSocketEvent` is synchronous (`void` return type) and called from a sync context within the socket mode event handler (after `await ack()` has already completed), these returned promises are fire-and-forget. If any reject, it triggers an unhandled promise rejection, which in Node.js 15+ terminates the process by default.
In contrast, in the webhook code path (`handleWebhook`), these same methods are always `return`-ed from async functions, so their promises are properly chained to the caller.
**Fix:**
Added `.catch()` handlers to both floating promises:
1. For `handleSlashCommand`: Added `.catch()` that logs the error via `this.logger.error`.
2. For `dispatchInteractivePayload`: Since it returns `Response | Promise<Response>` (only a Promise for `view_submission`), used `instanceof Promise` to conditionally attach a `.catch()` handler only when the result is a Promise.
This approach was chosen over making `routeSocketEvent` async because: (a) it doesn't change the method signature, (b) the caller doesn't need to await it (the ack has already been sent), and (c) errors are logged rather than silently swallowed.
Co-authored-by: Vercel <vercel[bot]@users.noreply.github.com>
Co-authored-by: haydenbleasel <hello@haydenbleasel.com>
- Export SlackForwardedSocketEvent type - Add x-slack-socket-token check at top of handleWebhook() for forwarded events - Update routeSocketEvent() to accept WebhookOptions and use waitUntil - Add startSocketModeListener(), runSocketModeListener(), forwardSocketEvent() Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Forwarded event accepted/rejected based on appToken - Bypasses signature verification for forwarded events - Options passthrough to handlers - startSocketModeListener returns 200/500 appropriately Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- New /api/slack/socket-mode route using createPersistentListener - Mirrors Discord gateway pattern (CRON_SECRET auth, Redis coordination) - Cron runs every 9 min, listener duration 10 min Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Make signingSecret optional (string | undefined) instead of falling back to "". verifySignature now returns false when no secret is configured, preventing HMAC with an empty key from silently passing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sync errors from processEventPayload were silently dropped in socket mode. Wrap with try-catch for parity with slash_commands and interactive cases. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@reaganmcf do me a favour try it again 🙏🏻 |
|
@dancer trying now! |
|
@dancer this is awesome! The only thing that doesn't work is that the This results in:
Supporting the |
|
sweet sounds good will fix / look into that shortly and get this merged over the weekend / monday thanks for the help @reaganmcf |
|
+1 |
…-support # Conflicts: # packages/adapter-slack/src/index.test.ts
|
@reaganmcf sorry for the delay can i get you to do another quick test pushed some more changes in 925f8cf let me know & thanks a bunch for the feedback |
|
after you test again i can get this merged today cc: @bensabic |
|
@dancer nested slack modals work! Works great - would love to get this merged |
|
SGTM let's ship it |
Summary
x-slack-socket-tokenheadermode: "webhook"(default) — socket mode listener is a separate external concernrouteSocketEvent()now acceptsWebhookOptionsand useswaitUntilfor async handlersstartSocketModeListener()/runSocketModeListener()/forwardSocketEvent()methods/api/slack/socket-modewithcreatePersistentListenerfor Redis-based cross-instance coordinationTest plan
appToken→ 200appTokenconfigured → 401startSocketModeListenerreturns 200 with valid configstartSocketModeListenerreturns 500 withoutwaitUntilorappTokenrouteSocketEventpasses options through to handlerspnpm validatepasses (knip, check, typecheck, test, build)Closes #123