Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Promise rejections not cleared immediately from cache #117

Open
lukashavrlant opened this issue Mar 1, 2021 · 2 comments
Open

Promise rejections not cleared immediately from cache #117

lukashavrlant opened this issue Mar 1, 2021 · 2 comments
Assignees
Labels
Milestone

Comments

@lukashavrlant
Copy link

lukashavrlant commented Mar 1, 2021

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
Copy link
Owner

medikoo commented Mar 1, 2021

@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.

@medikoo medikoo added the bug label Mar 1, 2021
@medikoo medikoo self-assigned this Mar 1, 2021
@medikoo medikoo added this to the v1 milestone Mar 1, 2021
@lukashavrlant
Copy link
Author

@medikoo OK, thank you for the explanation 🙂

@medikoo medikoo changed the title The library keeps rejected promises in the cache Promise rejections not cleared immediately from cache Mar 1, 2021
@medikoo medikoo mentioned this issue Nov 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants