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

[fix] Bree doesn't gracefully exit #242

Open
3 tasks done
ThisIsMissEm opened this issue Feb 1, 2024 · 4 comments
Open
3 tasks done

[fix] Bree doesn't gracefully exit #242

ThisIsMissEm opened this issue Feb 1, 2024 · 4 comments
Labels
bug Something isn't working

Comments

@ThisIsMissEm
Copy link

Describe the bug

Node.js version: 20

OS version: n/a — happens on both mac and linux

Description:

When the main Bree process is terminated with ctrl+c (SIGINT), using graceful, the worker process is told to terminate, which results in it exiting with a 1 exit code, even though it was terminated by the main process.

This results in the errorHandler option receiving a Worker for job "BackgroundJob" exited with code 1 message

Actual behavior

The logs printed during graceful termination look like the following (here I'm using pino with pino-pretty as the logger:

[22:44:47.504] INFO (BackgroundJob/71797): Received cancellation of job
[22:44:47.507] INFO (71797): Worker for job "BackgroundJob" sent a message
[22:44:47.507] INFO (71797): Gracefully cancelled worker for job "BackgroundJob"
[22:44:47.519] ERROR (71797): Worker for job "BackgroundJob" exited with code 1
    err: {
      "type": "Error",
      "message": "Worker for job \"BackgroundJob\" exited with code 1",
      "stack":
          Error: Worker for job "BackgroundJob" exited with code 1
              at Worker.<anonymous> (node_modules/bree/src/index.js:476:13)
              at Worker.emit (node:events:530:35)
              at [kOnExit] (node:internal/worker:315:10)
              at Worker.<computed>.onexit (node:internal/worker:229:20)
    }
[22:44:47.524] INFO (71797): Gracefully exited

Expected behavior

Bree should be aware that it requested the job to terminate, and therefore not treat this exit code of 1 as an error, i.e., it should actually gracefully shut down.

Per the node.js Worker documentation:

The 'exit' event is emitted once the worker has stopped. If the worker exited by calling process.exit(), the exitCode parameter is the passed exit code. If the worker was terminated, the exitCode parameter is 1.

Code to reproduce

// set up logging
const logger = pino({});

// set up jobs/scheduler
// configuration in src/jobs folder
const bree = new Bree({
  logger,
  root: path.join(__dirname, "../src/jobs"),
  jobs: [
    {
      name: "BackgroundJob", // only fires on start
    },
  ],
  errorHandler: (error) => {
    logger.error(error);
  },
});

It doesn't matter what "BackgroundJob" actually does.

Checklist

  • I have searched through GitHub issues for similar issues.
  • I have completely read through the README and documentation.
  • I have tested my code with the latest version of Node.js and this package and confirmed it is still not working.
@ThisIsMissEm ThisIsMissEm added the bug Something isn't working label Feb 1, 2024
@ThisIsMissEm
Copy link
Author

ThisIsMissEm commented Feb 1, 2024

I believe this is due to this exit handler not being aware that Bree is attempting to shutdown:

https://github.com/breejs/bree/blob/master/src/index.js#L469

@titanism
Copy link
Contributor

titanism commented Feb 1, 2024

PR welcome + added test case would be great too 🙏

@ThisIsMissEm
Copy link
Author

PR welcome + added test case would be great too 🙏

If I'm gonna be working on scheduler code, I'll write it from scratch and ditch Bree completely, tbh. I've been trying to avoid writing this code.

@titanism
Copy link
Contributor

titanism commented Feb 2, 2024

Best of luck

@titanism titanism reopened this Feb 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants