pulsecron / pulse

The modern MongoDB-powered job scheduler library for Node.js
https://pulsecron.com
MIT License
90 stars 4 forks source link

Pulse complete event is called before lockedAt is cleared #17

Closed tiberioalunni closed 3 months ago

tiberioalunni commented 3 months ago

Hi, I don't know if this is intentionally done, but the event pulse.on('complete', async (job) => {}) is called before job lockedAt into collection is cleared.

This bring to a consequence: if you call pulse.stop() into complete event stop function (https://github.com/pulsecron/pulse/blob/v1.1.9/src/pulse/stop.ts) will find locked job that is already completed. This behaviour seems that don't cause any problems, but maybe could be considered that the event complete is emitted after lockedAt is set to null.

code-xhyun commented 3 months ago

Thank you for the issue! I will solve these and give you an answer as soon as possible.

code-xhyun commented 3 months ago

I did this, but the case you mentioned is not reproduced. Please provide a detailed example code @tiberioalunni

pulse.define('delete old users', async (job, done) => {
  console.log('Deleting old users...');
  done();
});

 * Example of repeating a job
 */
(async function () {
  // IIFE to give access to async/await
  await pulse.start();

  await pulse.every('1 minutes', 'delete old users');
  pulse
    .stop()
    .then(() => {
      console.log('Pulse stopped');
    })
    .catch((error) => {
      console.log('error:', error);
    });
})();

pulse.on('start', (job) => {
  console.log(time(), `Job <${job.attrs.name}> starting`);
});
pulse.on('success', (job) => {
  console.log(time(), `Job <${job.attrs.name}> succeeded`);
});
pulse.on('fail', (error, job) => {
  console.log(time(), `Job <${job.attrs.name}> failed:`, error);
});
pulse.on('complete', async (job) => {
  console.log(time(), `Job <${job.attrs.name}> completed`);
});
tiberioalunni commented 3 months ago

I did this, but the case you mentioned is not reproduced. Please provide a detailed example code @tiberioalunni

pulse.define('delete old users', async (job, done) => {
  console.log('Deleting old users...');
  done();
});

 * Example of repeating a job
 */
(async function () {
  // IIFE to give access to async/await
  await pulse.start();

  await pulse.every('1 minutes', 'delete old users');
  pulse
    .stop()
    .then(() => {
      console.log('Pulse stopped');
    })
    .catch((error) => {
      console.log('error:', error);
    });
})();

pulse.on('start', (job) => {
  console.log(time(), `Job <${job.attrs.name}> starting`);
});
pulse.on('success', (job) => {
  console.log(time(), `Job <${job.attrs.name}> succeeded`);
});
pulse.on('fail', (error, job) => {
  console.log(time(), `Job <${job.attrs.name}> failed:`, error);
});
pulse.on('complete', async (job) => {
  console.log(time(), `Job <${job.attrs.name}> completed`);
});
pulse.define('delete old users', async (job, done) => {
  console.log('Deleting old users...');
  done();
});

 * Example of repeating a job
 */
(async function () {
  // IIFE to give access to async/await
  await pulse.start();

  await pulse.every('1 minutes', 'delete old users');

})();

pulse.on('start', (job) => {
  console.log(time(), `Job <${job.attrs.name}> starting`);
});
pulse.on('success', (job) => {
  console.log(time(), `Job <${job.attrs.name}> succeeded`);
});
pulse.on('fail', (error, job) => {
  console.log(time(), `Job <${job.attrs.name}> failed:`, error);
});
pulse.on('complete', async (job) => {
  console.log(time(), `Job <${job.attrs.name}> completed`);

   pulse
    .stop()  // <--- internally job.lockedAt still have a value, stop function will clear lockedAt field
    .then(() => {
      console.log('Pulse stopped');
    })
    .catch((error) => {
      console.log('error:', error);
    });

});

I'm not sure if that can be considered a real bug or just a behavior that is acceptable in that case (calling pulse.stop at the end of an execution is not really common). But I think that a complete event should be dispatched when state of job is set as completed. If you try to query Pulse collection on Mongo when complete event occurs now you will find the job responsable for the dispatch too.

code-xhyun commented 3 months ago

@tiberioalunni Well, it's a case that's not normally considered. However, as you said, for events that have already been processed, it seems better to be treated as an exception

I will attach the added document address just in case! https://docs-pulse.pulsecron.com/docs/managing-job-processor/stop

tiberioalunni commented 3 months ago

Thank you!