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

Failed to shutdown the HTTP server for OAuth flow, when using sockets + customRoutes #1368

Closed
4 of 10 tasks
sbcgua opened this issue Mar 14, 2022 · 4 comments · Fixed by #1369
Closed
4 of 10 tasks

Failed to shutdown the HTTP server for OAuth flow, when using sockets + customRoutes #1368

sbcgua opened this issue Mar 14, 2022 · 4 comments · Fixed by #1369
Labels
bug M-T: confirmed bug report. Issues are confirmed when the reproduction steps are documented
Milestone

Comments

@sbcgua
Copy link
Contributor

sbcgua commented Mar 14, 2022

Description

Using socketMode = true with customRoutes (simple health check from documentation in fact) results in [ERROR] Failed to shutdown the HTTP server for OAuth flow: undefined on app.stop()

The reason (in my opinion) is this code in SocketModeReceiver.js:

    stop() {
        if (this.httpServer !== undefined) {
            // This HTTP server is only for the OAuth flow support
            this.httpServer.close((error) => {
                this.logger.error(`Failed to shutdown the HTTP server for OAuth flow: ${error}`);
                ^^^^^
            });
        }

Here is no condition to check if error occured or not
The fix would be:

if (error) this.logger.error(`Failed to shutdown the HTTP server for OAuth flow: ${error}`);

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:

node version:

OS version(s):

Steps to reproduce:

Expected result:

What you expected to happen

Actual result:

What actually happened

Attachments:

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

sbcgua added a commit to sbcgua/bolt-js that referenced this issue Mar 14, 2022
@srajiang srajiang added the bug M-T: confirmed bug report. Issues are confirmed when the reproduction steps are documented label Mar 14, 2022
@srajiang srajiang added this to the 3.11.0 milestone Mar 14, 2022
@srajiang
Copy link
Member

srajiang commented Mar 15, 2022

@sbcgua - I agree that assuming that error will always be defined is incorrect, so your proposed changes make sense. I could repro the logger output simply by starting my app app.start() and stopping it app.stop(). Thanks for making a contribution with your PR - I will follow up tomorrow AM PST to see if we can improve the test coverage before merging.

I'm curious though how using your customRoutes in your use case led to you seeing this logging output and identifying the missing logic. Could you elaborate on your setup?

@sbcgua
Copy link
Contributor Author

sbcgua commented Mar 15, 2022

Honestly speaking I didn't debug it through to explain the reason in detail but here is the code sample:

// imports ... and env validation

const app = new App({
    signingSecret: process.env.SLACK_SIGNING_SECRET,
    token: process.env.SLACK_BOT_TOKEN,
    socketMode: true,
    appToken: process.env.SLACK_APP_TOKEN,
    logLevel,
    // customRoutes: [{
    //     path: '/health-check',
    //     method: ['GET'],
    //     handler: (req, res) => {
    //         res.writeHead(200);
    //         res.end('OK');
    //     },
    // }],
});

app.message(async (payload) => {
  ...
});

async function shutdown() {
    const timeout = setTimeout(() => {
        console.error('Stop server timed out, force exit');
        process.exit(1);
    }, 3000);
    console.log('Stopping server ...');
    await app.stop();
    clearTimeout(timeout);
    process.exit(0);
}

process.on('SIGTERM', shutdown);
process.on('SIGINT', shutdown);
process.on('SIGBREAK', shutdown);
process.on('unhandledRejection', (reason) => {
    console.error('Unhandled rejection');
    console.error(reason);
    process.exit(1);
});

(async () => {
    try {
        await app.start(process.env.PORT || 3000);
        console.log('Bolt app is running!');
    } catch (error) {
        console.error('Bolt app start failed', error.message);
    }
})();

This code works fine. If I uncomment the customRoutes param, it outputs the mentioned message

image

@sbcgua
Copy link
Contributor Author

sbcgua commented Mar 15, 2022

Also if this helps:

  • node version 16
  • the issue occures both in win and linux environments
  • "@slack/bolt": "^3.10.0"

@srajiang
Copy link
Member

Thanks! Merged your changes :)

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.

2 participants