search

nodeshift / opossum

Node.js circuit breaker - fails fast ⚡️
https://nodeshift.dev/opossum/
Apache License 2.0
1.33k stars 107 forks source link

Setting some options to `0` doesn't work (it still uses defaults) #824

Closed ollelindeman closed 1 year ago

ollelindeman commented 1 year ago

Problem

I noticed when writing tests that setting some options to 0 is not working as I expected.

An example is errorThresholdPercentage, which I wanted to set to 0 in a test to make it fail immediately. But setting it to 0 will result in the default value 50 being used!

Is that the expected behaviour?

The reason why this happens is in how the defaults are set in the CircuitBreaker constructor.

this.options.errorThresholdPercentage =
      options.errorThresholdPercentage || 50;

https://github.com/nodeshift/opossum/blob/main/lib/circuit.js#L157C5-L158C46

Since 0 is considered false it will take the 50.

Similar behaviour is also present for the options resetTimeout, rollingCountTimeout, rollingCountBuckets.

lholmquist commented 1 year ago

thanks for finding that!! we should probably add some tests to make sure we can set these values to 0

lholmquist commented 1 year ago

just released 8.1.2 on npm