Skip to content

fix(memory-storage): use platform basename in key-value store warning#3864

Merged
barjin merged 2 commits into
apify:masterfrom
anxkhn:patch-15
Jul 16, 2026
Merged

fix(memory-storage): use platform basename in key-value store warning#3864
barjin merged 2 commits into
apify:masterfrom
anxkhn:patch-15

Conversation

@anxkhn

@anxkhn anxkhn commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

What & why

KeyValueFileSystemEntry.get() in @crawlee/memory-storage imports basename
from node:path/win32 (the Windows-only path module) and uses it to build a
warning when a key-value entry is read without a file extension:

// packages/memory-storage/src/fs/key-value-store/fs.ts
import { dirname } from 'node:path';
import { basename } from 'node:path/win32';
// ...
memoryStorageLog.warning(
    [
        `Key-value entry "${this.rawRecord.key}" for store ${basename(
            this.storeDirectory,
        )} does not have a file extension, assuming it as text.`,
        // ...
    ].join('\n'),
);

path.win32.basename always treats \ as a path separator, even on POSIX. A
backslash is a legal filename character on Linux and macOS, so if the key-value
store directory's leaf name contains one, the warning shows a wrongly truncated
store name. For a store at .../my\weird\store, the message says for store store
instead of for store my\weird\store. Ordinary POSIX paths use /, which
path.win32.basename also handles, which is why this went unnoticed.

This is the only module under packages/*/src that imports from
node:path/win32; every sibling, including this same file's dirname and the
dataset and request-queue fs.ts modules, imports the platform-aware
node:path. The fix merges basename into the existing node:path import and
drops the node:path/win32 line, so the platform-correct basename is used:

import { basename, dirname } from 'node:path';

No logic changes; only the warning text is affected (the value is never used for
file I/O, real paths go through resolveWithinDirectory).

Added a regression test (test/core/storages/key_value_store_fs.test.ts) that
drives get() into the extension-less fallback with a store directory whose leaf
name contains a backslash, spies on the warning, and asserts the full leaf name is
present. It fails before the change (for store store) and passes after.

anxkhn and others added 2 commits July 16, 2026 11:54
The extension-less fallback in KeyValueFileSystemEntry.get() imported
basename from node:path/win32, so on POSIX it treated a backslash in the
store directory name as a path separator and truncated the store name in
its warning message. Import basename from node:path like the sibling
dataset and request-queue modules so the platform-correct basename is used.

Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
@barjin
barjin merged commit dcefe01 into apify:master Jul 16, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants