medikoo / memoizee

Complete memoize/cache solution for JavaScript
ISC License
1.73k stars 61 forks source link

Promise rejections not cleared immediately from cache #117

Open lukashavrlant opened 3 years ago

lukashavrlant commented 3 years ago

Hi, we tried to use memoizee to memoize a promise function. We used promise: true but it still keeps remembering rejected promises in the cache. I wrote a simple piece of code that demonstrates it. The functionToBeMemoized function rejects when called for the first time and it resolves when called for the second time. Then I memoized it and called the memoized function twice -- I got the error both of the times. As far as I understand it, I should get real value when called the memoized function for the second time as the rejected promise is not supposed to be cached. I tried promise-memoize library which has the same API and it behaves as expected, it resolves a value when called for the second time.

Code that simulates it (I used node v14.15.1 and memoizee@0.4.15):

const memoize = require("memoizee");

let callCount = 0;

// reject when called for the first time
// resolve when called for the second time
const functionToBeMemoized = function(a) {
    console.log(`callCount: ${callCount}`);

    return new Promise(function(resolve, reject) {
        if (callCount === 0) {
            callCount++;
            return reject(new Error(`error number ${callCount}`));
        } else {
            return resolve(a);
        }
    });
};

const memoized = memoize(functionToBeMemoized, {promise: true});

async function run() {
    await memoized(3).then(x => console.log(`result: ${x}`)).catch(e => console.error(e.message));
    await memoized(3).then(x => console.log(`result: ${x}`)).catch(e => console.error(e.message));
}

run();

/*
actual output:

callCount: 0
error number 1
error number 1

expected output:

callCount: 0
error number 1
callCount: 1
result: 3
*/
medikoo commented 3 years ago

@lukashavrlant thanks for report.

In this given case, it's because memoizee did not manage to reset the cache before this second invocation is issued (note that it's issued immediately after first resolved).

e.g. if you add one tick gap after first invocation resolves but before next one is made, you'd see expected result:

    await memoized(3)
        .then((x) => console.log(`result: ${x}`))
        .catch((e) => console.error(e.message));
    await new Promise((resolve) => process.nextTick(resolve));
    await memoized(3)
        .then((x) => console.log(`result: ${x}`))
        .catch((e) => console.error(e.message));

Delay in deleting the cache can be attributed to fact that memoizee internally attempts to escape the promise chain (to avoid errors swallowing effect), and that puts promise result processing one tick further.

This design will be questioned when working on v1, so it might be that in v1 it'll be improved, and you'll see cache being cleared on time. For that reason I'll keep this issue open.

lukashavrlant commented 3 years ago

@medikoo OK, thank you for the explanation 🙂