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

receiverAckTimeoutError should not throw if authorization of incoming event fails #364

Closed
4 of 9 tasks
zachsirotto opened this issue Jan 2, 2020 · 9 comments · Fixed by #891
Closed
4 of 9 tasks
Assignees
Labels
bug M-T: confirmed bug report. Issues are confirmed when the reproduction steps are documented
Milestone

Comments

@zachsirotto
Copy link

zachsirotto commented Jan 2, 2020

Description

When an incoming event fails to authenticate via Bolt's authorizeFn, the following warning is thrown:

https://github.com/slackapi/bolt/blob/227421b324bcf5548d1263db7b61fc66eb407908/src/App.ts#L395

Then, the following error is thrown that the incoming event is not acknowledged.

https://github.com/slackapi/bolt/blob/6cd88eb3a6246405dee9afe06f793ed7c59958f2/src/ExpressReceiver.ts#L75-L78

Is this the expected design? If listeners are not being called, why do we need to call ack()?

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

  • bug
  • enhancement (feature request)
  • question
  • documentation 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.

Bug Report

Filling out the following details about bugs will help us solve your issue sooner.

Reproducible in:

package version: >= 1.2.0

node version: v12.12.0

OS version(s): macOS Catalina 10.15.1

Steps to reproduce:

  1. With authorizeFn, do not authorize an incoming event.

Expected result:

Incoming event is not sent to listeners. receiverAckTimeoutError is not thrown.

Actual result:

receiverAckTimeoutError is thrown.

Attachments:

   2020-01-02T15:25:13.64-0500 [APP/PROC/WEB/0] ERR { Error: Authorization of incoming event did not succeed.
   2020-01-02T15:25:13.64-0500 [APP/PROC/WEB/0] ERR     at Object.errorWithCode (/home/vcap/deps/0/node_modules/@slack/bolt/dist/errors.js:17:19)
   2020-01-02T15:25:13.64-0500 [APP/PROC/WEB/0] ERR     at authorizationErrorFromOriginal (/home/vcap/deps/0/node_modules/@slack/bolt/dist/App.js:293:28)
   2020-01-02T15:25:13.64-0500 [APP/PROC/WEB/0] ERR     at authorizeResult.authorize.catch (/home/vcap/deps/0/node_modules/@slack/bolt/dist/App.js:145:32)
   2020-01-02T15:25:13.64-0500 [APP/PROC/WEB/0] ERR     at process._tickCallback (internal/process/next_tick.js:68:7)
   2020-01-02T15:25:13.64-0500 [APP/PROC/WEB/0] ERR   code: 'slack_bolt_authorization_error',
   2020-01-02T15:25:13.64-0500 [APP/PROC/WEB/0] ERR   original:
   2020-01-02T15:25:13.64-0500 [APP/PROC/WEB/0] ERR    Error: No matching authorizations for E27SFGS2W:T0258Q0SG
   2020-01-02T15:25:13.64-0500 [APP/PROC/WEB/0] ERR        at App.module.exports [as authorize] (/home/vcap/app/src/authorizeFn.js:15:15)
   2020-01-02T15:25:13.64-0500 [APP/PROC/WEB/0] ERR        at App.onIncomingEvent (/home/vcap/deps/0/node_modules/@slack/bolt/dist/App.js:144:45)
   2020-01-02T15:25:13.64-0500 [APP/PROC/WEB/0] ERR        at ExpressReceiver.App.receiver.on.message (/home/vcap/deps/0/node_modules/@slack/bolt/dist/App.js:55:53)
   2020-01-02T15:25:13.64-0500 [APP/PROC/WEB/0] ERR        at ExpressReceiver.emit (events.js:198:13)
   2020-01-02T15:25:13.64-0500 [APP/PROC/WEB/0] ERR        at ExpressReceiver.requestHandler (/home/vcap/deps/0/node_modules/@slack/bolt/dist/ExpressReceiver.js:70:14)
   2020-01-02T15:25:13.64-0500 [APP/PROC/WEB/0] ERR        at Layer.handle [as handle_request] (/home/vcap/deps/0/node_modules/express/lib/router/layer.js:95:5)
   2020-01-02T15:25:13.64-0500 [APP/PROC/WEB/0] ERR        at next (/home/vcap/deps/0/node_modules/express/lib/router/route.js:137:13)
   2020-01-02T15:25:13.64-0500 [APP/PROC/WEB/0] ERR        at respondToUrlVerification (/home/vcap/deps/0/node_modules/@slack/bolt/dist/ExpressReceiver.js:122:5)
   2020-01-02T15:25:13.64-0500 [APP/PROC/WEB/0] ERR        at Layer.handle [as handle_request] (/home/vcap/deps/0/node_modules/express/lib/router/layer.js:95:5)
   2020-01-02T15:25:13.64-0500 [APP/PROC/WEB/0] ERR        at next (/home/vcap/deps/0/node_modules/express/lib/router/route.js:137:13) }
   2020-01-02T15:25:13.64-0500 [APP/PROC/WEB/0] ERR [WARN]   Authorization of incoming event did not succeed. No listeners will be called.
   2020-01-02T15:25:16.44-0500 [APP/PROC/WEB/0] ERR { Error: An incoming event was not acknowledged before the timeout. Ensure that the ack() argument is called in your listeners.
   2020-01-02T15:25:16.44-0500 [APP/PROC/WEB/0] ERR     at receiverAckTimeoutError (/home/vcap/deps/0/node_modules/@slack/bolt/dist/ExpressReceiver.js:213:19)
   2020-01-02T15:25:16.44-0500 [APP/PROC/WEB/0] ERR     at Timeout.setTimeout [as _onTimeout] (/home/vcap/deps/0/node_modules/@slack/bolt/dist/ExpressReceiver.js:39:32)
   2020-01-02T15:25:16.44-0500 [APP/PROC/WEB/0] ERR     at ontimeout (timers.js:436:11)
   2020-01-02T15:25:16.44-0500 [APP/PROC/WEB/0] ERR     at tryOnTimeout (timers.js:300:5)
   2020-01-02T15:25:16.44-0500 [APP/PROC/WEB/0] ERR     at listOnTimeout (timers.js:263:5)
@seratch seratch added discussion M-T: An issue where more input is needed to reach a decision good first issue Good for newcomers labels Jan 3, 2020
@seratch
Copy link
Member

seratch commented Jan 3, 2020

👋Thank you for flagging this up.

Ensure that the ack() argument is called in your listeners

I agree that this part of the error message is not relevant to this case.

Currently, Bolt doesn't respond to a request in the case mentioned here. In the case of denied requests, personally I prefer to respond with HTTP status code 40x than just not responding.

However, if Bolt changes the behavior to respond with an error status code in future versions, it needs updates on the interface between App and Receiver first. Currently, App class doesn't have any means to tell Receivers to respond with a specific HTTP status code.

I would like to know @aoberoi @stevengill 's thoughts on this.

@zachsirotto
Copy link
Author

@seratch I would like to respond to the denied request but i'm unsure how to do that from the scope of the authorizeFn since the passed in parameters are only { teamId, enterpriseId }. How are you doing this?

@seratch
Copy link
Member

seratch commented Jan 3, 2020

@zachsirotto

How are you doing this?

At this moment, it's not possible to respond to requests in the case.

However, if Bolt changes the behavior to respond with an error status code in future versions, it needs updates on the interface between App and Receiver first.

That's why I mentioned the necessity to change App & Receiver if Bolt changes its internals to allow developers to specify response status code.

@barlock
Copy link
Contributor

barlock commented Jan 3, 2020

@aoberoi ☝️ This is a really good example of why I think App.processEvent in the proposal #353 (comment) should reject with an error.

@zachsirotto
Copy link
Author

Is it possible to prevent receiverAckTimeoutError from being thrown when a request is rejected? I don't understand why it would be relevant to throw this error for unauthenticated requests.

@zachsirotto
Copy link
Author

@seratch do you have any suggestions to prevent this error from firing?

{ Error: An incoming event was not acknowledged before the timeout. Ensure that the ack() argument is called in your listeners.
    at receiverAckTimeoutError (/node_modules/@slack/bolt/dist/ExpressReceiver.js:225:19)
    at Timeout.setTimeout [as _onTimeout] (/node_modules/@slack/bolt/dist/ExpressReceiver.js:43:32)
    at ontimeout (timers.js:436:11)
    at tryOnTimeout (timers.js:300:5)
    at listOnTimeout (timers.js:263:5)
    at Timer.processTimers (timers.js:223:10) code: 'slack_bolt_receiver_ack_timeout_error' }

@stevengill stevengill added bug M-T: confirmed bug report. Issues are confirmed when the reproduction steps are documented and removed discussion M-T: An issue where more input is needed to reach a decision good first issue Good for newcomers labels Apr 2, 2020
@zachsirotto
Copy link
Author

@stevengill are there any temporary workarounds that I can perform to prevent this error / warning from firing? It creates a lot of noise.

@seratch seratch added this to the 3.6.0 milestone Aug 18, 2021
@seratch
Copy link
Member

seratch commented Aug 18, 2021

#891 can improve this. When I started working on the pull request again, I will check if it resolves this issue as well.

@srajiang srajiang modified the milestones: 3.6.0, 3.7.0 Aug 19, 2021
@seratch
Copy link
Member

seratch commented Aug 26, 2021

I've confirmed that #891 can resolve this issue.

import { App, ExpressReceiver } from '@slack/bolt';
const app = new App({
  receiver: new ExpressReceiver({
    signingSecret: process.env.SLACK_SIGNING_SECRET!!,
  }),
  authorize: async (_) => { throw new Error("Failed!"); }
});

The above example app returns 401 HTTP status and shows the following logs only:

[WARN]  bolt-app Authorization of incoming event did not succeed. No listeners will be called.
[ERROR]  bolt-app Error: Failed!
    at /Users/ksera/github/bolt-js/examples/getting-started-typescript/dist/app.js:25:15
    at Generator.next (<anonymous>)
    at /Users/ksera/github/bolt-js/examples/getting-started-typescript/dist/app.js:8:71
    at new Promise (<anonymous>)
    at __awaiter (/Users/ksera/github/bolt-js/examples/getting-started-typescript/dist/app.js:4:12)
    at App.authorize (/Users/ksera/github/bolt-js/examples/getting-started-typescript/dist/app.js:24:23)
    at App.processEvent (/Users/ksera/github/bolt-js/dist/App.js:375:46)
    at ExpressReceiver.requestHandler (/Users/ksera/github/bolt-js/dist/receivers/ExpressReceiver.js:154:77)
    at Layer.handle [as handle_request] (/Users/ksera/github/bolt-js/node_modules/express/lib/router/layer.js:95:5)
    at next (/Users/ksera/github/bolt-js/node_modules/express/lib/router/route.js:137:13) {
  code: 'slack_bolt_authorization_error'
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug M-T: confirmed bug report. Issues are confirmed when the reproduction steps are documented
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants