get() Doesn't Get Called When Multiple Handlers Registered
Brief Description of the Bug
When multiple handlers are registered, the get method is not called correctly in sequence if the first handler does not have a value for that key.
Severity
Major
Frequency of Occurrence
Always.
Steps to Reproduce
- Register multiple handlers
- Notice that the
getmethod on the second is never called, even when the first cache starts out empty; page rebuilds are done instead.
Expected vs. Actual Behavior
I expect the get method to be called on every handler in order until a value for the key was retrieved or all handlers were tried, at which point the page should be rebuilt.
Environment:
- OS: MacOS
- Node.js version: 20.12.2
-
@neshca/cache-handlerversion: 1.3.1 -
nextversion: 14.2.3
Attempted Solutions or Workarounds
I was able to create a single handler that did the merge manually, but this should not be required.
Additional context
I think that the problem arises here, in the core multi-handler logic:
for await (const handler of handlersList) {
try {
let cacheHandlerValue = await handler.get(key, meta);
if (
cacheHandlerValue?.lifespan &&
cacheHandlerValue.lifespan.expireAt < Math.floor(Date.now() / 1000)
) {
cacheHandlerValue = null;
await handler.delete?.(key).catch((deleteError) => {
if (CacheHandler.#debug) {
console.warn(
`Handler "${handler.name}" failed to delete value for key "${key}".`,
deleteError,
);
}
});
}
if (CacheHandler.#debug) {
console.info(`get from "${handler.name}"`, key, Boolean(cacheHandlerValue));
}
return cacheHandlerValue;
} catch (error) {
if (CacheHandler.#debug) {
console.warn(`Handler "${handler.name}" failed to get value for key "${key}".`, error);
}
}
}
By examining the logs I was able to determine that the first handler I registered, the redis-strings handler included in this distribution, was not returning the value: Boolean(cacheHandlerValue) was false. However, it looks like the code above assumes that handlers will throw on failure to find the key, not return a falsey value (null here IIRC). So you hit the immediate return, and the second handler's get method is never called. I suspect that this could be fixed simply by continueing on falsey values, rather than returning.
@gchallen hi there! Sorry for the delay. The behavior you're observing is not a bug but a deliberate design choice. Let me explain.
When using multiple handlers, we want the data to be consistent across all cache stores. In this case, there's no point in retrieving the data from other handlers if it wasn't found in the first one.
However, there may be situations where it's necessary to go through multiple handlers. As you have already done, creating a custom handler is always the best approach.
That said, there is already a discussion on this topic, which you can find here. I'm ready to discuss it there actively.
And it might be worth considering adding an option to choose the strategy for traversing handlers.