gemini review comments fix

Author: surbhigarg92

spannerlib/wrappers/spannerlib-node/scripts/fix-extensions.cjs

@@ -24,10 +24,8 @@ function traverseDir(dir) {
             traverseDir(fullPath);
         } else if (fullPath.endsWith('.cjs')) {
             let content = fs.readFileSync(fullPath, 'utf8');
-            // Replace require('./... .js') with require('./... .cjs')
-            content = content.replace(/require\(['"](\.\/[^'"]+)\.js['"]\)/g, "require('$1.cjs')");
-            // Also handle ../
-            content = content.replace(/require\(['"](\.\.\/[^'"]+)\.js['"]\)/g, "require('$1.cjs')");
+            // Replace require paths starting with . / or .. / and ending with .js
+            content = content.replace(/require\(['"]((\.|\.\.)\/[^'"]+)\.js['"]\)/g, "require('$1.cjs')");
             // Fix import.meta.url syntax error in CommonJS
             content = content.replace(/import\.meta\.url/g, '""');
             fs.writeFileSync(fullPath, content);

spannerlib/wrappers/spannerlib-node/src/cpp/addon.cc

@@ -299,8 +299,47 @@ class MetadataWorker : public Napi::AsyncWorker {
     Metadata_return result_;
 };
 
+//
+// Worker 8: CloseRows asynchronously
+//
+class CloseRowsWorker : public Napi::AsyncWorker {
+public:
+    CloseRowsWorker(Napi::Function& callback, int64_t poolId, int64_t connId, int64_t rowsId)
+        : AsyncWorker(callback), poolId_(poolId), connId_(connId), rowsId_(rowsId), result_({0, 0, 0, 0, nullptr}) {}
+
+    void Execute() override {
+        result_ = ::CloseRows(poolId_, connId_, rowsId_);
+    }
+
+    void OnOK() override {
+        Napi::Env env = Env();
+        Napi::Object obj = Napi::Object::New(env);
+        obj.Set("r0", Napi::Number::New(env, result_.r0));
+        obj.Set("r1", Napi::Number::New(env, result_.r1));
+        obj.Set("r2", Napi::Number::New(env, result_.r2));
+        obj.Set("r3", Napi::Number::New(env, result_.r3));
+        if (result_.r4 != nullptr && result_.r3 > 0) {
+            obj.Set("r4", Napi::Buffer<uint8_t>::Copy(env, (uint8_t*)result_.r4, result_.r3));
+        } else {
+            obj.Set("r4", env.Null());
+        }
+        // Release the pinner ID of the response message to prevent native leak!
+        if (result_.r0 > 0) {
+            ::Release(result_.r0);
+        }
+        Callback().Call({env.Null(), obj});
+    }
+private:
+    int64_t poolId_, connId_, rowsId_;
+    CloseRows_return result_;
+};
+
 Napi::Value MetadataWrapper(const Napi::CallbackInfo& info) {
     Napi::Env env = info.Env();
+    if (info.Length() < 4) {
+        Napi::Error::New(env, "MetadataWrapper requires 4 arguments").ThrowAsJavaScriptException();
+        return env.Null();
+    }
     int64_t pid = info[0].As<Napi::Number>().Int64Value();
     int64_t cid = info[1].As<Napi::Number>().Int64Value();
     int64_t rid = info[2].As<Napi::Number>().Int64Value();
@@ -323,21 +362,17 @@ Napi::Value NativeRelease(const Napi::CallbackInfo& info) {
 // CloseRows dummy/missing implementation for POC length if needed, or we just rely on GC.
 Napi::Value CloseRowsWrapper(const Napi::CallbackInfo& info) {
     Napi::Env env = info.Env();
-    if (info.Length() < 3) return env.Null();
+    if (info.Length() < 4) {
+        Napi::Error::New(env, "CloseRowsWrapper requires 4 arguments").ThrowAsJavaScriptException();
+        return env.Null();
+    }
     int64_t pid = info[0].As<Napi::Number>().Int64Value();
     int64_t cid = info[1].As<Napi::Number>().Int64Value();
     int64_t rid = info[2].As<Napi::Number>().Int64Value();
+    Napi::Function cb = info[3].As<Napi::Function>();
 
-    // N-API sync close implementation
-    CloseRows(pid, cid, rid);
-    
-    // invokeAsync appends a callback at the end of properties
-    if (info.Length() >= 4 && info[3].IsFunction()) {
-        Napi::Object obj = Napi::Object::New(env);
-        obj.Set("r1", Napi::Number::New(env, 0));
-        Napi::Function cb = info[3].As<Napi::Function>();
-        cb.Call({env.Null(), obj}); // Mock empty GoReturnTuple callback
-    }
+    CloseRowsWorker* worker = new CloseRowsWorker(cb, pid, cid, rid);
+    worker->Queue();
     return env.Undefined();
 }
 

spannerlib/wrappers/spannerlib-node/src/lib/connection.ts

@@ -54,6 +54,7 @@ export class Connection {
 
     c.oid = handled.objectId;
     c.pinnerId = handled.pinnerId;
+    spannerLib.register(c, handled.pinnerId);
     return c;
   }
 
@@ -89,8 +90,9 @@ export class Connection {
       serializedPb
     );
     const rowsId = rowsResult.objectId;
+    const rowsPinnerId = rowsResult.pinnerId;
 
-    return new Rows(this, rowsId);
+    return new Rows(this, rowsId, rowsPinnerId);
   }
 
   /**

spannerlib/wrappers/spannerlib-node/src/lib/pool.ts

@@ -52,6 +52,7 @@ export class Pool {
 
     p.oid = handled.objectId;
     p.pinnerId = handled.pinnerId;
+    spannerLib.register(p, handled.pinnerId);
     return p;
   }
 

spannerlib/wrappers/spannerlib-node/src/lib/rows.ts

@@ -33,11 +33,12 @@ export class Rows {
   public oid: number;
   public pinnerId: number | null;
   public closed: boolean;
-  constructor(connection: Connection, oid: number) {
+  constructor(connection: Connection, oid: number, pinnerId: number) {
     this.connection = connection;
     this.oid = oid;
-    this.pinnerId = null;
+    this.pinnerId = pinnerId;
     this.closed = false;
+    spannerLib.register(this, pinnerId);
   }
 
   /**
@@ -50,11 +51,15 @@ export class Rows {
   async next(): Promise<unknown> {
     if (this.closed) throw new Error('Rows are already closed');
 
+    if (!this.connection.pool) {
+      throw new Error('Connection must be bound to a Pool to fetch rows');
+    }
+
     const handled = await ffi.invokeAsync(
       'Next',
       null,
       null,
-      this.connection.pool!.oid,
+      this.connection.pool.oid,
       this.connection.oid,
       this.oid,
       1,
@@ -65,7 +70,11 @@ export class Rows {
       return null;
     }
 
-    return ListValue.decode(handled.protobufBytes);
+    const row = ListValue.decode(handled.protobufBytes);
+    if (handled.pinnerId && handled.pinnerId > 0) {
+      ffi.Release(handled.pinnerId);
+    }
+    return row;
   }
 
   /**

spannerlib/wrappers/spannerlib-node/test/rows.test.ts

@@ -48,7 +48,7 @@ describe('Rows', () => {
     connection.pool = pool;
     connection.oid = 2;
 
-    const rows = new Rows(connection, 3);
+    const rows = new Rows(connection, 3, 4);
 
     // Create a dummy ListValue
     const listValue = ListValue.create({
@@ -88,7 +88,7 @@ describe('Rows', () => {
     connection.pool = pool;
     connection.oid = 2;
 
-    const rows = new Rows(connection, 3);
+    const rows = new Rows(connection, 3, 4);
 
     stub
       .onFirstCall()