Node: warp with try-catch the rust call and tests (valkey-io#2182)

* warp with try-catch the rust call and tests * add test --------- Signed-off-by: Adar Ovadia <[email protected]> Co-authored-by: Adar Ovadia <[email protected]>
Bit-Quill · Aug 26, 2024 · e1811c4 · e1811c4
1 parent 2276fd2
commit e1811c4
Show file tree

Hide file tree

Showing 4 changed files with 99 additions and 17 deletions.
diff --git a/node/src/BaseClient.ts b/node/src/BaseClient.ts
@@ -680,23 +680,32 @@ export class BaseClient {
             const callbackIndex = this.getCallbackIndex();
             this.promiseCallbackFunctions[callbackIndex] = [
                 (resolveAns: T) => {
-                    if (resolveAns instanceof PointerResponse) {
-                        if (typeof resolveAns === "number") {
-                            resolveAns = valueFromSplitPointer(
-                                0,
-                                resolveAns,
-                                stringDecoder,
-                            ) as T;
-                        } else {
-                            resolveAns = valueFromSplitPointer(
-                                resolveAns.high!,
-                                resolveAns.low!,
-                                stringDecoder,
-                            ) as T;
+                    try {
+                        if (resolveAns instanceof PointerResponse) {
+                            if (typeof resolveAns === "number") {
+                                resolveAns = valueFromSplitPointer(
+                                    0,
+                                    resolveAns,
+                                    stringDecoder,
+                                ) as T;
+                            } else {
+                                resolveAns = valueFromSplitPointer(
+                                    resolveAns.high!,
+                                    resolveAns.low!,
+                                    stringDecoder,
+                                ) as T;
+                            }
                         }
-                    }
 
-                    resolve(resolveAns);
+                        resolve(resolveAns);
+                    } catch (err) {
+                        Logger.log(
+                            "error",
+                            "Decoder",
+                            `Decoding error: '${err}'`,
+                        );
+                        reject(err);
+                    }
                 },
                 reject,
             ];

diff --git a/node/tests/GlideClient.test.ts b/node/tests/GlideClient.test.ts
@@ -32,6 +32,7 @@ import {
     checkFunctionStatsResponse,
     convertStringArrayToBuffer,
     createLuaLibWithLongRunningFunction,
+    DumpAndRestureTest,
     encodableTransactionTest,
     encodedTransactionTest,
     flushAndCloseClient,
@@ -242,6 +243,38 @@ describe("GlideClient", () => {
         },
     );
 
+    it.each([ProtocolVersion.RESP2, ProtocolVersion.RESP3])(
+        `dump and restore transactions_%p`,
+        async (protocol) => {
+            client = await GlideClient.createClient(
+                getClientConfigurationOption(cluster.getAddresses(), protocol),
+            );
+            const bytesTransaction = new Transaction();
+            const expectedBytesRes = await DumpAndRestureTest(
+                bytesTransaction,
+                Buffer.from("value"),
+            );
+            bytesTransaction.select(0);
+            const result = await client.exec(bytesTransaction, Decoder.Bytes);
+            expectedBytesRes.push(["select(0)", "OK"]);
+
+            validateTransactionResponse(result, expectedBytesRes);
+
+            const stringTransaction = new Transaction();
+            await DumpAndRestureTest(stringTransaction, "value");
+            stringTransaction.select(0);
+
+            // Since DUMP gets binary results, we cannot use the string decoder here, so we expected to get an error.
+            await expect(
+                client.exec(stringTransaction, Decoder.String),
+            ).rejects.toThrowError(
+                "invalid utf-8 sequence of 1 bytes from index 9",
+            );
+
+            client.close();
+        },
+    );
+
     it.each([ProtocolVersion.RESP2, ProtocolVersion.RESP3])(
         `can send transaction with default string decoder_%p`,
         async (protocol) => {

diff --git a/node/tests/GlideClusterClient.test.ts b/node/tests/GlideClusterClient.test.ts
@@ -239,11 +239,14 @@ describe("GlideClusterClient", () => {
             const valueEncoded = Buffer.from(value);
             expect(await client.set(key, value)).toEqual("OK");
             // Since DUMP gets binary results, we cannot use the default decoder (string) here, so we expected to get an error.
-            // TODO: fix custom command with unmatch decoder to return an error: https://github.com/valkey-io/valkey-glide/issues/2119
-            // expect(await client.customCommand(["DUMP", key])).toThrowError();
+            await expect(client.customCommand(["DUMP", key])).rejects.toThrow(
+                "invalid utf-8 sequence of 1 bytes from index 9",
+            );
+
             const dumpResult = await client.customCommand(["DUMP", key], {
                 decoder: Decoder.Bytes,
             });
+
             expect(await client.del([key])).toEqual(1);
 
             if (dumpResult instanceof Buffer) {

diff --git a/node/tests/TestUtilities.ts b/node/tests/TestUtilities.ts
@@ -602,6 +602,43 @@ export async function encodedTransactionTest(
     return responseData;
 }
 
+/** Populates a transaction with dump and restore commands
+ *
+ * @param baseTransaction - A transaction
+ * @param valueResponse - Represents the encoded response of "value" to compare
+ * @returns Array of tuples, where first element is a test name/description, second - expected return value.
+ */
+export async function DumpAndRestureTest(
+    baseTransaction: Transaction,
+    valueResponse: GlideString,
+): Promise<[string, ReturnType][]> {
+    const key = "dumpKey";
+    const dumpResult = Buffer.from([
+        0, 5, 118, 97, 108, 117, 101, 11, 0, 232, 41, 124, 75, 60, 53, 114, 231,
+    ]);
+    const value = "value";
+    // array of tuples - first element is test name/description, second - expected return value
+    const responseData: [string, ReturnType][] = [];
+
+    baseTransaction.set(key, value);
+    responseData.push(["set(key, value)", "OK"]);
+    baseTransaction.customCommand(["DUMP", key]);
+    responseData.push(['customCommand(["DUMP", key])', dumpResult]);
+    baseTransaction.del([key]);
+    responseData.push(["del(key)", 1]);
+    baseTransaction.get(key);
+    responseData.push(["get(key)", null]);
+    baseTransaction.customCommand(["RESTORE", key, "0", dumpResult]);
+    responseData.push([
+        'customCommand(["RESTORE", key, "0", dumpResult])',
+        "OK",
+    ]);
+    baseTransaction.get(key);
+    responseData.push(["get(key)", valueResponse]);
+
+    return responseData;
+}
+
 /**
  * Populates a transaction with commands to test.
  * @param baseTransaction - A transaction.