nodeshift / opossum

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

Cannot read properties of undefined (reading 'then') #893

Open cedrick-ah opened 1 week ago

cedrick-ah commented 1 week ago

Node.js Version:

v18.20.4

Operating System:

Debian 5.10.226-1 (2024-10-03) x86_64 GNU/Linux

Steps to Produce Error:

When provided a function that does not return a promise to the circuit breaker:

function get() {
  fetch(`http://localhost:8000`);
}

const breaker = new CircuitBreaker(get);
await breaker.fire();

Instead of a valid asynchronous function:

function get() {
  return fetch(`http://localhost:8000`);
}

const breaker = new CircuitBreaker(get);
await breaker.fire();

Result:

TypeError: Cannot read properties of undefined (reading 'then')
    at /home/zotcha/Documents/dev/workspace/contrib/opossum/lib/circuit.js:686:27
    at new Promise (<anonymous>)
    at CircuitBreaker.call (/home/zotcha/Documents/dev/workspace/contrib/opossum/lib/circuit.js:653:12)
    at CircuitBreaker.fire (/home/zotcha/Documents/dev/workspace/contrib/opossum/lib/circuit.js:564:17)
    at /home/zotcha/Documents/dev/workspace/contrib/opossum/test.js:21:19
    at Object.<anonymous> (/home/zotcha/Documents/dev/workspace/contrib/opossum/test.js:25:3)
    at Module._compile (node:internal/modules/cjs/loader:1364:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1422:10)
    at Module.load (node:internal/modules/cjs/loader:1203:32)
    at Module._load (node:internal/modules/cjs/loader:1019:12)

The problem

Opossum is a Node.js circuit breaker that executes asynchronous functions and monitors their execution status. When it happens that the function provided to Opossum does not meet the conditions of an asynchronous function or that the execution result cannot be obtained, this type of error occurs. To fix this, we can check if the result of executing the asynchronous method exists.

The fix

Simple fix would be to check if the result of the call of the provided action is not undefined:

const result = this.action.apply(context, args);

if (!result) throw new TypeError("The action provided can not be resolved.");  // The fix

const promise = typeof result.then === "function"
    ? result
    : Promise.resolve(result);
lholmquist commented 1 week ago

In that example, if adding return fixes things, then i'm not sure why you wouldn't just add it. Is there a reason why you wouldn't?

cedrick-ah commented 1 week ago

This is not me. Anyone would pass by inattention this kind of action. I thought this would be more appropriate to report this way than just leaving an error uncaught.