Delete stat is incorrectly incremented when calling `keyv.iterator()` function with expired items.

[jorovipe97]

Describe the bug
Calling keyv.iterator() causes the stats.deletes counter to increment repeatedly when expired items exist in the store, even after those items have already been deleted.

In other words, iterating multiple times over a store with already-expired keys continues to increase the delete counter, despite no additional deletions actually occurring.

How To Reproduce

Minimal reproducible example:

import { Keyv } from 'keyv';

async function wait(ms) {
    return new Promise(resolve => setTimeout(resolve, ms));
}

const keyv = new Keyv({
    stats: true,
});

await keyv.set('foo', 'bar', 100);

// Get all items using iterator function
for await (const item of keyv.iterator()) {
    console.log('Item')
    console.log(item) // [ 'foo', 'bar' ]
}
console.log(keyv.stats.deletes) // 0

// No items expired yet, bug don't happen here.
await wait(200);
// Now there is one item expired

// Get all items using iterator
for await (const item of keyv.iterator()) {
    // All items are expired, it doesn't enter the loop
    console.log('Item')
    console.log(item)
}
console.log(keyv.stats.deletes) // 1 (Since item is expired, it's deleted, this is expected)

// Get all items using iterator, again
for await (const item of keyv.iterator()) {
    // All items are expired, it doesn't enter the loop
    console.log('Item')
    console.log(item)
}
console.log(keyv.stats.deletes) // 2 (Item was already deleted, so it's not deleted again, this is unexpected)

As shown above, repeated calls to iterator() cause stats.deletes to increase indefinitely for the same expired item.

Analysis

During iteration, expired entries trigger a call to .delete(key). However, there is no check to ensure the item still exists in the store, nor does the delete operation verify that a deletion actually occurred before incrementing the stats counter.

Option 1: Skip non-existent data in generateIterator

Add a guard in generateIterator to skip entries that deserialize to null or undefined, preventing repeated delete attempts for already-removed items.

Relevant code:
https://github.com/jaredwray/keyv/blob/main/packages/keyv/src/index.ts#L506

generateIterator(iterator: IteratorFunction): IteratorFunction {
	// biome-ignore lint/suspicious/noExplicitAny: type format
	const function_: IteratorFunction = async function* (this: any) {
		for await (const [key, raw] of typeof iterator === "function"
			? iterator(this._store.namespace)
			: iterator) {
			const data = await this.deserializeData(raw);

+			if (data === undefined || data == null) {
+				continue;
+			}

			if (
				this._useKeyPrefix &&
				this._store.namespace &&
				!key.includes(this._store.namespace)
			) {
				continue;
			}

			if (typeof data.expires === "number" && Date.now() > data.expires) {
				this.delete(key);
				continue;
			}

			yield [this._getKeyUnprefix(key), data.value];
		}
	};

	return function_.bind(this);
}

This avoids calling .delete() when there is no valid data to process.

Option 2: Only increment delete stats when a deletion actually occurs

Update the .delete() method so that stats.deletes is only incremented when the underlying store confirms a successful deletion.

Relevant code:
https://github.com/jaredwray/keyv/blob/main/packages/keyv/src/index.ts#L1036

public async delete(key: string | string[]): Promise<boolean> {
    const { store } = this.opts;
    if (Array.isArray(key)) {
        return this.deleteMany(key);
    }

    const keyPrefixed = this._getKeyPrefix(key);
    this.hooks.trigger(KeyvHooks.PRE_DELETE, { key: keyPrefixed });

    let result = true;

    try {
        const value = await store.delete(keyPrefixed);

        /* v8 ignore next -- @preserve */
        if (typeof value === "boolean") {
            result = value;
        }
    } catch (error) {
        result = false;
        this.emit("error", error);
        if (this._throwOnErrors) {
            throw error;
        }
    }

    this.hooks.trigger(KeyvHooks.POST_DELETE, {
        key: keyPrefixed,
        value: result,
    });

+    // We only increment the delete stat if the delete was successful
+    if (result) {
+       this.stats.delete();
+    }

    return result;
}

Closing Thoughs

Personally, I lean toward Option 1, as it limits the behavioral change to the iterator logic and avoids altering .delete(), which is used in multiple code paths. That said, I’m happy to hear your thoughs on this and let me know if you’d like me to open a PR for either approach.

[jaredwray]

@jorovipe97 - thanks for finding this bug. Lets go with Option 1

[jorovipe97]

Hi @jaredwray, thanks for the quick response!

I forked the repo and debugged the issue in more detail (by running the code, not just reading it), and it looks like the root cause is slightly different from what I initially described.

The issue appears to be in the _getKeyPrefix function:
https://github.com/jaredwray/keyv/blob/main/packages/keyv/src/index.ts#L527

What's Happening

Inside generateIterator, when an item is expired, we call .delete(key) using the key yielded by the store iterator:

https://github.com/jaredwray/keyv/blob/main/packages/keyv/src/index.ts#L506

if (typeof data.expires === "number" && Date.now() > data.expires) {
	this.delete(key); // This key already has the namespace, eg: "keyv:foo"
	continue;
}

At this point, key is already prefixed with the namespace (for example, keyv:foo).

However, inside .delete(), _getKeyPrefix() is called again, which results in the namespace being applied a second time:

keyv:keyv:foo

Because the underlying store never had a key with that double prefix, the delete operation doesn’t actually remove anything but the iterator still treats the entry as expired on subsequent iterations, which explains why stats.deletes keeps increasing.

Proposed solutions

I see two possible fixes, with similar tradeoffs to the previous options we discussed.

Option 1: Unprefix the key before calling .delete() in generateIterator

This keeps the change localized to the iterator logic and ensures .delete() always receives an unprefixed key.

generateIterator(iterator: IteratorFunction): IteratorFunction {
	// biome-ignore lint/suspicious/noExplicitAny: type format
	const function_: IteratorFunction = async function* (this: any) {
		for await (const [key, raw] of typeof iterator === "function"
			? iterator(this._store.namespace)
			: iterator) {
			const data = await this.deserializeData(raw);

			if (
				this._useKeyPrefix &&
				this._store.namespace &&
				!key.includes(this._store.namespace)
			) {
				continue;
			}

			if (typeof data.expires === "number" && Date.now() > data.expires) {
-				this.delete(key);
+                               this.delete(this._getKeyUnprefix(key));
				continue;
			}

			yield [this._getKeyUnprefix(key), data.value];
		}
	};

	return function_.bind(this);
}

Option 2: Make _getKeyPrefix idempotent

Prevent _getKeyPrefix() from re-applying the namespace if the key is already prefixed.

_getKeyPrefix(key: string): string {
    if (!this._useKeyPrefix) {
        return key;
    }

    if (!this._namespace) {
        return key;
    }

+    // If the key is already prefixed, return it as is
+    if (key.startsWith(this._namespace)) {
+        return key;
+    }

    return `${this._namespace}:${key}`;
}

Closing Thoughs

Given your earlier preference for option 1, I think unprefixing the key inside generateIterator aligns well with that approach and avoids changing _getKeyPrefix() behavior globally. That said, making _getKeyPrefix() idempotent could also be beneficial defensively.

Let me know your preference and I’ll open a PR accordingly.

[jaredwray]

@jorovipe97 - We can attempt this change and run it via the tests if you want to do this quick fix. If not, we should hold off until Keyv v6 which will release in 2026 and change how namespace works

[jorovipe97]

Sure, I will do!

[jorovipe97]

@jaredwray - Here is the PR #1772 all test are passing