Fix blocking behaviour of the RateLimit.apply function

[jdalrymple]

Description

Due to the use of the setTimeout, the event queue remains active even after the node process if finished running.

Proposal

Modify the use of the RateLimit functionality, such that this doesnt occur. I've tried a few things, but havent had the best luck. Suggestions welcome!

See jdalrymple/gitbeaker#3493 for original issue

Checklist

• I have checked that this is not a duplicate issue.
• I have read the documentation.

[jdalrymple]

@remorses not sure if you have any suggestions, but i could use another set of eyes.

[jdalrymple]

import { RateLimit } from "../sema4/dist/index.mjs";

const start = process.hrtime();

process.on("exit", (code) => {
  const hrt = process.hrtime(start);
  const elapsed = (hrt[0] * 1000 + hrt[1] / 1e6) / 1000;

  console.log(`cost time: ${elapsed}ms`);
});

// Still gets stuck holding event queue active
async function sema4() {
  const rl = new RateLimit(1, { interval: 10000 });

  for(let a = 0; a<5; a++) {
    const hrt1 = process.hrtime(start);
    const elapsed1 = (hrt1[0] * 1000 + hrt1[1] / 1e6) / 1000;
    console.log(`b4 ${elapsed1}: .`)

    await rl.apply()

    const hrt2 = process.hrtime(start);
    const elapsed2 = (hrt2[0] * 1000 + hrt2[1] / 1e6) / 1000;
    console.log(`af ${elapsed2}: .`)
    console.log()
  }

  const hrt = process.hrtime(start);
  const elapsed = (hrt[0] * 1000 + hrt[1] / 1e6) / 1000;

  console.log('Time taken to run: ' + elapsed)
  console.log('Finished')
}


sema4()

Output:

af 0.007123369: .

b4 0.007276779999999999: .
af 10.009045545000001: .

b4 10.009536919: .
af 20.016119750999998: .

b4 20.016420011999998: .
af 30.023008505: .

b4 30.023649946000003: .
af 40.03290881: .

Time taken to run: 40.033329427999995
Finished
cost time: 50.04298276ms

[remorses]

The problem is that there are still pending timeouts when you try to finish the process, if you don’t care if these functions complete you could pass an AbortController to RateLimit, split the setTimeout in many smaller ones that run sequentially, checking if the signal was aborted, if yes stop waiting.

[remorses]

Using an abort controller instead of process.on(exit) let’s you use it in non node environments and is more flexible

[jdalrymple]

The problem is that there are still pending timeouts when you try to finish the process, if you don’t care if these functions complete you could pass an AbortController to RateLimit, split the setTimeout in many smaller ones that run sequentially, checking if the signal was aborted, if yes stop waiting.

I thought about this, also unreffing the timeout, but that instantly kills the process after the first await. Ideally the rate limit would pause once hit until the time has elapsed and the continue. Using the abort controller requires external interaction to halt the setTimeout.

I've tested with applying the setTimeout before the acquire function to not call it until its needed like:

  async apply() {
    if (this.acquireTimes.length >0) {
      const now = Date.now()
      
      // Check if any times have expired
      const canBeCleared = this.acquireTimes.filter(t => now-t >= this.delay)
    
      canBeCleared.forEach(() => {
        this.sema.release()
        this.acquireTimes.shift()
      })
    
      // if nothing can be immediately cleared, wait until one can
      if (canBeCleared.length == 0) {
        await new Promise((resolve) => setTimeout(() => resolve(this.sema.release())), this.delay-(now-this.acquireTimes[0]))
      }
    }
    
    await this.sema.acquire()
    
    this.acquireTimes.push(Date.now())
}

But i seemed to be off the mark somewhere.