Skip to content

Commit

Permalink
Revert "Add job timeout (agenda#1420)" (agenda#1426)
Browse files Browse the repository at this point in the history
This reverts commit b15f555.
  • Loading branch information
harisvsulaiman authored Jan 20, 2022
1 parent b15f555 commit 6ec531d
Show file tree
Hide file tree
Showing 7 changed files with 11 additions and 126 deletions.
20 changes: 0 additions & 20 deletions lib/agenda/default-fail-on-timeout.ts

This file was deleted.

17 changes: 0 additions & 17 deletions lib/agenda/default-timeout.ts

This file was deleted.

15 changes: 1 addition & 14 deletions lib/agenda/define.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,16 +39,6 @@ export interface DefineOptions {
* Should the return value of the job be persisted
*/
shouldSaveResult?: boolean;

/**
* Get the number of milliseconds the job can run.
*/
timeout?: number;

/**
* Determine if the job should fail when it timeouts.
*/
failOnTimeout?: boolean;
}

export type Processor =
Expand Down Expand Up @@ -85,10 +75,7 @@ export const define = function (
(options as DefineOptions).lockLifetime || this._defaultLockLifetime,
running: 0,
locked: 0,
shouldSaveResult: (options as DefineOptions).shouldSaveResult || false,
timeout: (options as DefineOptions).timeout || this._defaultTimeout,
shouldFailOnTimeout:
(options as DefineOptions).failOnTimeout || this._defaultFailOnTimeout,
shouldSaveResult: (options as DefineOptions).shouldSaveResult || false
};
debug(
"job [%s] defined with following options: \n%O",
Expand Down
14 changes: 0 additions & 14 deletions lib/agenda/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,6 @@ import { schedule } from "./schedule";
import { sort } from "./sort";
import { start } from "./start";
import { stop } from "./stop";
import { defaultTimeout } from "./default-timeout";
import { defaultFailOnTimeout } from "./default-fail-on-timeout";

export interface AgendaConfig {
name?: string;
Expand All @@ -46,8 +44,6 @@ export interface AgendaConfig {
lockLimit?: number;
defaultLockLimit?: number;
defaultLockLifetime?: number;
defaultTimeout?: number;
defaultFailOnTimeout?: boolean;
sort?: any;
mongo?: MongoDb;
db?: {
Expand Down Expand Up @@ -77,8 +73,6 @@ export interface AgendaConfig {
* @property {Boolean} _isLockingOnTheFly - true if 'lockingOnTheFly' is currently running. Prevent concurrent execution of this method.
* @property {Map} _isJobQueueFilling - A map of jobQueues and if the 'jobQueueFilling' method is currently running for a given map. 'lockingOnTheFly' and 'jobQueueFilling' should not run concurrently for the same jobQueue. It can cause that lock limits aren't honored.
* @property {Array} _jobsToLock
* @property {Number} _defaultTimeout
* @property {Boolean} _defaultFailOnTimeout
*/
class Agenda extends EventEmitter {
_defaultConcurrency: any;
Expand All @@ -105,8 +99,6 @@ class Agenda extends EventEmitter {
_collection!: Collection;
_nextScanAt: any;
_processInterval: any;
_defaultTimeout: number;
_defaultFailOnTimeout: boolean;

cancel!: typeof cancel;
close!: typeof close;
Expand All @@ -133,8 +125,6 @@ class Agenda extends EventEmitter {
sort!: typeof sort;
start!: typeof start;
stop!: typeof stop;
defaultTimeout!: typeof defaultTimeout;
defaultFailOnTimeout!: typeof defaultFailOnTimeout;

/**
* Constructs a new Agenda object.
Expand Down Expand Up @@ -162,8 +152,6 @@ class Agenda extends EventEmitter {
this._lockedJobs = [];
this._jobQueue = new JobProcessingQueue();
this._defaultLockLifetime = config.defaultLockLifetime || 10 * 60 * 1000; // 10 minute default lockLifetime
this._defaultTimeout = config.defaultTimeout || 10 * 60 * 1000; // 10 minute default timeout
this._defaultFailOnTimeout = config.defaultFailOnTimeout || false;
this._sort = config.sort || { nextRunAt: 1, priority: -1 };
this._indices = {
name: 1,
Expand Down Expand Up @@ -229,7 +217,5 @@ Agenda.prototype.schedule = schedule;
Agenda.prototype.sort = sort;
Agenda.prototype.start = start;
Agenda.prototype.stop = stop;
Agenda.prototype.defaultTimeout = defaultTimeout;
Agenda.prototype.defaultFailOnTimeout = defaultFailOnTimeout;

export { Agenda };
17 changes: 1 addition & 16 deletions lib/job/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ export const run = async function (this: Job): Promise<Job> {
this.computeNextRunAt();
await this.save();

let timer: NodeJS.Timeout | null = null;
let finished = false;
const jobCallback = async (error?: Error, result?: unknown) => {
// We don't want to complete the job multiple times
Expand All @@ -34,16 +33,13 @@ export const run = async function (this: Job): Promise<Job> {
}

finished = true;
if (timer) {
clearInterval(timer);
}

if (error) {
this.fail(error);
} else {
this.attrs.lastFinishedAt = new Date();

if (this.attrs.shouldSaveResult && result) {
if(this.attrs.shouldSaveResult && result) {
this.attrs.result = result;
}
}
Expand Down Expand Up @@ -105,17 +101,6 @@ export const run = async function (this: Job): Promise<Job> {
throw new Error("Undefined job");
}

if (definition.shouldFailOnTimeout) {
timer = setInterval(() => {
const jobFinishDeadline = new Date(
(this.attrs.lastRunAt as Date).valueOf() + definition.timeout
);
if (jobFinishDeadline < new Date()) {
jobCallback(new Error(`${this.attrs.name} timeout`));
}
}, agenda._processEvery);
}

if (definition.fn.length === 2) {
debug(
"[%s:%s] process function being called",
Expand Down
18 changes: 0 additions & 18 deletions test/agenda.js
Original file line number Diff line number Diff line change
Expand Up @@ -230,24 +230,6 @@ describe("Agenda", () => {
expect(agenda._sort).to.eql({ nextRunAt: -1 });
});
});
describe("defaultTimeout", () => {
it("returns itself", () => {
expect(agenda.defaultTimeout(1000)).to.be(agenda);
});
it("sets the default timeout", () => {
agenda.defaultTimeout(9999);
expect(agenda._defaultTimeout).to.be(9999);
});
});
describe("defaultFailOnTimeout", () => {
it("returns itself", () => {
expect(agenda.defaultFailOnTimeout(false)).to.be(agenda);
});
it("sets the defaultFailOnTimeout option", () => {
agenda.defaultFailOnTimeout(true);
expect(agenda._defaultFailOnTimeout).to.be(true);
});
});
});

describe("job methods", () => {
Expand Down
36 changes: 9 additions & 27 deletions test/job.js
Original file line number Diff line number Diff line change
Expand Up @@ -621,24 +621,6 @@ describe("Job", () => {
const deletedJob = await agenda.jobs({ name: "failBoat3" });
expect(deletedJob).to.have.length(0);
});

it("handles timeout errors", async () => {
await agenda.processEvery(5);
job.attrs.name = "failBoat4";
await job.save();
agenda.define(
"failBoat4",
{ timeout: 15, failOnTimeout: true },
async (job, cb) => {
await delay(100);
cb();
}
);

await job.run();

expect(job.attrs.failReason).to.be("failBoat4 timeout");
});
});

describe("touch", () => {
Expand Down Expand Up @@ -1302,7 +1284,7 @@ describe("Job", () => {
expect(job.attrs.shouldSaveResult).to.be(true);
});
it("should set option via constructor", () => {
const job = new Job({ shouldSaveResult: true });
const job = new Job({shouldSaveResult: true});
expect(job.attrs.shouldSaveResult).to.be(true);
});
it("returns the job", () => {
Expand All @@ -1320,7 +1302,7 @@ describe("Job", () => {
await agenda.now("savedResultJob");
await delay(jobTimeout);
const result = await agenda.jobs({ name: "savedResultJob" });
expect(result[0].attrs.result).to.be(undefined);
expect(result[0].attrs.result).to.be(undefined)
});
it("should not save result if option is false", async () => {
agenda.define("savedResultJob", {shouldSaveResult: false}, (job, cb) => {
Expand All @@ -1330,7 +1312,7 @@ describe("Job", () => {
await agenda.now("savedResultJob");
await delay(jobTimeout);
const result = await agenda.jobs({ name: "savedResultJob" });
expect(result[0].attrs.result).to.be(undefined);
expect(result[0].attrs.result).to.be(undefined)
});
it("should save result if option is true", async () => {
agenda.define("savedResultJob", {shouldSaveResult: true}, (job, cb) => {
Expand All @@ -1353,7 +1335,7 @@ describe("Job", () => {
await agenda.now("savedResultJob");
await delay(jobTimeout);
const result = await agenda.jobs({ name: "savedResultJob" });
expect(result[0].attrs.result).to.be(undefined);
expect(result[0].attrs.result).to.be(undefined)
});
it("should not save result if option is false", async () => {
agenda.define("savedResultJob", {shouldSaveResult: false}, async (job) => {
Expand All @@ -1363,7 +1345,7 @@ describe("Job", () => {
await agenda.now("savedResultJob");
await delay(jobTimeout);
const result = await agenda.jobs({ name: "savedResultJob" });
expect(result[0].attrs.result).to.be(undefined);
expect(result[0].attrs.result).to.be(undefined)
});
it("should save result if option is true", async () => {
agenda.define("savedResultJob", {shouldSaveResult: true}, async (job) => {
Expand All @@ -1385,7 +1367,7 @@ describe("Job", () => {
await agenda.now("savedResultJob");
await delay(jobTimeout);
const result = await agenda.jobs({ name: "savedResultJob" });
expect(result[0].attrs.result).to.be(undefined);
expect(result[0].attrs.result).to.be(undefined)
});
it("should not save result if option is false", async () => {
agenda.define("savedResultJob", {shouldSaveResult: false}, (job) => {
Expand All @@ -1395,10 +1377,10 @@ describe("Job", () => {
await agenda.now("savedResultJob");
await delay(jobTimeout);
const result = await agenda.jobs({ name: "savedResultJob" });
expect(result[0].attrs.result).to.be(undefined);
expect(result[0].attrs.result).to.be(undefined)
});
it("should save result if option is true", async () => {
agenda.define("savedResultJob", { shouldSaveResult: true }, (job) => {
agenda.define("savedResultJob", {shouldSaveResult: true}, (job) => {
return "job-result";
});
await agenda.start();
Expand All @@ -1409,7 +1391,7 @@ describe("Job", () => {
});
});
});
});
})

describe("Integration Tests", () => {
describe(".every()", () => {
Expand Down

0 comments on commit 6ec531d

Please sign in to comment.