fix(KeyValueStore): big buffers should not crash

[vladfrangu]

Closes #1732
Closes #1710

Simple fix really, tell shapeshift inputs can be buffers too (I'll also see if we can add a generic "just check that its an object" in shapeshift for times like these)

[corford]

Simple fix really, tell shapeshift inputs can be buffers too (I'll also see if we can add a generic "just check that its an object" in shapeshift for times like these)

@vladfrangu Have you confirmed this also fixes the performance issues? In our issue (#1710) we experienced horrendous perf problems from shapeshift handling Buffers (to the extent it started blocking Node's event loop). Asking as I see the buffer test in this PR is also showing perf issues.

[vladfrangu]

Its most likely that you're having those performance issues due to shapeshift interpreting the buffer as an object, and trying to parse its fields (given that a buffer is basically an array of bytes, you can guess what happens when size is big)

This PR should shortcircuit that check since any buffer is an instance of a buffer, and thus speed up tremendously... That said, if you can test it yourself that would be awesome! I'll try it out locally too)

[Umisyus]

Thanks, glad you worked out the solution quickly. Have a gr8 year everyone!

[corford]

@vladfrangu @B4nan Some bad news: we've just tested this (via Crawle v3.3.0) and the issue persists. It's slightly less pervasive than prior to this PR but still present (affects about 40% of our spiders, with same symptom as before: pegged event loop, causing degraded request throughput and in many cases spider crash).

Workaround for us has been to apply the following patch to node_modules/@crawlee/memory-storage/resource-clients/key-value-store.js:

@@ -192,11 +192,13 @@
         return record;
     }
     async setRecord(record) {
-        shapeshift_1.s.object({
-            key: shapeshift_1.s.string.lengthGreaterThan(0),
-            value: shapeshift_1.s.union(shapeshift_1.s.null, shapeshift_1.s.string, shapeshift_1.s.number, shapeshift_1.s.instance(Buffer), shapeshift_1.s.object({}).passthrough),
-            contentType: shapeshift_1.s.string.lengthGreaterThan(0).optional,
-        }).parse(record);
+        if (!Buffer.isBuffer(record?.value)) {
+            shapeshift_1.s.object({
+                key: shapeshift_1.s.string.lengthGreaterThan(0),
+                value: shapeshift_1.s.union(shapeshift_1.s.null, shapeshift_1.s.string, shapeshift_1.s.number, shapeshift_1.s.instance(Buffer), shapeshift_1.s.object({}).passthrough),
+                contentType: shapeshift_1.s.string.lengthGreaterThan(0).optional,
+            }).parse(record);
+        }
         // Check by id
         const existingStoreById = await (0, cache_helpers_1.findOrCacheKeyValueStoreByPossibleId)(this.client, this.name ?? this.id);
         if (!existingStoreById) {
@@ -204,8 +206,8 @@
         }
         const { key } = record;
         let { value, contentType } = record;
-        const valueIsStream = (0, utils_1.isStream)(value);
-        const isValueStreamOrBuffer = valueIsStream || (0, utils_1.isBuffer)(value);
+        const valueIsStream = Buffer.isBuffer(value) ? false : (0, utils_1.isStream)(value);
+        const isValueStreamOrBuffer = valueIsStream || Buffer.isBuffer(value);
         // To allow saving Objects to JSON without providing content type
         if (!contentType) {
             if (isValueStreamOrBuffer)

Applying the patch immediately resolved the issue.