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

Promise result from removeRepeatable #1047

Open
xoryouyou opened this issue Sep 6, 2018 · 2 comments
Open

Promise result from removeRepeatable #1047

xoryouyou opened this issue Sep 6, 2018 · 2 comments

Comments

@xoryouyou
Copy link

Description

When having a repeatable Job running and calling queue.removeRepeatable is just simply resolves a promise with null regardless of the removal of the job.

In the sample below I create a repeatable Job and watch on the queue for completed jobs matching the repeatable job id and try to remove the job.

When passing job.opts.repeat to removeRepeatable it removes the job as it should returning null, but just passing something wrong like {foo:"bar"} does nothing and also returns null.

It would be greate when https://github.com/OptimalBits/bull/blob/master/lib/repeatable.js#L98
would return a meaningfull result from its _this.client.RemoveRepeatable call.

Minimal, Working Test code to reproduce the issue.

test.js

var Queue = require("bull");
var path = require("path");
describe("Cron Test", () => {
  let helloQueue = undefined;

  before(done => {
    // create a worker queue in redis
    helloQueue = new Queue("hello", "redis://127.0.0.1:6379");
    // spawn a given number of workers
    helloQueue.process(1, path.resolve(".hello_module.js"));
    done();
  });

  it("should be able to have cron job running every 5s", done => {
    let jobId = undefined;

    helloQueue
      .add({ hello: "Cron Job" }, { repeat: { cron: "*/5 * * * * *" } })
      .then(job => {
        jobId = job.id.split(":")[1];

        console.log("job added", jobId);
        job.finished(result => {
          console.log("finished ", result);
        });
      });

    helloQueue.on("completed", (job, result) => {
      // filter for the cron job
      if (job.id.includes(jobId)) {
        console.log("[+][Cron  Job: " + job.id + "] Result:", result);

        console.log("[.] Removing cronjob (needs job.opts.repeat object)");
        helloQueue
          .removeRepeatable({ foo: "bar" }) // change to ".removeRepeatable(job.opts.repeat)  and it works
          .then(result => {
            console.log("[=] Cron Job removed:", result);
          })
          .catch(err => {
            console.log("[x] Failed to remove Cron job", err);
          });
      }
    });
  }).timeout(500000);

  after(done => {
    helloQueue.close();
    done();
  });
});

hello_module.js

let timeout = t => {
  return new Promise((resolve, reject) => {
    setTimeout(() => resolve(), t);
  });
};

module.exports = async job => {
  console.log(
    "[.][Worker Job:" + job.id + "] Working on: " + JSON.stringify(job.data)
  );
  await timeout(100);
  return Promise.resolve("Hello " + job.data.hello);
};

Bull version

^3.4.8

Additional information

@manast
Copy link
Member

manast commented Sep 6, 2018

thanks for the complete issue report. I will mark this as an enhancement and PR request.

@xoryouyou
Copy link
Author

I've traced the call back to https://github.com/luin/ioredis/blob/551e67f977cda78e377dd688a1aaf7ab0f2651cf/lib/redis.js#L635

Which gets called by _script.execute in the commander.js https://github.com/luin/ioredis/blob/master/lib/commander.js#L171

Which gets called by https://github.com/luin/ioredis/blob/master/lib/script.js#L23

So yeah i've no idea how to bubble the result back up to queue.js but it should be possible :)

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

No branches or pull requests

2 participants