Buffer bug in @crawlee/memory-storage triggering eventLoop overloads and Node heap crashes

[corford]

Which package is this bug report for? If unsure which one to select, leave blank

@crawlee/memory-storage

Issue description

The setRecord() function in @crawlee/memory-storage/resource-clients/key-value-store.js uses shapeshift to enforce the types of the record object. If a Buffer is passed through this, it can sometimes cause shapeshift's .parse() to hang for upwards of 10 seconds before returning (sometimes it blows the heap and kills Node).

This has caused havoc with many of our spiders after migrating to Crawlee v3.1.3 (from Apify 2.3.2). One symptom is a pegged eventLoop, the other is heap allocation errors that eventually kill the Node main process.

Many but not all of our spiders were affected which leads us to believe it depends heavily on the raw contents of the Buffer as to what pathological behaviour shapeshift exhibits when trying to parse it. Preventing shapeshift from parsing Buffers fixed all the issues immediately.

Our workaround looks like this:

    async setRecord(record: storage.KeyValueStoreRecord): Promise {
        if (!Buffer.isBuffer(record?.value)) {
            s.object({
                key: s.string.lengthGreaterThan(0),
                value: s.union(s.null, s.string, s.number, s.object({}).passthrough),
                contentType: s.string.lengthGreaterThan(0).optional,
            }).parse(record);
        }

        // Check by id
        const existingStoreById = await findOrCacheKeyValueStoreByPossibleId(this.client, this.name ?? this.id);

        if (!existingStoreById) {
            this.throwOnNonExisting(StorageTypes.KeyValueStore);
        }

        const { key } = record;
        let { value, contentType } = record;

        const valueIsStream = Buffer.isBuffer(value) ? false : isStream(value);

       ...

Original source is here:

crawlee/packages/memory-storage/src/resource-clients/key-value-store.ts

Line 211 in 5ff04fa

async setRecord(record: storage.KeyValueStoreRecord): Promise<void> {

Code sample

// Note the `body` and `contentType` vars come from https://crawlee.dev/api/cheerio-crawler/interface/CheerioCrawlingContext

import { v4 as uuidv4 } from 'uuid';

const encoding = Buffer.isBuffer(body) && contentType?.encoding
      ? contentType.encoding
      : 'utf8';

const resourceBuffer = Buffer.isBuffer(body)
    ? body
    : Buffer.from(body, encoding);

const resourceWal = await KeyValueStore.open('wal-resource');
await resourceWal.setValue(
    uuidv4(),
    resourceBuffer,
    { contentType: 'application/octet-stream' },
);

Package version

@crawlee/memory-storage@3.1.2

Node.js version

18.11.0

Operating system

Ubuntu focal

Apify platform

• Tick me if you encountered this issue on the Apify platform

I have tested this on the next release

No response

Other context

No response

[B4nan]

cc @vladfrangu I guess something to fix in shapeshift?

[corford]

Looking at the shapeshift README, I couldn't see a single mention of Buffer anywhere, suggesting it's not a supported or anticipated type. So even if the performance issue was solved, not sure how Buffers would be treated by s.union(s.null, s.string, s.number, s.object({}).passthrough) ?