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

Access HTTP Headers from Event Handlers #759

Closed
4 of 10 tasks
kaifresh opened this issue Jan 16, 2021 · 4 comments · Fixed by #1177
Closed
4 of 10 tasks

Access HTTP Headers from Event Handlers #759

kaifresh opened this issue Jan 16, 2021 · 4 comments · Fixed by #1177
Assignees
Labels
enhancement M-T: A feature request for new functionality
Milestone

Comments

@kaifresh
Copy link

kaifresh commented Jan 16, 2021

Description

I am hosting Bolt on a serverless option from GCP which has a slow VM start time. As a result, on the first request to a VM, the response is delayed and so the Slack Events API interprets this as an error, and sends a second (and sometimes a third request). However, the slow Bolt server does eventually load and processes the first, second (and sometimes third) request, resulting in duplicate responses. This is a bad time.

I am aware of the presence of the Slack header which indicates the retry number, which could be used to ameliorate the issue described above. However, - at least in the case of the ExpressReceiver - it appears that the event data which is passed to the bolt handler only contains the request body.

I am proposing that headers be included in the the event data passed to Bolt handler functions. This allows handlers to be sensitive to Slack Retry information (& any other information also carried in the HTTP Headers). In other words alter the definition of ReceiverEvent & its usages:

  export interface ReceiverEvent {
    body: StringIndexed;
    headers: StringIndexed; // new field
    ack: AckFn<any>;
  }

What type of issue is this? (place an x in one of the [ ])

  • bug
  • enhancement (feature request)
  • question
  • documentation related
  • example code related
  • testing related
  • discussion

Requirements (place an x in each of the [ ])

  • I've read and understood the Contributing guidelines and have done my best effort to follow them.
  • I've read and agree to the Code of Conduct.
  • I've searched for any related issues and avoided creating a duplicate issue.
@gitwave gitwave bot added the untriaged label Jan 16, 2021
@mwbrooks mwbrooks added enhancement M-T: A feature request for new functionality and removed untriaged labels Jan 18, 2021
@mwbrooks
Copy link
Member

Hey @kaifresh, thanks for suggesting the HTTP Header enhancement and discussing your implementation details. We're also grateful to hear how you're hosting your Bolt app - hearing these details helps us make better decisions! 🙏🏻

(Also, thank you to everyone who's +1 this issue, please keep those coming!)

Access to HTTP headers has been requested before and we originally planned to include it in the Bolt 3.0 release (see #670). Unfortunately, it didn't make the major release but we want to follow up with a minor update to include it.

I think we'll start to use this issue as the place to track that work. I've labeled it as an enhancement but we may switch it to a discussion if we need to work out more details.

@GuineaPigApp
Copy link

Hi,

I was just about to open a similar request. We're experiencing the same issues running on Firebase Cloud Functions. Almost everything is working well, but our biggest issue right now are the cold starts of our functions which generates retries which generates duplicate requests. I've tried different ways to stop this, but the easiest way by far would be to use the Slack Event API header which shows retry number, but I'm right now unable to access it. Would love to get access to this. Would that be possible?

Thanks!

@mwbrooks
Copy link
Member

Thanks for the +1 @GuineaPigApp!

The cold start is a frustrating issue with FaaS hosting. Just to make sure, are you also using the processBeforeResponse options. It won't solve the cold start issue, but it can solve some race conditions that happen with Lambda (not sure if the same issue exists on Firebase/Google Cloud).

@GuineaPigApp
Copy link

Hi @mwbrooks,

Yes! Here's how we're doing it right now:
const app = new App({ receiver: expressReceiver, authorize: authorizeFn, processBeforeResponse: true });

I just tried to set up a http function that'd first check the headers from Slack and then push it to the functions.https.onRequest(expressReceiver.app), but I forgot that I'd get this error so it didn't end up working: Request verification failed (code: slack_bolt_receiver_authenticity_error, message: Slack request signing verification failed. Signature mismatch.).

Is there any quick workaround that you could suggest to access the request headers or somehow avoid getting these retries due to cold starts? As a follow up to that: Do retries have any of the same data, such as trigger_id or event_id? If so, maybe I could try to store these in a database when I get a new request and remove it when the function is done, checking at the start of every function if this trigger_id or event_id is already there? If the ids are not the same, I don't know how else I could check if it's a retry without the request headers. Really appreciate the help, thanks!

@seratch seratch added this to the 3.5.0 milestone Mar 23, 2021
@seratch seratch modified the milestones: 3.5.0, 3.6.0 Jul 2, 2021
@seratch seratch modified the milestones: 3.6.0, 3.7.0, 3.8.0 Aug 17, 2021
@seratch seratch self-assigned this Oct 4, 2021
seratch added a commit to seratch/bolt-js that referenced this issue Oct 26, 2021
…ies in ReceiverEvent and Context objects
seratch added a commit that referenced this issue Oct 28, 2021
… Context objets (#1177)

* Fix #759 #1109 #1110 by adding custom properties in ReceiverEvent and Context objects
* Add AWS receiver implementation
* Fix bugs in SocketModeReceiver and add more tests
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 a pull request may close this issue.

4 participants