jbielick / faktory_worker_node

a node.js client and worker framework for Faktory job server
https://github.com/contribsys/faktory
MIT License
142 stars 31 forks source link

Worker never resumes after calling `quiet()`. Resuming by calling `worker.work()` never resets the `quieted` property. #403

Closed gijoehosaphat closed 1 month ago

gijoehosaphat commented 1 month ago

node.js version: 18.15.0

npm/yarn version: 1.22.21

faktory-server version: 1.9.0

facktory-worker package version: 4.5.1

Code sample:

import faktory, { Job, Worker } from 'faktory-worker'

const worker = new Worker()

//Conditional value as an example. In our use case this boolean is determined elsewhere.
let condition = false

async function workerFunc(data) {
  //Some condition we want to quiet for...
  if (condition) {
    worker.quiet()
    setTimeout(() => {
      condition = false
      worker.work() //Does not set quieted to false
    }, 1000 * 60)
  } else {
    //Actual job work goes here
    condition = true
  }
}

function startWorker() {
  worker.register('JobType', workerFunc)
  worker.work()
}

startWorker()

Expected Behavior:

After the worker quiet period is over, calling worker.work() should flip the bit on the Worker.quieted boolean value so the worker actually continues to process jobs again.

Actual Behavior:

When calling worker.work() the Worker.quieted value remains set to true

Steps to reproduce the behavior or a test case:

Run the above referenced code, and/or attempt to quiet your worker and see that calling work never resumes job handling.


I'd be happy to open a PR for this. I manually edited the worker.js file like this to "fix" things.

async work() {
        debug("work concurrency=%i", this.concurrency);
        this.quieted = false; //This ensures a quieted worker is unquieted.
        this.execute = (0, create_execution_chain_1.default)(this.middleware, this.registry);
        await this.beat();
        this.pulse = setInterval(async () => {
            try {
                await this.beat();
            }
            catch (error) {
                this.emit("error", new Error(`Worker failed heartbeat: ${error.message}\n${error.stack}`));
            }
        }, this.beatInterval);
        this.trapSignals();
        this.tick();
        return this;
    }
jbielick commented 1 month ago

4.6.0 includes your fix. Want to give it a try?

gijoehosaphat commented 1 month ago

Works like a charm, thank you!