Node queuing jobs

Question

I have a MethodQueue class which enqueue method in queue array and execute them. The tricky part here is that I want to execute one job at a time and next job has to start after some x seconds after previous completed to avoid exceeding request within no time.

So I implemented MethodQueue class like this.

class MethodQueue {
    private queue: (() => Promise<any>)[] = [];
    private isExecuting = false;

    async enqueue(method: () => Promise<any>): Promise<void> {
        this.queue.push(method);
        if (!this.isExecuting) {
            this.executeNext();
        }
    }
    private async executeNext(): Promise<void> {
        if (this.isExecuting) return;
        this.isExecuting = true;
        const method = this.queue.shift();
        if (method) {
            await method();
            this.isExecuting = false;
            setTimeout(() => this.executeNext(), 1000);
        } else {
            this.isExecuting = false;
        }
    }
}

And I have SendQueue class with static methods that I use from other files.

export class SendQueue {
    private static methodQueue = new MethodQueue();

    public static async m1(param1: string, param2: number): Promise<string> {
        return new Promise<string>((resolve) => {
            SendQueue.methodQueue.enqueue(async () => {
                const result = await Send.m1(param1, param2);
                resolve(result);
            });
        });
    }

    public static async m2(param1: string): Promise<string> {
        return new Promise<string>((resolve) => {
            SendQueue.methodQueue.enqueue(async () => {
                const result = await Send.m2(param1);
                resolve(result);
            });
        });
    }

    public static async m3(param1: string, param2: boolean): Promise<string> {
        return new Promise<string>((resolve) => {
            SendQueue.methodQueue.enqueue(async () => {
                const result = await Send.m3(param1, param2);
                resolve(result);
            });
        });
    }
}

Finally I have Send class which has actual code

class Send {
    public static async m1(param1: string, _param2: number): Promise<string> {
        console.log(`Done ${param1}: ${new Date().getHours()}:${new Date().getMinutes()}:${new Date().getSeconds()}:${new Date().getMilliseconds()}`);
        return "result from m1";
    }

    public static async m2(param1: string): Promise<string> {
        console.log(`Done ${param1}: ${new Date().getHours()}:${new Date().getMinutes()}:${new Date().getSeconds()}:${new Date().getMilliseconds()}`);
        return "result from m2";
    }

    public static async m3(param1: string, _param3: boolean): Promise<string> {
        console.log(`Done ${param1}: ${new Date().getHours()}:${new Date().getMinutes()}:${new Date().getSeconds()}:${new Date().getMilliseconds()}`);
        return "result from m3";
    }
}

When I am executing with multiple request Its hit and miss with multiple awaits between calls

(async () => {
    SendQueue.m1("m1", 1);
    SendQueue.m2("m2");
    SendQueue.m3("m3", true);
    await new Promise((resolve) => setTimeout(resolve, 1000));
    SendQueue.m1("m4", 1);
    await new Promise((resolve) => setTimeout(resolve, 2000));
    SendQueue.m2("m5");
    await new Promise((resolve) => setTimeout(resolve, 2500));
    SendQueue.m3("m6", true);
    await new Promise((resolve) => setTimeout(resolve, 1000));
    SendQueue.m1("m7", 1);
    SendQueue.m2("m8");
    SendQueue.m3("m9", true);
    await new Promise((resolve) => setTimeout(resolve, 13000));
    SendQueue.m1("m10", 1);
    await new Promise((resolve) => setTimeout(resolve, 1000));
    SendQueue.m2("m11");
    SendQueue.m3("m12", true);
    await new Promise((resolve) => setTimeout(resolve, 1000));
    SendQueue.m1("m13", 1);
})();

Output:

What is the problem with my code?

Answer 1

I see a couple potential problems.

First off, you have no try/catch around the await method();. So, if that method rejects, you will never set this.isExecuting = false; back to false and you will not schedule the next method in the queue.

Second, you do the setTimeout(), but if before the timer fires, some other code calls enqueue(), then it will call executeNext() before that timer fires and then the timer will fire and call it again. It seems that executeNext() needs to know when a timer is already running and not do anything until the timer fires.

Here's a suggested fix for these issues (note I'm not a TypeScript person so you probably have to assign a type to the new timer instance variable):

class MethodQueue {
    private queue: (() => Promise<any>)[] = [];
    private isExecuting = false;
    private timer = null;

    async enqueue(method: () => Promise<any>): Promise<void> {
        this.queue.push(method);
        if (!this.isExecuting) {
            this.executeNext();
        }
    }
    private async executeNext(): Promise<void> {
        // don't run another method if we're already
        // running one or if one is scheduled to run
        if (this.isExecuting || this.timer) return;
        this.isExecuting = true;
        const method = this.queue.shift();
        if (method) {
            try {
                await method();
            } catch(e) {
                console.log(e);
            } finally {
                this.isExecuting = false;
            }
            this.timer = setTimeout(() => {
                this.executeNext();
                this.timer = null;
            }, 1000);
        } else {
            this.isExecuting = false;
        }
    }
}