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

Esbuild causing unhandledRejection #3219

Closed
JoshuaWise opened this issue Jul 11, 2023 · 3 comments · Fixed by sst/sst#3131
Closed

Esbuild causing unhandledRejection #3219

JoshuaWise opened this issue Jul 11, 2023 · 3 comments · Fixed by sst/sst#3131

Comments

@JoshuaWise
Copy link

I have a program that uses esbuild internally. My program is designed such that it can be cancelled at any time via ctrl+c (SIGINT). When a SIGINT is received, it doesn't just exit the process abruptly, rather, it initiates a graceful shutdown procedure. However, sometimes when I do this, I get an unhandledRejection event from esbuild. It doesn't happen all the time; it's inconsistent, probably due to the timing of things.

Anyways, esbuild should never cause unhandledRejection events. I'm doing await esbuild.build() and catching any thrown errors within a try-catch block, so it should (ideally) be impossible for any errors within esbuild to escape as unhandledRejection events.

Here is the complete stderr output:

process.on('unhandledRejection', (err) => { throw err; });
                                            ^

Error: The service was stopped
    at /[redacted]/node_modules/esbuild/lib/main.js:1076:25
    at responseCallbacks.<computed> (/[redacted]/node_modules/esbuild/lib/main.js:700:9)
    at Socket.afterClose (/[redacted]/node_modules/esbuild/lib/main.js:690:28)
    at Socket.emit (node:events:525:35)
    at endReadableNT (node:internal/streams/readable:1359:12)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21)

Node.js v18.16.1

The version of esbuild is 0.18.10.

@evanw
Copy link
Owner

evanw commented Jul 13, 2023

I was unable to reproduce this. Here's what I tried:

const esbuild = require('esbuild')

async function run() {
  try {
    await esbuild.build({
      entryPoints: ['entry'],
      plugins: [{
        name: 'temp',
        setup(build) {
          build.onResolve({ filter: /./ }, () => new Promise(() => { }))
        },
      }],
    })
  }

  // Comment this out to cause an unhandled rejection
  catch (err) { console.log('Caught:', err) }

  finally { }
}

process.on('unhandledRejection', err => {
  console.log('UNHANDLED REJECTION')
  throw err
})

console.log('Waiting for Ctrl+C...')
process.on('SIGINT', () => {
  console.log('Waiting for timeout...')
  setTimeout(() => console.log('Success'), 2000)
})

run()

It seems like esbuild is successfully funnelling the error to the build API call here.

Please provide a way to reproduce the issue. I'm marking this as unactionable until a way to reproduce this issue is provided.

Anyways, esbuild should never cause unhandledRejection events. I'm doing await esbuild.build() and catching any thrown errors within a try-catch block, so it should (ideally) be impossible for any errors within esbuild to escape as unhandledRejection events.

Agreed, and I'd like to fix this. It should be straightforward to fix this if I can reproduce it.

@JoshuaWise
Copy link
Author

Below is a script that allows me to reproduce the unhandledRejection 100% of the time. Here are the steps:

  1. Run the script below
  2. Press ctrl+c to trigger a SIGINT before the build finishes
  3. Wait for the build to finish

Here is the script:

const esbuild = require('esbuild');

async function main() {
    console.log('building...');

    await esbuild.build({
        entryPoints: ['entry.js'],
        write: false,
        bundle: true,
        plugins: [{
            name: 'loader',
            setup: (build) => {
                build.onResolve({ filter: /()/ }, async (args) => {
                    return { path: args.path, namespace: 'fake' };
                });
                build.onLoad({ filter: /()/, namespace: 'fake' }, async (args) => {
                    await sleep(2000);

                    if (args.path === 'entry.js') {
                        return {
                            loader: 'js',
                            contents: 'console.log(require("string.js"));',
                        };
                    }
                    if (args.path === 'string.js') {
                        return {
                            loader: 'js',
                            contents: 'module.exports = "hello world";',
                        };
                    }
                });
            },
        }],
    });

    console.log('done!');
}

async function sleep(ms) {
    return new Promise(resolve => setTimeout(resolve, ms));
}

process.on('SIGHUP', () => console.log('SIGHUP detected'));
process.on('SIGINT', () => console.log('SIGINT detected'));
process.on('SIGTERM', () => console.log('SIGTERM detected'));

process.on('unhandledRejection', (err) => {
    console.log('Unhandled rejection:', err);
});

main().catch((err) => {
    console.log('Error caught:', err);
});

The output I get is:

building...
^CSIGINT detected
Error caught: Error: The service was stopped
    at /[redacted]/node_modules/esbuild/lib/main.js:1076:25
    at responseCallbacks.<computed> (/[redacted]/node_modules/esbuild/lib/main.js:700:9)
    at Socket.afterClose (/[redacted]/node_modules/esbuild/lib/main.js:690:28)
    at Socket.emit (node:events:525:35)
    at endReadableNT (node:internal/streams/readable:1359:12)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21)
Unhandled rejection: Error: The service is no longer running
    at sendResponse (/[redacted]/node_modules/esbuild/lib/main.js:712:13)
    at handleRequest (/[redacted]/node_modules/esbuild/lib/main.js:733:7)

@JoshuaWise
Copy link
Author

Thanks for fixing this! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants