updateSettings clobbers reservoir increment logic

[Cireo]

It appears that updateSettings causes reservoir logic to break, this appears to be true whether or not anything is updated, whether it is run before anything is scheduled or not. The only updates that take place are ones that set reservoir, which are happily consumed (but never refreshed).

bug.js

// updateSettings breaks reservoirIncrease logic (reservoir value is updated)
const Bottleneck = require('bottleneck');

async function foo() {
  const limiter = new Bottleneck({
    reservoir: 5,
    reservoirIncreaseInterval: 1000,
    reservoirIncreaseAmount: 1,
    minTime: 100
  });
  // Takes 5s to log 1->10
  const items = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10];
  await Promise.all(
    items.map((x) => limiter.schedule(() => console.log(`hi ${x}`)))
  );
  limiter.updateSettings({ reservoir: 5 });
  // Takes .5s to hang/die on 5
  await Promise.all(
    items.map((x) => limiter.schedule(() => console.log(`hi ${x}`)))
  );
}

foo();

Running:

node bottleneckBug.js  
hi 1
hi 2
hi 3
hi 4
hi 5
hi 6
hi 7
hi 8
hi 9
hi 10
hi 1
hi 2
hi 3
hi 4
hi 5

[eugene-kim]

Hello @SGrondin , do you have any more information on this? Thank you

[kdeclerck]

We're also seeing this issue.

[louneskmt]

Facing this issue as well.

[fkoemep]

I spent like 5 hours trying to debug the code and ended up here facing this very same issue.

[fsq]

Facing same issue. Error seems related to heartbeat handling in lib/LocalDatastore.js: _startHeartbeat() creates a heartbeat closure when a Bottleneck is created. This closure is handling reservoir refresh.

When updateSettings, a new Bottleneck is generated, but somehow the old heartbeat callback is still running, and reading the old storeOptions instead of reading the updated one.

Removing this.heartbeat==null in the if statement to create a new closure fixes my case, altho not sure if this is the correct way. Not a JS expert, would love to see this fixed.

[Cireo]

Nice debugging! I wonder if there is a binding fix