fix: resolve flaky sqlite each snapshot tests

`each(rowCallback?, complete?)` was called via `promisify` without an
explicit row callback, so promisify's callback landed in the row slot
instead of the completion slot. The sql_query return event was then
recorded in a later event loop tick, racing with subsequent events and
producing non-deterministic event IDs in the snapshot.

Fix by passing `undefined` as a row callback placeholder so promisify's
callback correctly binds to `complete`. Update the sqlite hook to
recognise null/undefined in the row callback position accordingly.

Author: dividedmind

src/hooks/sqlite.ts

@@ -60,10 +60,15 @@ function createRecordingProxy<T extends RecordingProxyTarget>(
       const lastArgIsaFunction = () =>
         argArray.length > 0 && typeof argArray[argArray.length - 1] === "function";
       if (lastArgIsaFunction()) functionArgs.unshift(argArray.pop());
-      if (needsRowCallback && lastArgIsaFunction()) functionArgs.unshift(argArray.pop());
+      // Also accept undefined/null as an explicit "no row callback" placeholder.
+      const lastArgIsRowCallback = () =>
+        argArray.length > 0 &&
+        (typeof argArray[argArray.length - 1] === "function" ||
+          argArray[argArray.length - 1] == null);
+      if (needsRowCallback && lastArgIsRowCallback()) functionArgs.unshift(argArray.pop());
 
       // if needsRowCallback:
-      //   functionArgs is [] or [rowCallback] or [rowCallback, completionCallback]
+      //   functionArgs is [] or [rowCallback] or [null, completionCallback] or [rowCallback, completionCallback]
       // otherwise:
       //   functionArgs is [] or [completionCallback]
 

test/__snapshots__/sqlite.test.ts.snap

@@ -215,7 +215,7 @@ exports[`mapping Sqlite tests 1`] = `
         {
           "class": "Function",
           "name": "method",
-          "value": "[Function (anonymous)]",
+          "value": "[Function: exec]",
         },
         {
           "class": "Database",
@@ -225,7 +225,7 @@ exports[`mapping Sqlite tests 1`] = `
         },
         {
           "class": "String",
-          "value": ""SELECT 'Database.each'"",
+          "value": ""SELECT 'Database.exec'"",
         },
       ],
       "path": "index.js",
@@ -237,7 +237,7 @@ exports[`mapping Sqlite tests 1`] = `
       "id": 16,
       "sql_query": {
         "database_type": "sqlite",
-        "sql": "SELECT 'Database.each'",
+        "sql": "SELECT 'Database.exec'",
       },
       "thread_id": 0,
     },
@@ -247,12 +247,19 @@ exports[`mapping Sqlite tests 1`] = `
       "id": 17,
       "parent_id": 15,
       "return_value": {
-        "class": "Promise<Object>",
+        "class": "Promise<undefined>",
         "object_id": 6,
-        "value": "Promise { { "'Database.each'": 'Database.each' } }",
+        "value": "Promise { undefined }",
       },
       "thread_id": 0,
     },
+    {
+      "elapsed": 31.337,
+      "event": "return",
+      "id": 18,
+      "parent_id": 16,
+      "thread_id": 0,
+    },
     {
       "defined_class": "index",
       "event": "call",
@@ -263,7 +270,7 @@ exports[`mapping Sqlite tests 1`] = `
         {
           "class": "Function",
           "name": "method",
-          "value": "[Function: exec]",
+          "value": "[Function (anonymous)]",
         },
         {
           "class": "Database",
@@ -273,7 +280,11 @@ exports[`mapping Sqlite tests 1`] = `
         },
         {
           "class": "String",
-          "value": ""SELECT 'Database.exec'"",
+          "value": ""SELECT 'Database.each'"",
+        },
+        {
+          "class": "undefined",
+          "value": "undefined",
         },
       ],
       "path": "index.js",
@@ -285,7 +296,7 @@ exports[`mapping Sqlite tests 1`] = `
       "id": 20,
       "sql_query": {
         "database_type": "sqlite",
-        "sql": "SELECT 'Database.exec'",
+        "sql": "SELECT 'Database.each'",
       },
       "thread_id": 0,
     },
@@ -295,19 +306,12 @@ exports[`mapping Sqlite tests 1`] = `
       "id": 21,
       "parent_id": 19,
       "return_value": {
-        "class": "Promise<undefined>",
+        "class": "Promise<Object>",
         "object_id": 7,
-        "value": "Promise { undefined }",
+        "value": "Promise { { "'Database.each'": 'Database.each' } }",
       },
       "thread_id": 0,
     },
-    {
-      "elapsed": 31.337,
-      "event": "return",
-      "id": 22,
-      "parent_id": 20,
-      "thread_id": 0,
-    },
     {
       "defined_class": "index",
       "event": "call",
@@ -664,6 +668,10 @@ exports[`mapping Sqlite tests 1`] = `
           "object_id": 18,
           "value": "Statement {}",
         },
+        {
+          "class": "undefined",
+          "value": "undefined",
+        },
       ],
       "path": "index.js",
       "static": true,
@@ -839,8 +847,8 @@ exports[`mapping Sqlite tests 1`] = `
     {
       "elapsed": 31.337,
       "event": "return",
-      "id": 18,
-      "parent_id": 16,
+      "id": 22,
+      "parent_id": 20,
       "thread_id": 0,
     },
     {

test/sqlite/index.js

@@ -16,13 +16,17 @@ async function main() {
     [sqlite.Database.prototype.run, "run"],
     [sqlite.Database.prototype.all, "all"],
     [sqlite.Database.prototype.get, "get"],
-    [sqlite.Database.prototype.each, "each"],
     [sqlite.Database.prototype.exec, "exec"],
   ]) {
     const [method, name] = item;
     await promisify(method, db, `SELECT 'Database.${name}'`);
   }
 
+  // each() takes an optional row callback followed by a completion callback.
+  // Pass undefined for the row callback so promisify's callback lands in the
+  // completion slot, ensuring functionReturn is recorded before the promise resolves.
+  await promisify(sqlite.Database.prototype.each, db, `SELECT 'Database.each'`, undefined);
+
   await promisify(
     sqlite.Database.prototype.each,
     db,
@@ -50,14 +54,17 @@ async function main() {
     [sqlite.Statement.prototype.run, "run"],
     [sqlite.Statement.prototype.all, "all"],
     [sqlite.Statement.prototype.get, "get"],
-    [sqlite.Statement.prototype.each, "each"],
   ]) {
     const [method, name] = item;
     const st = db.prepare(`SELECT 'Statement.${name}'`);
     await promisify(method, st);
     st.finalize();
   }
 
+  const stEach = db.prepare(`SELECT 'Statement.each'`);
+  await promisify(sqlite.Statement.prototype.each, stEach, undefined);
+  stEach.finalize();
+
   // Same statement twice should produce two sql events in appmap.
   const st = db.prepare(`SELECT 'Statement.run - prepare once run twice'`);
   await promisify(sqlite.Statement.prototype.run, st);