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

Initialization includes unhandled async request #248

Closed
4 of 9 tasks
SpencerKaiser opened this issue Sep 11, 2019 · 7 comments · Fixed by #1303
Closed
4 of 9 tasks

Initialization includes unhandled async request #248

SpencerKaiser opened this issue Sep 11, 2019 · 7 comments · Fixed by #1303
Assignees
Labels
auto-triage-skip Prevent this issue from being closed due to lack of activity bug M-T: confirmed bug report. Issues are confirmed when the reproduction steps are documented
Milestone

Comments

@SpencerKaiser
Copy link

Description

On initializing an App, it appears a call to the API test endpoint is made but the response is not being handled appropriately.

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

Creating an app with bad credentials results in an UnhandledPromiseRejectionWarning.

Reproducible in:

package version: *

node version: *

OS version(s): *

Steps to reproduce:

const Bolt = require('@slack/bolt');
let app = new Bolt.App({
    signingSecret: 'JUNK',
    token: 'JUNK',
    logLevel: Bolt.LogLevel.DEBUG
});

Expected result:

The app should handle the failed response and then throw an appropriate error when used after the faulty initialization.

In my opinion, this auth test call shouldn't be a part of the initialization, but I can see why y'all would add it.

Actual result:

The app initializes and an UnhandledPromiseRejectionWarning occurs

Additional Info

Here are the logs for the above snippet:

> const Bolt = require('@slack/bolt');
undefined
> let app = new Bolt.App({
...     signingSecret: 'JUNK',
...     token: 'JUNK',
...     logLevel: Bolt.LogLevel.DEBUG
... });
[DEBUG]  WebClient:0 initialized
[DEBUG]  WebClient:0 apiCall('auth.test') start
[DEBUG]  WebClient:0 will perform http request
undefined
> [DEBUG]  WebClient:0 http response received
(node:50240) UnhandledPromiseRejectionWarning: Error: An API error occurred: invalid_auth
    at Object.platformErrorFromResult (.../node_modules/@slack/web-api/dist/errors.js:50:33)
    at WebClient.apiCall (.../node_modules/@slack/web-api/dist/WebClient.js:382:28)
    at process._tickCallback (internal/process/next_tick.js:68:7)
(node:50240) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:50240) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
@shaydewael shaydewael added the bug M-T: confirmed bug report. Issues are confirmed when the reproduction steps are documented label Oct 1, 2019
@shaydewael
Copy link
Contributor

I can reproduce this. It looks like there's a bug with our SingleTeamAuthorization function that should be fixed to handle these error responses correctly. Both the users.info and auth.test calls in that method aren't being properly handled.

@SpencerKaiser
Copy link
Author

@shaydewael @seratch @aoberoi any update on this? No rush, just curious.

The only reason it's an issue is it can sometimes mask/muddle other problems like this one (see the comment at the bottom of the issue).

@aoberoi
Copy link
Contributor

aoberoi commented Jan 25, 2020

👋 Hey there, thanks for checking in. We haven't started working on this yet, but I agree that it should be a relatively high priority to get this bug out of your way. I'm adding it to our project board now.

@SpencerKaiser
Copy link
Author

@aoberoi we ended up fixing it on our end just by making the Slack init something we wait for instead of having it just start and assuming it'll work.

We realized that that pattern was introducing other problems too, so I'm glad we refactored to fix it.

I'd still love to see this handled in some way so I could chose to ignore the error.

@seratch seratch modified the milestone: V2 Feb 13, 2020
@DanielTamkin
Copy link

@SpencerKaiser Having the similar issue, have any code to share your fix? I'm just looking to verify tokens before discarding the App.

@SpencerKaiser
Copy link
Author

@DanielTamkin we have a decent workaround, but still not great.

@github-actions
Copy link

github-actions bot commented Dec 5, 2021

👋 It looks like this issue has been open for 30 days with no activity. We'll mark this as stale for now, and wait 10 days for an update or for further comment before closing this issue out.

@seratch seratch added auto-triage-skip Prevent this issue from being closed due to lack of activity and removed auto-triage-stale labels Dec 5, 2021
seratch added a commit to seratch/bolt-js that referenced this issue Feb 4, 2022
@seratch seratch added this to the 3.10.0 milestone Feb 7, 2022
@seratch seratch self-assigned this Feb 7, 2022
seratch added a commit that referenced this issue Feb 14, 2022
Co-authored-by: Sarah Jiang <srajiang@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-triage-skip Prevent this issue from being closed due to lack of activity bug M-T: confirmed bug report. Issues are confirmed when the reproduction steps are documented
Projects
None yet
5 participants