Throwing error in fetchInstallation leads to "An incoming event was not acknowledged within 3 seconds" error and Lambda timeout

[broom9]

Description

Hi there,

We have some use cases that we need to throw errors in the fetchInstallation method in the OAuth flow (Slack teams that are not recognized by us installed our Slack app).

Based on the document https://slack.dev/bolt-js/concepts#error-handling. Throwing an error in the fetchInstallation is the way to handle errors.

But when an error is thrown in the fetchInstallation method, after 3 seconds, an error log "[ERROR] An incoming event was not acknowledged within 3 seconds. Ensure that the ack() argument is called in a listener." will be printed.

And using Lambda serverless deployment, the Lambda will timeout and be considered as failed to execute.

I think there might be some error handling issue with errors thrown from fetchInstallation. The 3 seconds error message shouldn't show, and Lambda shouldn't timeout or fail in this case.

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.

---

Bug Report

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

Reproducible in:

package version: 3.3.0

node version: 12.18.4

OS version(s): Linux

Steps to reproduce:

1. Create an application using OAuth flow.
2. Throw a dummy error in the fetchInstallation method
3. Send a DM message to the bot.

Expected result:

A 401 or 403 response is returned on authorization error.
Lambda doesn't timeout or fail.

Actual result:

No response was returned.
Lambda timeout

Attachments:

Logs, screenshots, screencast, sample project, funny gif, etc.

[seratch]

@broom9 Thanks for taking the time to report this!

The issue

As you mentioned, in this scenario, a Bolt app outputs the following log messages and returns 500 Internal Server Error. There is no way to customize the behavior as long as you use any of the built-in receivers. I agree there is room for improvement regarding this.

[WARN]  bolt-app Authorization of incoming event did not succeed. No listeners will be called.
[ERROR]  bolt-app Error: Failed fetching data from the Installation Store
    at new AuthorizationError (/path-to-app/node_modules/@slack/oauth/dist/errors.js:71:28)
    at InstallProvider.<anonymous> (/path-to-app/node_modules/@slack/oauth/dist/index.js:158:31)
    at step (/path-to-app/node_modules/@slack/oauth/dist/index.js:44:23)
    at Object.next (/path-to-app/node_modules/@slack/oauth/dist/index.js:25:53)
    at fulfilled (/path-to-app/node_modules/@slack/oauth/dist/index.js:16:58)
    at processTicksAndRejections (internal/process/task_queues.js:97:5) {
  code: 'slack_bolt_authorization_error'
}
[ERROR]   An unhandled error occurred while Bolt processed an event
[DEBUG]   Error details: Error: Failed fetching data from the Installation Store, storedResponse: undefined
[ERROR]   An incoming event was not acknowledged within 3 seconds. Ensure that the ack() argument is called in a listener.

The following is a simple OAuth example app I used for verifying this issue:

const { LogLevel } = require("@slack/logger");
const { App } = require("@slack/bolt");

const database = {};

const app = new App({
  logLevel: process.env.SLACK_LOG_LEVEL || LogLevel.DEBUG,
  signingSecret: process.env.SLACK_SIGNING_SECRET,
  clientId: process.env.SLACK_CLIENT_ID,
  clientSecret: process.env.SLACK_CLIENT_SECRET,
  stateSecret: 'my-state-secret',
  scopes: ['commands', 'chat:write'],
  installationStore: {
    storeInstallation: async (installation) => {
      // change the line below so it saves to your database
      if (installation.isEnterpriseInstall) {
        // support for org wide app installation
        database[installation.enterprise.id] = installation;
      } else {
        // single team app installation
        database[installation.team.id] = installation;
      }
    },
    fetchInstallation: async (installQuery) => {
      // change the line below so it fetches from your database
      if (installQuery.isEnterpriseInstall && installQuery.enterpriseId !== undefined) {
        // org wide app installation lookup
        return database[installQuery.enterpriseId];
      }
      if (installQuery.teamId !== undefined) {
        // single team app installation lookup
        return database[installQuery.teamId];
      }
      throw new Error('Failed fetching installation');
    },
  },
});

(async () => {
  await app.start(process.env.PORT || 3000);
  console.log("⚡️ Bolt app is running!");
})();

Possible solution for this issue

To improve this, there are several approach we can consider. We can do all of them at the same time if necessary. To eliminate the issue described here, I think we can work on 1: first. And then, we may consider implementing 2: separately when we have the bandwidth for it.

• 1: Change the built-in receivers' default behavior (return 40x status, not 500 - skip the "no ack" error message)
• 2: Enable developers to set a custom error handler in the built-in receiver

1: Change the built-in receivers' default behavior (return 40x status, not 500 - skip the "no ack" error message)

We can improve all the built-in receivers that have this issue. In the case of HTTPReceiver, we can change the following parts:

• Return 40x error instead if error code is slack_bolt_authorization_error: https://github.com/slackapi/bolt-js/blob/%40slack/bolt%403.3.0/src/receivers/HTTPReceiver.ts#L367-L371
• Set isAcknowledged to true also when the receiver returns a 40x status response: https://github.com/slackapi/bolt-js/blob/%40slack/bolt%403.3.0/src/receivers/HTTPReceiver.ts#L310-L317
• Note that this part is unable to set the HTTP response

I assume no existing apps expect 500 response status in this scenario. If there is a strong concern on it, we may move the change to v4 milestone and also provide some fallback option. With being that said, I'm thinking it's safe enough to "improve" the behavior in a minor release for the reason I mentioned above (the bold part).

2: Enable developers to set a custom error handler in the built-in receiver

Developers can customize the global error handler on the App listener execution. However, any of the built-in receivers do not provide the similar yet.

If developers would like to customize the behavior in the scenario of this issue, they have to implement their own custom receiver. If it's possible to customize only the error handling part by passing a function in the constructor, it would be far better for maintainability of the code.

The possible use case would be:
If a required user token associated with the command/shortcut invoker is missing and the app can respond with an ephemeral message using response_url or an HTTP response, the app may reply with a message like "This app needs your approval to do this operation, please install this app from [here](slack installation URL) :bow:".

We may want to have a new task issue for this enhancement. Thoughts?

---

I would like to know other maintainers' thoughts on this before starting working on 1: task.

[seratch]

I will implement the solution 1 in the v3.4.0 release. For the solution 2, I will create another issue for it. We may work on it in the future versions.

[seratch]

👋 I just started working on this task at #891 - let us know if you have feedback or suggestions on it

[seratch]

As we are aiming to release v3.4.0 within a few days and this pull request needs more time, let me move this one to v3.5 milestone.