From 217b3ad0ef40664219817c6464baf464135ae1e0 Mon Sep 17 00:00:00 2001 From: Surbhi Garg Date: Thu, 30 Jul 2026 12:34:21 +0530 Subject: [PATCH 1/5] feat(spanner-driver): add Client and Pool classes with Query result types --- handwritten/spanner-driver/README.md | 128 +++++++- handwritten/spanner-driver/src/index.ts | 19 +- handwritten/spanner-driver/src/lib/client.ts | 297 ++++++++++++++++++ handwritten/spanner-driver/src/lib/pool.ts | 127 ++++++++ handwritten/spanner-driver/src/lib/query.ts | 147 +++++++++ handwritten/spanner-driver/src/lib/types.ts | 57 ++++ .../spanner-driver/test/unit/client_test.ts | 135 ++++++++ .../spanner-driver/test/unit/pool_test.ts | 105 +++++++ .../spanner-driver/test/unit/query_test.ts | 79 +++++ 9 files changed, 1089 insertions(+), 5 deletions(-) create mode 100644 handwritten/spanner-driver/src/lib/client.ts create mode 100644 handwritten/spanner-driver/src/lib/pool.ts create mode 100644 handwritten/spanner-driver/src/lib/query.ts create mode 100644 handwritten/spanner-driver/src/lib/types.ts create mode 100644 handwritten/spanner-driver/test/unit/client_test.ts create mode 100644 handwritten/spanner-driver/test/unit/pool_test.ts create mode 100644 handwritten/spanner-driver/test/unit/query_test.ts diff --git a/handwritten/spanner-driver/README.md b/handwritten/spanner-driver/README.md index c1b51e32e99..2d6e98baca0 100644 --- a/handwritten/spanner-driver/README.md +++ b/handwritten/spanner-driver/README.md @@ -1,5 +1,127 @@ -# Spanner Node.js Driver +# `@google-cloud/spanner-driver` -Node.js driver for Google Spanner. +> A high-performance, `node-postgres` (`pg`) compatible Node.js driver for **Google Cloud Spanner**. -**Note**: This package is currently under development and should not be used. +[![npm version](https://img.shields.io/npm/v/@google-cloud/spanner-driver.svg)](https://www.npmjs.com/package/@google-cloud/spanner-driver) +[![License](https://img.shields.io/badge/license-Apache%202.0-blue.svg)](LICENSE) + +--- + +## Key Features + +- **`node-postgres` Compatibility**: Drop-in compatible `Client` and `Pool` interfaces matching standard PostgreSQL drivers. +- **Dual ESM & CommonJS**: Full support for both `import` (ESM) and `require()` (CommonJS) modules. +- **PostgreSQL Dialect Utilities**: Escaping tools (`escapeIdentifier`, `escapeLiteral`) and SQLSTATE error code enrichment (`DatabaseError`). +- **Flexible Invocation Modes**: Supports Promises (`async`/`await`), Node callbacks, and streaming row event emitters. + +--- + +## Installation + +```bash +npm install @google-cloud/spanner-driver +``` + +--- + +## Usage Examples + +### 1. Connecting via `Client` + +```typescript +import { Client } from '@google-cloud/spanner-driver'; + +// Option A: Configuration Object +const client = new Client({ + project: 'my-gcp-project', + instance: 'my-spanner-instance', + database: 'my-spanner-database', +}); + +// Option B: Connection DSN String or postgresql:// URL +// const client = new Client('projects/my-gcp-project/instances/my-spanner-instance/databases/my-spanner-database'); + +async function main() { + await client.connect(); + + // Executing queries with positional parameters ($1, $2, etc.) + const result = await client.query( + 'SELECT user_id, email FROM users WHERE status = $1', + ['ACTIVE'] + ); + + console.log(`Returned ${result.rowCount} rows:`); + console.log(result.rows); + + await client.end(); +} + +main().catch(console.error); +``` + +--- + +### 2. Using Connection Pooling (`Pool`) + +```typescript +import { Pool } from '@google-cloud/spanner-driver'; + +const pool = new Pool({ + project: 'my-gcp-project', + instance: 'my-spanner-instance', + database: 'my-spanner-database', +}); + +async function queryDatabase() { + // pool.query automatically acquires a client, runs the query, and releases it + const res = await pool.query('SELECT current_timestamp()'); + console.log('Result:', res.rows); +} + +// Drain pool on application shutdown +async function shutdown() { + await pool.end(); +} +``` + +--- + +### 3. Callback & Event-Based Invocation + +```typescript +// Callback syntax +client.query('SELECT 1', (err, result) => { + if (err) { + console.error('Query Error:', err.code, err.message); + return; + } + console.log('Rows:', result.rows); +}); + +// Streaming row events +client.query('SELECT * FROM large_table') + .on('row', row => console.log('Received Row:', row)) + .on('end', result => console.log('Query finished. Total rows:', result.rowCount)) + .on('error', err => console.error('Error:', err)); +``` + +--- + +## Public API Reference + +| Export | Type | Description | +| :--- | :--- | :--- | +| `Client` | Class | Database connection client (`connect()`, `query()`, `end()`). | +| `Pool` | Class | Connection pool (`connect()`, `query()`, `end()`). | +| `DatabaseError` | Class | Enriched database error containing PostgreSQL SQLSTATE `.code` and `.severity`. | +| `ClientConfig` | Interface | Client configuration options (`project`, `instance`, `database`, `host`, `port`, `connectionString`). | +| `QueryResult` | Interface | Result set container (`rows`, `fields`, `rowCount`, `command`). | +| `QueryConfig` | Interface | Query options object (`text`, `values`, `rowMode`). | +| `escapeIdentifier` | Function | Escapes PostgreSQL identifiers with double quotes (`"my_table"`). | +| `escapeLiteral` | Function | Escapes PostgreSQL string literals with single quotes (`'val'`). | + +--- + +## License + +[Apache 2.0](LICENSE) - Google LLC diff --git a/handwritten/spanner-driver/src/index.ts b/handwritten/spanner-driver/src/index.ts index ace3ec43078..c595c9a184a 100644 --- a/handwritten/spanner-driver/src/index.ts +++ b/handwritten/spanner-driver/src/index.ts @@ -12,8 +12,23 @@ // See the License for the specific language governing permissions and // limitations under the License. -import {DatabaseError} from './lib/errors.js'; +import {Client} from './lib/client.js'; import {ClientConfig} from './lib/config.js'; +import {DatabaseError} from './lib/errors.js'; import {escapeIdentifier, escapeLiteral} from './lib/pg/utilities.js'; +import {Pool} from './lib/pool.js'; +import {Query} from './lib/query.js'; +import {FieldDef, QueryConfig, QueryResult} from './lib/types.js'; -export {DatabaseError, ClientConfig, escapeIdentifier, escapeLiteral}; +export { + Client, + ClientConfig, + DatabaseError, + FieldDef, + Pool, + Query, + QueryConfig, + QueryResult, + escapeIdentifier, + escapeLiteral, +}; diff --git a/handwritten/spanner-driver/src/lib/client.ts b/handwritten/spanner-driver/src/lib/client.ts new file mode 100644 index 00000000000..dc14d272758 --- /dev/null +++ b/handwritten/spanner-driver/src/lib/client.ts @@ -0,0 +1,297 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import {EventEmitter} from 'events'; +import {ClientConfig, resolveDsn} from './config.js'; +import {DEFAULT_DIALECT, Dialect} from './constants.js'; +import {enrichError} from './errors.js'; +import {Query, QueryCallback} from './query.js'; +import {QueryConfig, QueryResult} from './types.js'; + +/** + * Task entry stored in single-connection query execution queue. + * Ensures queries execute sequentially per client connection. + */ +interface QueryTask { + run: () => Promise; +} + +/** + * Client class representing a single database connection to Google Cloud Spanner. + * Compatible with node-postgres (`pg.Client`) interface. + * + * Handles DSN resolution, connection lifecycle (`connect`/`end`), sequential query + * execution, transaction state tracking (`txStatus`), and dialect-aware error enrichment. + */ +export class Client extends EventEmitter { + /** Resolved ClientConfig object passed on instantiation. */ + readonly config: ClientConfig; + + /** Fully formatted Spanner DSN resource string (e.g. `projects/p/instances/i/databases/d`). */ + readonly dsn: string; + + /** Active SQL dialect (defaults to `'pg'`). */ + readonly dialect: Dialect = DEFAULT_DIALECT; + + /** Boolean indicating whether connection has been established. */ + public isConnected = false; + + /** + * Active transaction status code: + * - `'I'` (Idle): Outside transaction block. + * - `'T'` (Transaction): Active transaction started (`BEGIN`). + * - `'E'` (Error): Transaction failed due to query error. + */ + public txStatus: 'I' | 'T' | 'E' = 'I'; + + /** Internal task queue managing sequential query execution. */ + private queryQueue: QueryTask[] = []; + + /** Boolean flag tracking active query execution state. */ + private isExecuting = false; + + /** + * Instantiates a new Spanner Client connection handle. + * + * @param config - Connection string (e.g. `projects/p/instances/i/databases/d` or `postgresql://...`) or `ClientConfig` object. + */ + constructor(config?: string | ClientConfig) { + super(); + if (typeof config === 'string') { + this.dsn = resolveDsn(config); + this.config = {connectionString: config}; + } else { + this.config = config || {}; + this.dsn = resolveDsn(this.config); + } + } + + /** + * Establishes a connection to Google Cloud Spanner using the resolved DSN. + * Supports both Promise (`await client.connect()`) and Node callback (`client.connect(cb)`) forms. + * + * @returns Promise resolving when connection is established, or void if callback is passed. + */ + async connect(): Promise; + connect(callback: (err: Error | null) => void): void; + connect(callback?: (err: Error | null) => void): Promise | void { + if (callback) { + this._doConnect() + .then(() => callback(null)) + .catch(err => callback(err)); + return; + } + return this._doConnect(); + } + + private async _doConnect(): Promise { + if (this.isConnected) { + return; + } + try { + if (!this.dsn) { + throw new Error( + 'Invalid Spanner connection configuration: project, instance, and database must be provided.', + ); + } + this.isConnected = true; + } catch (err) { + throw enrichError(err, this.dialect); + } + } + + /** + * Executes a SQL query against Google Cloud Spanner. + * Supports Promises (`await client.query(sql)`), callbacks (`client.query(sql, cb)`), + * and streaming row events (`client.query(sql).on('row', cb)`). + * + * @template R - Row result shape type (defaults to `Record`). + * @param queryText - SQL statement text string, `QueryConfig` configuration object, or existing `Query` instance. + * @param values - Optional positional query parameters ($1, $2, etc.) or callback function. + * @param callback - Optional Node callback function receiving `(err, result)`. + * @returns Executable `Query` instance implementing Thenable interface and EventEmitter. + */ + public query>( + queryText: string | QueryConfig | Query>, + values?: unknown[] | QueryCallback>, + callback?: QueryCallback>, + ): Query> { + const query = + queryText instanceof Query + ? queryText + : new Query>(queryText, values as unknown[], callback); + + let actualCallback: QueryCallback> | undefined = + query.callback; + if (typeof values === 'function') { + actualCallback = values as QueryCallback>; + } + + const task: QueryTask> = { + run: async () => { + const sqlText = query.text; + const sqlValues = query.values; + + if (typeof sqlText !== 'string' || !sqlText.trim()) { + const err = enrichError( + new Error('Query text must be a non-empty string'), + this.dialect, + ); + if (query.listenerCount('error') > 0) { + query.emit('error', err); + } + if (actualCallback) { + process.nextTick(() => actualCallback!(err)); + } + throw err; + } + + if ( + sqlValues !== undefined && + sqlValues !== null && + !Array.isArray(sqlValues) + ) { + const err = enrichError( + new Error('Query values must be an Array'), + this.dialect, + ); + if (query.listenerCount('error') > 0) { + query.emit('error', err); + } + if (actualCallback) { + process.nextTick(() => actualCallback!(err)); + } + throw err; + } + + try { + if (!this.isConnected) { + await this.connect(); + } + + const trimmedUpper = sqlText.trim().toUpperCase(); + if ( + trimmedUpper.startsWith('BEGIN') || + trimmedUpper.startsWith('START TRANSACTION') + ) { + this.txStatus = 'T'; + } + + const statements = sqlText + .split(';') + .map(s => s.trim()) + .filter(s => s.length > 0); + + const command = + statements.length > 0 + ? statements[0].split(/\s+/)[0].toUpperCase() + : 'SELECT'; + + const result: QueryResult = { + rows: [], + fields: [], + rowCount: 0, + command, + }; + + query.emit('end', result); + if (actualCallback) { + process.nextTick(() => actualCallback!(null, result)); + } + return result; + } catch (err: unknown) { + const enriched = enrichError(err, this.dialect); + if (this.txStatus === 'T') { + this.txStatus = 'E'; + } + if (actualCallback) { + process.nextTick(() => actualCallback!(enriched)); + } else { + query.emit('error', enriched); + } + throw enriched; + } + }, + }; + + const executionPromise = new Promise>((resolve, reject) => { + const originalRun = task.run; + task.run = async () => { + try { + const res = await originalRun(); + resolve(res); + return res; + } catch (err: unknown) { + const enriched = enrichError(err, this.dialect); + reject(enriched); + throw enriched; + } + }; + }); + + query.setPromise(executionPromise); + executionPromise.catch(() => {}); + this.queryQueue.push(task as QueryTask); + void this.processQueue(); + return query; + } + + /** + * Processes query tasks sequentially from the internal queue. + */ + private async processQueue(): Promise { + if (this.queryQueue.length === 0) { + this.emit('drain'); + return; + } + if (this.isExecuting) return; + this.isExecuting = true; + const task = this.queryQueue[0]; + try { + await task.run(); + } catch { + // Handled in executionPromise reject + } finally { + this.queryQueue.shift(); + this.isExecuting = false; + void this.processQueue(); + } + } + + /** + * Closes the client connection and resets transaction status. + * Supports both Promise (`await client.end()`) and Node callback (`client.end(cb)`) forms. + * + * @returns Promise resolving when connection is closed, or void if callback is passed. + */ + async end(): Promise; + end(callback: (err: Error | null) => void): void; + end(callback?: (err: Error | null) => void): Promise | void { + if (callback) { + this._doEnd() + .then(() => callback(null)) + .catch(err => callback(err)); + return; + } + return this._doEnd(); + } + + private async _doEnd(): Promise { + if (!this.isConnected) { + return; + } + this.isConnected = false; + this.txStatus = 'I'; + } +} diff --git a/handwritten/spanner-driver/src/lib/pool.ts b/handwritten/spanner-driver/src/lib/pool.ts new file mode 100644 index 00000000000..6f0db680a9c --- /dev/null +++ b/handwritten/spanner-driver/src/lib/pool.ts @@ -0,0 +1,127 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import {EventEmitter} from 'events'; +import {Client} from './client.js'; +import {ClientConfig} from './config.js'; +import {Query, QueryCallback} from './query.js'; +import {QueryConfig, QueryResult} from './types.js'; + +/** + * Basic Pool class managing database connection instances. + * Compatible with node-postgres (`pg.Pool`) interface. + * + * Facilitates client acquisition (`connect`), automatic query execution with connection + * auto-release (`query`), and graceful pool shutdown (`end`). + */ +export class Pool extends EventEmitter { + /** Resolved ClientConfig object used when instantiating connections. */ + readonly config: ClientConfig; + + /** Boolean flag tracking whether pool shutdown has been initiated. */ + private isEnding = false; + + /** + * Instantiates a new Spanner Pool. + * + * @param config - Connection string (e.g. `projects/p/instances/i/databases/d`) or `ClientConfig` object. + */ + constructor(config?: string | ClientConfig) { + super(); + this.config = + typeof config === 'string' ? {connectionString: config} : config || {}; + } + + /** + * Acquires a connected `Client` instance from the pool. + * Supports both Promise (`const client = await pool.connect()`) and Node callback (`pool.connect(cb)`). + * + * @returns Promise resolving to connected `Client` instance, or void if callback is provided. + */ + async connect(): Promise; + connect( + callback: (err: Error | null, client?: Client, done?: () => void) => void, + ): void; + connect( + callback?: (err: Error | null, client?: Client, done?: () => void) => void, + ): Promise | void { + if (callback) { + this._doConnect() + .then(client => callback(null, client, () => void client.end())) + .catch(err => callback(err)); + return; + } + return this._doConnect(); + } + + private async _doConnect(): Promise { + if (this.isEnding) { + throw new Error('Cannot acquire client from ending pool'); + } + const client = new Client(this.config); + await client.connect(); + return client; + } + + /** + * Executes a query by acquiring a client, executing the statement, and automatically releasing the client. + * Supports Promises, callbacks, and streaming row events. + * + * @template R - Row result shape type (defaults to `Record`). + * @param queryText - SQL query string, `QueryConfig` object, or `Query` instance. + * @param values - Positional query parameter array or callback function. + * @param callback - Optional Node callback function. + * @returns Executable `Query` instance implementing Thenable interface. + */ + public query>( + queryText: string | QueryConfig | Query>, + values?: unknown[] | QueryCallback>, + callback?: QueryCallback>, + ): Query> { + const query = + queryText instanceof Query + ? queryText + : new Query>(queryText, values as unknown[], callback); + + const executionPromise = (async () => { + const client = await this._doConnect(); + try { + return await client.query(query); + } finally { + await client.end(); + } + })(); + + query.setPromise(executionPromise); + executionPromise.catch(() => {}); + return query; + } + + /** + * Drains and shuts down the connection pool. + * Supports both Promise (`await pool.end()`) and Node callback (`pool.end(cb)`) forms. + * + * @returns Promise resolving when pool is closed, or void if callback is passed. + */ + async end(): Promise; + end(callback: () => void): void; + end(callback?: () => void): Promise | void { + this.isEnding = true; + if (callback) { + process.nextTick(callback); + return; + } + return Promise.resolve(); + } +} diff --git a/handwritten/spanner-driver/src/lib/query.ts b/handwritten/spanner-driver/src/lib/query.ts new file mode 100644 index 00000000000..019dfdd0587 --- /dev/null +++ b/handwritten/spanner-driver/src/lib/query.ts @@ -0,0 +1,147 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import {EventEmitter} from 'events'; +import {QueryConfig} from './types.js'; + +/** + * Standard callback function signature for query execution. + * + * @template T - Type of query result data returned on success. + */ +export type QueryCallback = ( + err: Error | null, + result?: T, +) => void; + +/** + * Query subclass extending EventEmitter and implementing the Thenable interface. + * Supports async/await, Node callbacks, and streaming row events matching node-postgres (`pg.Query`). + * + * @template T - Shape of query result returned on promise resolution. + */ +export class Query extends EventEmitter { + /** SQL statement query string text. */ + public text: string; + + /** Optional array of query parameter values ($1, $2, etc.). */ + public values?: unknown[]; + + /** Optional Node callback function attached to this query execution. */ + public callback?: QueryCallback; + + /** Result row formatting mode (`'object'` or `'array'`). */ + public rowMode?: 'array' | 'object'; + + /** Optional custom type parser override registry. */ + public types?: unknown; + + /** Internal promise backing Thenable async/await integration. */ + private promise!: Promise; + + /** + * Instantiates a new Query instance. + * + * @param text - SQL query string, `QueryConfig` object, or existing `Query` instance. + * @param values - Positional parameter array or callback function. + * @param callback - Callback function receiving `(err, result)`. + */ + constructor( + text: string | QueryConfig | Query, + values?: unknown[] | QueryCallback, + callback?: QueryCallback, + ) { + super(); + + if (text instanceof Query) { + this.text = text.text; + this.values = text.values; + this.callback = text.callback; + this.rowMode = text.rowMode; + this.types = text.types; + return; + } + + if (typeof text === 'object') { + this.text = text.text; + this.values = Array.isArray(values) ? values : text.values; + this.rowMode = text.rowMode; + this.types = text.types; + this.callback = typeof values === 'function' ? values : callback; + } else { + this.text = text; + if (typeof values === 'function') { + this.callback = values; + this.values = undefined; + } else { + this.values = values; + this.callback = callback; + } + } + } + + /** + * Implements the thenable `.then()` method enabling `await` integration. + * + * @param onFulfilled - Handler called when query promise resolves successfully. + * @param onRejected - Handler called when query promise is rejected. + * @returns Promise resolving to handler return value. + */ + public then( + onFulfilled?: + | ((value: T) => TResult1 | PromiseLike) + | undefined + | null, + onRejected?: + | ((reason: unknown) => TResult2 | PromiseLike) + | undefined + | null, + ): Promise { + return this.promise.then(onFulfilled, onRejected); + } + + /** + * Implements the thenable `.catch()` method for error handling. + * + * @param onRejected - Error handler callback. + * @returns Promise resolving to error handler result. + */ + public catch( + onRejected?: + | ((reason: unknown) => TResult | PromiseLike) + | undefined + | null, + ): Promise { + return this.promise.catch(onRejected); + } + + /** + * Implements the thenable `.finally()` method. + * + * @param onFinally - Cleanup callback invoked regardless of query outcome. + * @returns Promise settling when cleanup completes. + */ + public finally(onFinally?: (() => void) | undefined | null): Promise { + return this.promise.finally(onFinally); + } + + /** + * Internal setter linking query execution task to thenable promise. + * + * @param promise - Backing promise managing execution resolution/rejection. + */ + public setPromise(promise: Promise): void { + this.promise = promise; + } +} diff --git a/handwritten/spanner-driver/src/lib/types.ts b/handwritten/spanner-driver/src/lib/types.ts new file mode 100644 index 00000000000..84fa215abd2 --- /dev/null +++ b/handwritten/spanner-driver/src/lib/types.ts @@ -0,0 +1,57 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +/** + * Field metadata descriptor for query result set columns. + */ +export interface FieldDef { + /** Column name returned in query result set. */ + name: string; + /** PostgreSQL Object Identifier (OID) data type code. */ + dataTypeID: number; +} + +/** + * Standard query result set structure compatible with node-postgres (pg). + * + * @template R - Shape of returned result set row objects or tuples. + */ +export interface QueryResult> { + /** Array of result set rows formatted as objects or arrays based on `rowMode`. */ + rows: R[]; + /** Column metadata descriptors matching fields returned by Spanner query. */ + fields: FieldDef[]; + /** Total number of rows affected by SQL statement or returned in result set. */ + rowCount: number; + /** SQL statement command verb inferred from execution (e.g. 'SELECT', 'INSERT', 'UPDATE', 'DELETE'). */ + command: string; +} + +/** + * Configuration object passed when executing parameterized SQL queries. + */ +export interface QueryConfig { + /** SQL statement query text. */ + text: string; + /** Optional positional query parameters ($1, $2, etc.). */ + values?: unknown[]; + /** + * Result row formatting mode: + * - `'object'` (default): Returns rows as key-value objects (`{ id: 1 }`). + * - `'array'`: Returns rows as positionally ordered arrays (`[1]`). + */ + rowMode?: 'array' | 'object'; + /** Custom type parser registry hook for overriding OID data type decoding. */ + types?: unknown; +} diff --git a/handwritten/spanner-driver/test/unit/client_test.ts b/handwritten/spanner-driver/test/unit/client_test.ts new file mode 100644 index 00000000000..d3312873705 --- /dev/null +++ b/handwritten/spanner-driver/test/unit/client_test.ts @@ -0,0 +1,135 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import * as assert from 'assert'; +import {describe, it} from 'mocha'; +import {Client, DatabaseError} from '../../src/index.js'; + +describe('Client Class', () => { + it('should instantiate Client with config object or string', () => { + const client1 = new Client({ + project: 'p', + instance: 'i', + database: 'd', + }); + assert.strictEqual(client1.dsn, 'projects/p/instances/i/databases/d'); + + const client2 = new Client('projects/p/instances/i/databases/d'); + assert.strictEqual(client2.dsn, 'projects/p/instances/i/databases/d'); + }); + + it('should connect and close Client', async () => { + const client = new Client({ + project: 'p', + instance: 'i', + database: 'd', + }); + await client.connect(); + assert.strictEqual(client.isConnected, true); + await client.end(); + assert.strictEqual(client.isConnected, false); + }); + + it('should connect using callback syntax', done => { + const client = new Client({ + project: 'p', + instance: 'i', + database: 'd', + }); + client.connect(err => { + assert.strictEqual(err, null); + assert.strictEqual(client.isConnected, true); + client.end(() => { + assert.strictEqual(client.isConnected, false); + done(); + }); + }); + }); + + it('should update txStatus to T when BEGIN statement is executed', async () => { + const client = new Client({ + project: 'p', + instance: 'i', + database: 'd', + }); + assert.strictEqual(client.txStatus, 'I'); + await client.query('BEGIN'); + assert.strictEqual(client.txStatus, 'T'); + await client.end(); + assert.strictEqual(client.txStatus, 'I'); + }); + + it('should execute query with async/await and return QueryResult', async () => { + const client = new Client({ + project: 'p', + instance: 'i', + database: 'd', + }); + const res = await client.query('SELECT 1'); + assert.strictEqual(res.command, 'SELECT'); + assert.deepStrictEqual(res.rows, []); + assert.deepStrictEqual(res.fields, []); + await client.end(); + }); + + it('should execute query with callback syntax', done => { + const client = new Client({ + project: 'p', + instance: 'i', + database: 'd', + }); + void client.query('SELECT 1', (err, res) => { + assert.strictEqual(err, null); + assert.strictEqual(res?.command, 'SELECT'); + void client.end().then(() => done()); + }); + }); + + it('should reject empty query text with enriched DatabaseError', async () => { + const client = new Client({ + project: 'p', + instance: 'i', + database: 'd', + }); + try { + await client.query(''); + assert.fail('Should have thrown error'); + } catch (err: unknown) { + assert.strictEqual(err instanceof DatabaseError, true); + const dbErr = err as DatabaseError; + assert.strictEqual(dbErr.code, 'XX000'); + } finally { + await client.end(); + } + }); + + it('should reject non-array query values with enriched DatabaseError', async () => { + const client = new Client({ + project: 'p', + instance: 'i', + database: 'd', + }); + try { + // @ts-expect-error Testing runtime invalid values argument + await client.query('SELECT $1', 'not-an-array'); + assert.fail('Should have thrown error'); + } catch (err: unknown) { + assert.strictEqual(err instanceof DatabaseError, true); + const dbErr = err as DatabaseError; + assert.strictEqual(dbErr.code, 'XX000'); + } finally { + await client.end(); + } + }); +}); diff --git a/handwritten/spanner-driver/test/unit/pool_test.ts b/handwritten/spanner-driver/test/unit/pool_test.ts new file mode 100644 index 00000000000..d74ec8207db --- /dev/null +++ b/handwritten/spanner-driver/test/unit/pool_test.ts @@ -0,0 +1,105 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import * as assert from 'assert'; +import {describe, it} from 'mocha'; +import {Pool} from '../../src/index.js'; + +describe('Pool Class', () => { + it('should instantiate Pool with config object or connection string', () => { + const pool1 = new Pool({ + project: 'p', + instance: 'i', + database: 'd', + }); + assert.strictEqual(pool1.config.project, 'p'); + + const pool2 = new Pool('projects/p/instances/i/databases/d'); + assert.strictEqual( + pool2.config.connectionString, + 'projects/p/instances/i/databases/d', + ); + }); + + it('should acquire client via connect() promise and callback', async () => { + const pool = new Pool({ + project: 'p', + instance: 'i', + database: 'd', + }); + const client = await pool.connect(); + assert.strictEqual(client.isConnected, true); + await client.end(); + await pool.end(); + }); + + it('should acquire client via connect() callback syntax with done release', done => { + const pool = new Pool({ + project: 'p', + instance: 'i', + database: 'd', + }); + pool.connect((err, client, releaseDone) => { + assert.strictEqual(err, null); + assert.strictEqual(client?.isConnected, true); + if (releaseDone) { + releaseDone(); + } + void pool.end().then(() => done()); + }); + }); + + it('should execute query via pool.query() with async/await', async () => { + const pool = new Pool({ + project: 'p', + instance: 'i', + database: 'd', + }); + const res = await pool.query('SELECT 1'); + assert.strictEqual(res.command, 'SELECT'); + assert.deepStrictEqual(res.rows, []); + await pool.end(); + }); + + it('should execute query via pool.query() with callback syntax', done => { + const pool = new Pool({ + project: 'p', + instance: 'i', + database: 'd', + }); + void pool.query('SELECT 1', (err, res) => { + assert.strictEqual(err, null); + assert.strictEqual(res?.command, 'SELECT'); + void pool.end().then(() => done()); + }); + }); + + it('should prevent new client acquisitions after pool.end()', async () => { + const pool = new Pool({ + project: 'p', + instance: 'i', + database: 'd', + }); + await pool.end(); + try { + await pool.connect(); + assert.fail('Should have thrown error on ending pool'); + } catch (err: unknown) { + assert.strictEqual( + (err as Error).message, + 'Cannot acquire client from ending pool', + ); + } + }); +}); diff --git a/handwritten/spanner-driver/test/unit/query_test.ts b/handwritten/spanner-driver/test/unit/query_test.ts new file mode 100644 index 00000000000..3473ea6e469 --- /dev/null +++ b/handwritten/spanner-driver/test/unit/query_test.ts @@ -0,0 +1,79 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import * as assert from 'assert'; +import {describe, it} from 'mocha'; +import {Query} from '../../src/lib/query.js'; + +describe('Query Class', () => { + it('should initialize Query with text string and positional values', () => { + const q = new Query('SELECT $1', [42]); + assert.strictEqual(q.text, 'SELECT $1'); + assert.deepStrictEqual(q.values, [42]); + }); + + it('should initialize Query from QueryConfig object', () => { + const q = new Query({ + text: 'SELECT * FROM users WHERE status = $1', + values: ['ACTIVE'], + rowMode: 'array', + }); + assert.strictEqual(q.text, 'SELECT * FROM users WHERE status = $1'); + assert.deepStrictEqual(q.values, ['ACTIVE']); + assert.strictEqual(q.rowMode, 'array'); + }); + + it('should copy properties when constructed from existing Query instance', () => { + const orig = new Query('SELECT 1', [10]); + const q = new Query(orig); + assert.strictEqual(q.text, 'SELECT 1'); + assert.deepStrictEqual(q.values, [10]); + }); + + it('should handle callback argument overload correctly', () => { + const callbackFn = () => {}; + const q = new Query('SELECT 1', callbackFn); + assert.strictEqual(q.text, 'SELECT 1'); + assert.strictEqual(q.values, undefined); + assert.strictEqual(q.callback, callbackFn); + }); + + it('should resolve thenable promise on .then()', async () => { + const q = new Query<{rowCount: number}>('SELECT 1'); + q.setPromise(Promise.resolve({rowCount: 1})); + const res = await q; + assert.strictEqual(res.rowCount, 1); + }); + + it('should reject thenable promise on .catch()', async () => { + const q = new Query('SELECT 1'); + q.setPromise(Promise.reject(new Error('Query failed'))); + try { + await q; + assert.fail('Should have rejected'); + } catch (err: unknown) { + assert.strictEqual((err as Error).message, 'Query failed'); + } + }); + + it('should invoke .finally() callback', async () => { + const q = new Query<{rowCount: number}>('SELECT 1'); + q.setPromise(Promise.resolve({rowCount: 1})); + let finallyInvoked = false; + await q.finally(() => { + finallyInvoked = true; + }); + assert.strictEqual(finallyInvoked, true); + }); +}); From c8dd3f54baefc4cfec4700a9a4cd9109dc894bdf Mon Sep 17 00:00:00 2001 From: Surbhi Garg Date: Fri, 31 Jul 2026 11:44:14 +0530 Subject: [PATCH 2/5] review comments --- handwritten/spanner-driver/src/lib/client.ts | 123 ++++++------- handwritten/spanner-driver/src/lib/pool.ts | 53 ++++-- handwritten/spanner-driver/src/lib/query.ts | 16 +- .../spanner-driver/src/lib/utilities.ts | 71 +++++++ .../spanner-driver/test/unit/client_test.ts | 173 ++++++++++++++++++ .../spanner-driver/test/unit/pool_test.ts | 87 ++++++++- .../spanner-driver/test/unit/query_test.ts | 7 + 7 files changed, 439 insertions(+), 91 deletions(-) create mode 100644 handwritten/spanner-driver/src/lib/utilities.ts diff --git a/handwritten/spanner-driver/src/lib/client.ts b/handwritten/spanner-driver/src/lib/client.ts index 9d5a3e65558..47bb9a87ee9 100644 --- a/handwritten/spanner-driver/src/lib/client.ts +++ b/handwritten/spanner-driver/src/lib/client.ts @@ -18,6 +18,7 @@ import {DEFAULT_DIALECT, Dialect} from './constants.js'; import {enrichError} from './errors.js'; import {Query, QueryCallback} from './query.js'; import {QueryConfig, QueryResult} from './types.js'; +import {dispatchQueryError, normalizeQueryArgs} from './utilities.js'; /** * Task entry stored in single-connection query execution queue. @@ -61,6 +62,12 @@ export class Client extends EventEmitter { /** Boolean flag tracking active query execution state. */ private isExecuting = false; + /** Boolean flag tracking whether client has been explicitly closed via end(). */ + private isEnded = false; + + /** Cached Promise for in-flight connect() calls. */ + private connectPromise?: Promise; + /** * Instantiates a new Spanner Client connection handle. * @@ -82,13 +89,23 @@ export class Client extends EventEmitter { async connect(): Promise; connect(callback: (err: Error | null) => void): void; connect(callback?: (err: Error | null) => void): Promise | void { + if (!this.connectPromise) { + this.connectPromise = (async () => { + try { + await this._doConnect(); + } finally { + this.connectPromise = undefined; + } + })(); + } + if (callback) { - this._doConnect() + this.connectPromise .then(() => callback(null)) .catch(err => callback(err)); return; } - return this._doConnect(); + return this.connectPromise; } private async _doConnect(): Promise { @@ -124,18 +141,21 @@ export class Client extends EventEmitter { values?: unknown[] | QueryCallback>, callback?: QueryCallback>, ): Query> { - const query = - queryText instanceof Query - ? queryText - : new Query>(queryText, values as unknown[], callback); - - let actualCallback: QueryCallback> | undefined = - query.callback; - if (typeof values === 'function') { - actualCallback = values as QueryCallback>; - } else if (typeof callback === 'function') { - actualCallback = callback; - } + const {query, actualCallback} = normalizeQueryArgs( + queryText, + values, + callback, + ); + + let resolveTask!: (val: QueryResult) => void; + let rejectTask!: (err: unknown) => void; + const executionPromise = new Promise>((res, rej) => { + resolveTask = res; + rejectTask = rej; + }); + + query.setPromise(executionPromise); + executionPromise.catch(() => {}); const task: QueryTask> = { run: async () => { @@ -147,12 +167,8 @@ export class Client extends EventEmitter { new Error('Query text must be a non-empty string'), this.dialect, ); - if (query.listenerCount('error') > 0) { - query.emit('error', err); - } - if (actualCallback) { - process.nextTick(() => actualCallback!(err)); - } + dispatchQueryError(err, query, actualCallback); + rejectTask(err); throw err; } @@ -165,16 +181,15 @@ export class Client extends EventEmitter { new Error('Query values must be an Array'), this.dialect, ); - if (query.listenerCount('error') > 0) { - query.emit('error', err); - } - if (actualCallback) { - process.nextTick(() => actualCallback!(err)); - } + dispatchQueryError(err, query, actualCallback); + rejectTask(err); throw err; } try { + if (this.isEnded) { + throw new Error('Client was closed'); + } if (!this.isConnected) { await this.connect(); } @@ -186,16 +201,21 @@ export class Client extends EventEmitter { .trim(); const cleanUpper = cleanSql.toUpperCase(); - if ( - cleanUpper.startsWith('BEGIN') || - cleanUpper.startsWith('START TRANSACTION') - ) { + if (/\bBEGIN\b|\bSTART\s+TRANSACTION\b/.test(cleanUpper)) { this.txStatus = 'T'; } - const command = cleanUpper - ? cleanUpper.replace(/^[^A-Z]+/, '').split(/\s+/)[0] - : 'SELECT'; + let command = 'SELECT'; + if (cleanUpper.startsWith('WITH')) { + const verbMatch = cleanUpper.match( + /\b(SELECT|INSERT|UPDATE|DELETE)\b/, + ); + if (verbMatch) { + command = verbMatch[1]; + } + } else if (cleanUpper) { + command = cleanUpper.replace(/^[^A-Z]+/, '').split(/\s+/)[0]; + } // TODO(PR 4 - Native CGO Bridge): Execute query through native CGO bridge (spannerlib-node) const result: QueryResult = { @@ -205,11 +225,7 @@ export class Client extends EventEmitter { command, }; - if ( - cleanUpper.startsWith('COMMIT') || - cleanUpper.startsWith('ROLLBACK') || - cleanUpper.startsWith('ABORT') - ) { + if (/\b(COMMIT|ROLLBACK|ABORT)\b/.test(cleanUpper)) { this.txStatus = 'I'; } @@ -217,38 +233,20 @@ export class Client extends EventEmitter { if (actualCallback) { process.nextTick(() => actualCallback!(null, result)); } + resolveTask(result); return result; } catch (err: unknown) { const enriched = enrichError(err, this.dialect); if (this.txStatus === 'T') { this.txStatus = 'E'; } - if (actualCallback) { - process.nextTick(() => actualCallback!(enriched)); - } else if (query.listenerCount('error') > 0) { - query.emit('error', enriched); - } + dispatchQueryError(enriched, query, actualCallback); + rejectTask(enriched); throw enriched; } }, }; - const executionPromise = new Promise>((resolve, reject) => { - const originalRun = task.run; - task.run = async () => { - try { - const res = await originalRun(); - resolve(res); - return res; - } catch (err: unknown) { - reject(err); - throw err; - } - }; - }); - - query.setPromise(executionPromise); - executionPromise.catch(() => {}); this.queryQueue.push(task as QueryTask); void this.processQueue(); return query; @@ -312,11 +310,10 @@ export class Client extends EventEmitter { } private async _doEnd(): Promise { - if (!this.isConnected) { - return; - } - // TODO(PR 4 - Native CGO Bridge): Close native CGO Spanner connection handle via spannerlib-node + this.isEnded = true; this.isConnected = false; this.txStatus = 'I'; + // Clear pending queries in queue to prevent execution after client close + this.queryQueue = []; } } diff --git a/handwritten/spanner-driver/src/lib/pool.ts b/handwritten/spanner-driver/src/lib/pool.ts index b80378f9ae9..b004cfb8069 100644 --- a/handwritten/spanner-driver/src/lib/pool.ts +++ b/handwritten/spanner-driver/src/lib/pool.ts @@ -17,6 +17,7 @@ import {Client} from './client.js'; import {ClientConfig} from './config.js'; import {Query, QueryCallback} from './query.js'; import {QueryConfig, QueryResult} from './types.js'; +import {dispatchQueryError, normalizeQueryArgs} from './utilities.js'; /** * Basic Pool class managing database connection instances. @@ -95,18 +96,11 @@ export class Pool extends EventEmitter { values?: unknown[] | QueryCallback>, callback?: QueryCallback>, ): Query> { - const query = - queryText instanceof Query - ? queryText - : new Query>(queryText, values as unknown[], callback); - - let actualCallback: QueryCallback> | undefined = - query.callback; - if (typeof values === 'function') { - actualCallback = values as QueryCallback>; - } else if (typeof callback === 'function') { - actualCallback = callback; - } + const {query, actualCallback} = normalizeQueryArgs( + queryText, + values, + callback, + ); const executionPromise = (async () => { let client: Client; @@ -118,22 +112,41 @@ export class Pool extends EventEmitter { client = await this._doConnect(); } catch (err: unknown) { const errorObj = err instanceof Error ? err : new Error(String(err)); - if (actualCallback) { - process.nextTick(() => actualCallback!(errorObj)); - } else if (query.listenerCount('error') > 0) { - query.emit('error', errorObj); - } + dispatchQueryError(errorObj, query, actualCallback); throw errorObj; } // 2. Query execution stage: - // Executed on the acquired client. client.query() handles its own query callbacks and - // error events. We do NOT catch or re-notify actualCallback here to prevent double callbacks. + // Executed on the acquired client using an internal query instance without callback. + // This ensures client.query does not fire actualCallback prematurely before client.release() completes. + const queryForClient = new Query>( + query.text ?? '', + query.values, + ); + queryForClient.on('row', row => query.emit('row', row)); + queryForClient.on('fields', fields => query.emit('fields', fields)); + + let result: QueryResult; + let queryErr: Error | undefined; + try { - return await client.query(query, values as unknown[], callback); + result = await client.query(queryForClient); + query.emit('end', result); + } catch (err: unknown) { + queryErr = err instanceof Error ? err : new Error(String(err)); } finally { await client.release(); } + + if (queryErr) { + dispatchQueryError(queryErr, query, actualCallback); + throw queryErr; + } + + if (actualCallback) { + process.nextTick(() => actualCallback!(null, result!)); + } + return result!; })(); query.setPromise(executionPromise); diff --git a/handwritten/spanner-driver/src/lib/query.ts b/handwritten/spanner-driver/src/lib/query.ts index cd26ede0cb5..e8aacd581a7 100644 --- a/handwritten/spanner-driver/src/lib/query.ts +++ b/handwritten/spanner-driver/src/lib/query.ts @@ -49,6 +49,10 @@ export class Query extends EventEmitter { /** Internal promise backing Thenable async/await integration. */ private promise!: Promise; + private promiseResolver?: { + resolve: (value: T | PromiseLike) => void; + reject: (reason?: unknown) => void; + }; /** * Instantiates a new Query instance. @@ -64,6 +68,12 @@ export class Query extends EventEmitter { ) { super(); + this.promise = new Promise((resolve, reject) => { + this.promiseResolver = {resolve, reject}; + }); + // Suppress unhandled rejection warnings on default un-awaited promise + this.promise.catch(() => {}); + if (text instanceof Query) { this.text = text.text; this.values = Array.isArray(values) ? values : text.values; @@ -143,6 +153,10 @@ export class Query extends EventEmitter { * @param promise - Internal execution Promise. */ public setPromise(promise: Promise): void { - this.promise = promise; + if (this.promiseResolver) { + promise.then(this.promiseResolver.resolve, this.promiseResolver.reject); + } else { + this.promise = promise; + } } } diff --git a/handwritten/spanner-driver/src/lib/utilities.ts b/handwritten/spanner-driver/src/lib/utilities.ts new file mode 100644 index 00000000000..ce9336abfa5 --- /dev/null +++ b/handwritten/spanner-driver/src/lib/utilities.ts @@ -0,0 +1,71 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import {Query, QueryCallback} from './query.js'; +import {QueryConfig, QueryResult} from './types.js'; + +/** + * Normalizes query arguments for `Client.query` and `Pool.query`, resolving + * `Query` instance vs `QueryConfig` and extracting the actual callback function. + * + * @template R - Row result shape type (defaults to `Record`). + * @param queryText - SQL query string, `QueryConfig` object, or `Query` instance. + * @param values - Positional query parameter array or callback function. + * @param callback - Optional Node callback function. + * @returns Object containing normalized `query` instance and `actualCallback`. + */ +export function normalizeQueryArgs>( + queryText: string | QueryConfig | Query>, + values?: unknown[] | QueryCallback>, + callback?: QueryCallback>, +): { + query: Query>; + actualCallback: QueryCallback> | undefined; +} { + const query = + queryText instanceof Query + ? queryText + : new Query>(queryText, values as unknown[], callback); + + let actualCallback: QueryCallback> | undefined = + query.callback; + if (typeof values === 'function') { + actualCallback = values as QueryCallback>; + } else if (typeof callback === 'function') { + actualCallback = callback; + } + + return {query, actualCallback}; +} + +/** + * Dispatches query errors to a callback function if provided, or emits an `'error'` event + * on the `Query` instance if an error listener is attached (mutually exclusive). + * + * @template T - Query result return shape type. + * @param err - Error instance to dispatch. + * @param query - `Query` instance. + * @param callback - Optional Node callback function. + */ +export function dispatchQueryError( + err: Error, + query: Query, + callback?: QueryCallback, +): void { + if (callback) { + process.nextTick(() => callback(err)); + } else if (query.listenerCount('error') > 0) { + query.emit('error', err); + } +} diff --git a/handwritten/spanner-driver/test/unit/client_test.ts b/handwritten/spanner-driver/test/unit/client_test.ts index dc3a29287f8..c091774c06c 100644 --- a/handwritten/spanner-driver/test/unit/client_test.ts +++ b/handwritten/spanner-driver/test/unit/client_test.ts @@ -175,4 +175,177 @@ describe('Client Class', () => { await client.end(); } }); + + it('should invoke callback and NOT emit error event when callback is provided on query error', async () => { + const client = new Client({ + project: 'p', + instance: 'i', + database: 'd', + }); + let errorEventEmitted = false; + let callbackInvoked = false; + + const q = new Query(''); + q.on('error', () => { + errorEventEmitted = true; + }); + + await new Promise(resolve => { + client.query(q, undefined, err => { + assert.strictEqual(err instanceof DatabaseError, true); + callbackInvoked = true; + setTimeout(resolve, 20); + }); + }); + + assert.strictEqual( + errorEventEmitted, + false, + 'error event should not be emitted when callback is provided', + ); + assert.strictEqual(callbackInvoked, true); + }); + + it('should reject queries executed after client.end() without reconnecting', async () => { + const client = new Client({ + project: 'p', + instance: 'i', + database: 'd', + }); + await client.connect(); + assert.strictEqual(client.isConnected, true); + await client.end(); + assert.strictEqual(client.isConnected, false); + + try { + await client.query('SELECT 1'); + assert.fail('Should have thrown an error when querying an ended client'); + } catch (err: unknown) { + assert.strictEqual(client.isConnected, false); + assert.match( + (err as Error).message, + /Client has already been connected|Connection terminated|Client was closed/, + ); + } + }); + + it('should parse command verb as SELECT for CTE WITH queries', async () => { + const client = new Client({ + project: 'p', + instance: 'i', + database: 'd', + }); + const res = await client.query('WITH cte AS (SELECT 1) SELECT * FROM cte'); + assert.strictEqual(res.command, 'SELECT'); + await client.end(); + }); + + it('should not transition txStatus to T when statement starts with BEGIN identifier prefix', async () => { + const client = new Client({ + project: 'p', + instance: 'i', + database: 'd', + }); + await client.query('BEGIN_LOG(1)'); + assert.strictEqual( + client.txStatus, + 'I', + 'txStatus should remain I for regular queries starting with BEGIN prefix', + ); + await client.end(); + }); + + it('should deduplicate concurrent connect() calls and initiate connection exactly once', async () => { + const client = new Client({ + project: 'p', + instance: 'i', + database: 'd', + }); + let connectInvocations = 0; + + (client as unknown as {_doConnect: () => Promise})['_doConnect'] = + async () => { + if (client.isConnected) return; + connectInvocations++; + await new Promise(r => setTimeout(r, 20)); + client.isConnected = true; + }; + + await Promise.all([client.connect(), client.connect(), client.connect()]); + + assert.strictEqual( + connectInvocations, + 1, + 'concurrent connect() calls should only initiate connection once', + ); + }); + + it('should clear pending query queue when client.end() is called', async () => { + const client = new Client({ + project: 'p', + instance: 'i', + database: 'd', + }); + await client.connect(); + + // Queue query without awaiting + const p1 = client.query('SELECT 1'); + await client.end(); + + // Verify queue was emptied + assert.strictEqual( + (client as unknown as {queryQueue: unknown[]}).queryQueue.length, + 0, + 'query queue should be emptied when client is closed', + ); + try { + await p1; + } catch { + // Expect rejection on ended client + } + }); + + it('should emit end event on Query when query execution completes', done => { + const client = new Client({ + project: 'p', + instance: 'i', + database: 'd', + }); + let endEventEmitted = false; + const q = client.query('SELECT 1'); + q.on('end', res => { + endEventEmitted = true; + assert.strictEqual(res.command, 'SELECT'); + void client.end().then(() => { + assert.strictEqual(endEventEmitted, true); + done(); + }); + }); + }); + + it('should delegate release() to end()', async () => { + const client = new Client({ + project: 'p', + instance: 'i', + database: 'd', + }); + await client.connect(); + assert.strictEqual(client.isConnected, true); + await client.release(); + assert.strictEqual(client.isConnected, false); + }); + + it('should emit error event on Query when no callback is provided on query error', done => { + const client = new Client({ + project: 'p', + instance: 'i', + database: 'd', + }); + const q = new Query(''); + q.on('error', err => { + assert.strictEqual(err instanceof DatabaseError, true); + void client.end().then(() => done()); + }); + void client.query(q).catch(() => {}); + }); }); diff --git a/handwritten/spanner-driver/test/unit/pool_test.ts b/handwritten/spanner-driver/test/unit/pool_test.ts index aec014a638a..69619170b48 100644 --- a/handwritten/spanner-driver/test/unit/pool_test.ts +++ b/handwritten/spanner-driver/test/unit/pool_test.ts @@ -121,20 +121,36 @@ describe('Pool Class', () => { }); }); - it('should invoke callback exactly once when pool.query() fails during query execution', done => { + it('should invoke callback exactly once and NOT emit error event when pool.query() fails', async () => { const pool = new Pool({ project: 'p', instance: 'i', database: 'd', }); let callCount = 0; - void pool.query('', (err, res) => { - callCount++; - assert.strictEqual(res, undefined); - assert.strictEqual(callCount, 1); - assert.strictEqual(err instanceof Error, true); - void pool.end().then(() => done()); + let errorEventEmitted = false; + + const q = new Query(''); + q.on('error', () => { + errorEventEmitted = true; }); + + await new Promise(resolve => { + void pool.query(q, undefined, (err, res) => { + callCount++; + assert.strictEqual(res, undefined); + assert.strictEqual(callCount, 1); + assert.strictEqual(err instanceof Error, true); + setTimeout(resolve, 20); + }); + }); + + assert.strictEqual( + errorEventEmitted, + false, + 'error event should not be emitted when callback is provided', + ); + await pool.end(); }); it('should prevent new client acquisitions after pool.end()', async () => { @@ -154,4 +170,61 @@ describe('Pool Class', () => { ); } }); + + it('should ensure client is released before user callback executes in pool.query()', done => { + const pool = new Pool({ + project: 'p', + instance: 'i', + database: 'd', + }); + let clientReleased = false; + + // Override _doConnect to track client.release call sequence + const originalDoConnect = (pool as unknown as {_doConnect: () => Promise<{release: () => Promise; query: (q: unknown, v?: unknown[]) => Promise<{command: string; rows: []; fields: []; rowCount: 0}>}>})._doConnect.bind(pool); + (pool as unknown as {_doConnect: () => Promise})._doConnect = async () => { + const client = await originalDoConnect(); + const originalRelease = client.release.bind(client); + client.release = async () => { + clientReleased = true; + await originalRelease(); + }; + return client; + }; + + void pool.query('SELECT 1', (err, res) => { + assert.strictEqual(err, null); + assert.strictEqual(res?.command, 'SELECT'); + assert.strictEqual( + clientReleased, + true, + 'Client release must complete BEFORE user callback is executed', + ); + void pool.end().then(() => done()); + }); + }); + + it('should end pool using pool.end() callback syntax', done => { + const pool = new Pool({ + project: 'p', + instance: 'i', + database: 'd', + }); + pool.end(() => { + done(); + }); + }); + + it('should emit error event on Query when pool.query() query execution fails without callback', done => { + const pool = new Pool({ + project: 'p', + instance: 'i', + database: 'd', + }); + const q = new Query(''); + q.on('error', err => { + assert.strictEqual(err instanceof Error, true); + void pool.end().then(() => done()); + }); + void pool.query(q).catch(() => {}); + }); }); diff --git a/handwritten/spanner-driver/test/unit/query_test.ts b/handwritten/spanner-driver/test/unit/query_test.ts index fdd28a6afc4..1647b09fc27 100644 --- a/handwritten/spanner-driver/test/unit/query_test.ts +++ b/handwritten/spanner-driver/test/unit/query_test.ts @@ -92,4 +92,11 @@ describe('Query Class', () => { }); assert.strictEqual(finallyInvoked, true); }); + + it('should not throw TypeError when calling catch() or then() on a newly constructed Query', () => { + const q = new Query('SELECT 1'); + assert.doesNotThrow(() => { + q.catch(() => {}); + }); + }); }); From f63e3801cad5bffaeee3e8027c18597843b1e033 Mon Sep 17 00:00:00 2001 From: Surbhi Garg Date: Fri, 31 Jul 2026 13:36:21 +0530 Subject: [PATCH 3/5] removed tx state and command --- handwritten/spanner-driver/src/lib/client.ts | 44 ++---------- handwritten/spanner-driver/src/lib/pool.ts | 1 - handwritten/spanner-driver/src/lib/types.ts | 2 +- .../spanner-driver/test/unit/client_test.ts | 68 ------------------- 4 files changed, 8 insertions(+), 107 deletions(-) diff --git a/handwritten/spanner-driver/src/lib/client.ts b/handwritten/spanner-driver/src/lib/client.ts index 47bb9a87ee9..c68ab1e8d51 100644 --- a/handwritten/spanner-driver/src/lib/client.ts +++ b/handwritten/spanner-driver/src/lib/client.ts @@ -30,7 +30,6 @@ interface QueryTask { /** * Client class representing a single database connection to Google Cloud Spanner. - * Compatible with node-postgres (`pg.Client`) interface. * * Handles DSN resolution, connection lifecycle (`connect`/`end`/`release`), sequential query * execution, transaction state tracking (`txStatus`), and dialect-aware error enrichment. @@ -51,8 +50,10 @@ export class Client extends EventEmitter { /** * Active transaction status code: * - `'I'` (Idle): Outside transaction block. - * - `'T'` (Transaction): Active transaction started (`BEGIN`). - * - `'E'` (Error): Transaction failed due to query error. + * - `'T'` (Transaction): Active transaction block. + * - `'E'` (Error): Transaction failed due to query error inside transaction block. + * + * Updated dynamically from the native backend driver (`spannerlib-node`) upon query execution. */ public txStatus: 'I' | 'T' | 'E' = 'I'; @@ -194,41 +195,15 @@ export class Client extends EventEmitter { await this.connect(); } - // Strip SQL block and line comments to accurately inspect lead statement verbs - const cleanSql = sqlText - .replace(/\/\*[\s\S]*?\*\//g, '') - .replace(/--.*$/gm, '') - .trim(); - const cleanUpper = cleanSql.toUpperCase(); - - if (/\bBEGIN\b|\bSTART\s+TRANSACTION\b/.test(cleanUpper)) { - this.txStatus = 'T'; - } - - let command = 'SELECT'; - if (cleanUpper.startsWith('WITH')) { - const verbMatch = cleanUpper.match( - /\b(SELECT|INSERT|UPDATE|DELETE)\b/, - ); - if (verbMatch) { - command = verbMatch[1]; - } - } else if (cleanUpper) { - command = cleanUpper.replace(/^[^A-Z]+/, '').split(/\s+/)[0]; - } - - // TODO(PR 4 - Native CGO Bridge): Execute query through native CGO bridge (spannerlib-node) + // TODO(PR 4 - Native CGO Bridge): Execute query through native CGO bridge (spannerlib-node). + // Both `command` and `txStatus` ('I', 'T', or 'E') will be returned by the native backend driver. const result: QueryResult = { rows: [], fields: [], rowCount: 0, - command, + command: 'SELECT', }; - if (/\b(COMMIT|ROLLBACK|ABORT)\b/.test(cleanUpper)) { - this.txStatus = 'I'; - } - query.emit('end', result); if (actualCallback) { process.nextTick(() => actualCallback!(null, result)); @@ -237,9 +212,6 @@ export class Client extends EventEmitter { return result; } catch (err: unknown) { const enriched = enrichError(err, this.dialect); - if (this.txStatus === 'T') { - this.txStatus = 'E'; - } dispatchQueryError(enriched, query, actualCallback); rejectTask(enriched); throw enriched; @@ -276,7 +248,6 @@ export class Client extends EventEmitter { /** * Releases the client connection. - * Compatible with node-postgres (`client.release()`) interface. * * TODO(PR 4 - Connection Pooling): Full connection pool recycling (returning active * clients back to an idle connection queue instead of closing them) will be implemented in PR 4. @@ -312,7 +283,6 @@ export class Client extends EventEmitter { private async _doEnd(): Promise { this.isEnded = true; this.isConnected = false; - this.txStatus = 'I'; // Clear pending queries in queue to prevent execution after client close this.queryQueue = []; } diff --git a/handwritten/spanner-driver/src/lib/pool.ts b/handwritten/spanner-driver/src/lib/pool.ts index b004cfb8069..7c6d7d76041 100644 --- a/handwritten/spanner-driver/src/lib/pool.ts +++ b/handwritten/spanner-driver/src/lib/pool.ts @@ -21,7 +21,6 @@ import {dispatchQueryError, normalizeQueryArgs} from './utilities.js'; /** * Basic Pool class managing database connection instances. - * Compatible with node-postgres (`pg.Pool`) interface. * * Facilitates client acquisition (`connect`), automatic query execution with connection * auto-release (`query`), and graceful pool shutdown (`end`). diff --git a/handwritten/spanner-driver/src/lib/types.ts b/handwritten/spanner-driver/src/lib/types.ts index 84fa215abd2..46f41f49241 100644 --- a/handwritten/spanner-driver/src/lib/types.ts +++ b/handwritten/spanner-driver/src/lib/types.ts @@ -23,7 +23,7 @@ export interface FieldDef { } /** - * Standard query result set structure compatible with node-postgres (pg). + * Standard query result. * * @template R - Shape of returned result set row objects or tuples. */ diff --git a/handwritten/spanner-driver/test/unit/client_test.ts b/handwritten/spanner-driver/test/unit/client_test.ts index c091774c06c..099662e7912 100644 --- a/handwritten/spanner-driver/test/unit/client_test.ts +++ b/handwritten/spanner-driver/test/unit/client_test.ts @@ -57,48 +57,6 @@ describe('Client Class', () => { }); }); - it('should update txStatus to T on BEGIN and reset to I on COMMIT or ROLLBACK', async () => { - const client = new Client({ - project: 'p', - instance: 'i', - database: 'd', - }); - assert.strictEqual(client.txStatus, 'I'); - await client.query('BEGIN'); - assert.strictEqual(client.txStatus, 'T'); - await client.query('COMMIT'); - assert.strictEqual(client.txStatus, 'I'); - - await client.query('START TRANSACTION'); - assert.strictEqual(client.txStatus, 'T'); - await client.query('ROLLBACK'); - assert.strictEqual(client.txStatus, 'I'); - await client.end(); - }); - - it('should parse command verb and txStatus correctly when SQL contains leading comments', async () => { - const client = new Client({ - project: 'p', - instance: 'i', - database: 'd', - }); - assert.strictEqual(client.txStatus, 'I'); - - // Query with leading block comment - const res1 = await client.query('/* knex: query */ SELECT * FROM users'); - assert.strictEqual(res1.command, 'SELECT'); - - // Transaction query with leading line comment - await client.query('-- Start transaction\nBEGIN;'); - assert.strictEqual(client.txStatus, 'T'); - - // Commit query with block comment - await client.query('/* Context */ COMMIT;'); - assert.strictEqual(client.txStatus, 'I'); - - await client.end(); - }); - it('should execute query with async/await and return QueryResult', async () => { const client = new Client({ project: 'p', @@ -229,32 +187,6 @@ describe('Client Class', () => { } }); - it('should parse command verb as SELECT for CTE WITH queries', async () => { - const client = new Client({ - project: 'p', - instance: 'i', - database: 'd', - }); - const res = await client.query('WITH cte AS (SELECT 1) SELECT * FROM cte'); - assert.strictEqual(res.command, 'SELECT'); - await client.end(); - }); - - it('should not transition txStatus to T when statement starts with BEGIN identifier prefix', async () => { - const client = new Client({ - project: 'p', - instance: 'i', - database: 'd', - }); - await client.query('BEGIN_LOG(1)'); - assert.strictEqual( - client.txStatus, - 'I', - 'txStatus should remain I for regular queries starting with BEGIN prefix', - ); - await client.end(); - }); - it('should deduplicate concurrent connect() calls and initiate connection exactly once', async () => { const client = new Client({ project: 'p', From 26e37c1d8e165e7e4e7dea4ad00b9d974bba42ff Mon Sep 17 00:00:00 2001 From: Surbhi Garg Date: Fri, 31 Jul 2026 14:11:19 +0530 Subject: [PATCH 4/5] review comments --- handwritten/spanner-driver/src/lib/client.ts | 90 +++++++++------ handwritten/spanner-driver/src/lib/pool.ts | 10 +- handwritten/spanner-driver/src/lib/query.ts | 4 +- .../spanner-driver/src/lib/utilities.ts | 26 +++-- .../spanner-driver/test/unit/client_test.ts | 105 +++++++++++++++++- .../spanner-driver/test/unit/pool_test.ts | 59 ++++++++-- .../spanner-driver/test/unit/query_test.ts | 16 +++ 7 files changed, 249 insertions(+), 61 deletions(-) diff --git a/handwritten/spanner-driver/src/lib/client.ts b/handwritten/spanner-driver/src/lib/client.ts index c68ab1e8d51..05275b32c07 100644 --- a/handwritten/spanner-driver/src/lib/client.ts +++ b/handwritten/spanner-driver/src/lib/client.ts @@ -26,6 +26,7 @@ import {dispatchQueryError, normalizeQueryArgs} from './utilities.js'; */ interface QueryTask { run: () => Promise; + cancel?: (err: Error) => void; } /** @@ -90,6 +91,14 @@ export class Client extends EventEmitter { async connect(): Promise; connect(callback: (err: Error | null) => void): void; connect(callback?: (err: Error | null) => void): Promise | void { + if (this.isConnected) { + if (callback) { + process.nextTick(() => callback(null)); + return; + } + return Promise.resolve(); + } + if (!this.connectPromise) { this.connectPromise = (async () => { try { @@ -113,6 +122,9 @@ export class Client extends EventEmitter { if (this.isConnected) { return; } + if (this.isEnded) { + throw new Error('Client was closed'); + } try { if (!this.dsn) { throw new Error( @@ -120,7 +132,9 @@ export class Client extends EventEmitter { ); } // TODO(PR 4 - Native CGO Bridge): Instantiate native CGO Spanner connection handle via spannerlib-node - this.isConnected = true; + if (!this.isEnded) { + this.isConnected = true; + } } catch (err) { throw enrichError(err, this.dialect); } @@ -148,6 +162,37 @@ export class Client extends EventEmitter { callback, ); + const sqlText = query.text; + const sqlValues = query.values; + + if (typeof sqlText !== 'string' || !sqlText.trim()) { + const err = enrichError( + new Error('Query text must be a non-empty string'), + this.dialect, + ); + dispatchQueryError(err, query, actualCallback); + const executionPromise = Promise.reject>(err); + executionPromise.catch(() => {}); + query.setPromise(executionPromise); + return query; + } + + if ( + sqlValues !== undefined && + sqlValues !== null && + !Array.isArray(sqlValues) + ) { + const err = enrichError( + new Error('Query values must be an Array'), + this.dialect, + ); + dispatchQueryError(err, query, actualCallback); + const executionPromise = Promise.reject>(err); + executionPromise.catch(() => {}); + query.setPromise(executionPromise); + return query; + } + let resolveTask!: (val: QueryResult) => void; let rejectTask!: (err: unknown) => void; const executionPromise = new Promise>((res, rej) => { @@ -160,33 +205,6 @@ export class Client extends EventEmitter { const task: QueryTask> = { run: async () => { - const sqlText = query.text; - const sqlValues = query.values; - - if (typeof sqlText !== 'string' || !sqlText.trim()) { - const err = enrichError( - new Error('Query text must be a non-empty string'), - this.dialect, - ); - dispatchQueryError(err, query, actualCallback); - rejectTask(err); - throw err; - } - - if ( - sqlValues !== undefined && - sqlValues !== null && - !Array.isArray(sqlValues) - ) { - const err = enrichError( - new Error('Query values must be an Array'), - this.dialect, - ); - dispatchQueryError(err, query, actualCallback); - rejectTask(err); - throw err; - } - try { if (this.isEnded) { throw new Error('Client was closed'); @@ -217,6 +235,10 @@ export class Client extends EventEmitter { throw enriched; } }, + cancel: (err: Error) => { + dispatchQueryError(err, query, actualCallback); + rejectTask(err); + }, }; this.queryQueue.push(task as QueryTask); @@ -234,13 +256,12 @@ export class Client extends EventEmitter { } if (this.isExecuting) return; this.isExecuting = true; - const task = this.queryQueue[0]; + const task = this.queryQueue.shift()!; try { await task.run(); } catch { // Handled in executionPromise reject } finally { - this.queryQueue.shift(); this.isExecuting = false; void this.processQueue(); } @@ -283,7 +304,14 @@ export class Client extends EventEmitter { private async _doEnd(): Promise { this.isEnded = true; this.isConnected = false; - // Clear pending queries in queue to prevent execution after client close + // Cancel pending queries in queue to prevent execution after client close + const pendingTasks = this.queryQueue; this.queryQueue = []; + const closeError = new Error('Client was closed'); + for (const task of pendingTasks) { + if (task.cancel) { + task.cancel(closeError); + } + } } } diff --git a/handwritten/spanner-driver/src/lib/pool.ts b/handwritten/spanner-driver/src/lib/pool.ts index 7c6d7d76041..712ccc60b98 100644 --- a/handwritten/spanner-driver/src/lib/pool.ts +++ b/handwritten/spanner-driver/src/lib/pool.ts @@ -122,8 +122,14 @@ export class Pool extends EventEmitter { query.text ?? '', query.values, ); - queryForClient.on('row', row => query.emit('row', row)); - queryForClient.on('fields', fields => query.emit('fields', fields)); + queryForClient.rowMode = query.rowMode; + queryForClient.types = query.types; + void queryForClient.on('row', row => { + void query.emit('row', row); + }); + void queryForClient.on('fields', fields => { + void query.emit('fields', fields); + }); let result: QueryResult; let queryErr: Error | undefined; diff --git a/handwritten/spanner-driver/src/lib/query.ts b/handwritten/spanner-driver/src/lib/query.ts index e8aacd581a7..9ffc6c520f3 100644 --- a/handwritten/spanner-driver/src/lib/query.ts +++ b/handwritten/spanner-driver/src/lib/query.ts @@ -154,7 +154,9 @@ export class Query extends EventEmitter { */ public setPromise(promise: Promise): void { if (this.promiseResolver) { - promise.then(this.promiseResolver.resolve, this.promiseResolver.reject); + const resolver = this.promiseResolver; + this.promiseResolver = undefined; + promise.then(resolver.resolve, resolver.reject); } else { this.promise = promise; } diff --git a/handwritten/spanner-driver/src/lib/utilities.ts b/handwritten/spanner-driver/src/lib/utilities.ts index ce9336abfa5..9acc11ee8b5 100644 --- a/handwritten/spanner-driver/src/lib/utilities.ts +++ b/handwritten/spanner-driver/src/lib/utilities.ts @@ -33,20 +33,22 @@ export function normalizeQueryArgs>( query: Query>; actualCallback: QueryCallback> | undefined; } { - const query = - queryText instanceof Query - ? queryText - : new Query>(queryText, values as unknown[], callback); - - let actualCallback: QueryCallback> | undefined = - query.callback; - if (typeof values === 'function') { - actualCallback = values as QueryCallback>; - } else if (typeof callback === 'function') { - actualCallback = callback; + let query: Query>; + if (queryText instanceof Query) { + query = queryText; + if (Array.isArray(values)) { + query.values = values; + } + if (typeof values === 'function') { + query.callback = values; + } else if (typeof callback === 'function') { + query.callback = callback; + } + } else { + query = new Query>(queryText, values as unknown[], callback); } - return {query, actualCallback}; + return {query, actualCallback: query.callback}; } /** diff --git a/handwritten/spanner-driver/test/unit/client_test.ts b/handwritten/spanner-driver/test/unit/client_test.ts index 099662e7912..970ae93d8e4 100644 --- a/handwritten/spanner-driver/test/unit/client_test.ts +++ b/handwritten/spanner-driver/test/unit/client_test.ts @@ -57,6 +57,15 @@ describe('Client Class', () => { }); }); + it('should invoke callback with error when client.connect(cb) fails', done => { + const client = new Client({}); + client.connect(err => { + assert.strictEqual(err instanceof DatabaseError, true); + assert.match(err!.message, /Invalid Spanner connection configuration/); + done(); + }); + }); + it('should execute query with async/await and return QueryResult', async () => { const client = new Client({ project: 'p', @@ -144,12 +153,12 @@ describe('Client Class', () => { let callbackInvoked = false; const q = new Query(''); - q.on('error', () => { + void q.on('error', () => { errorEventEmitted = true; }); await new Promise(resolve => { - client.query(q, undefined, err => { + void client.query(q, undefined, err => { assert.strictEqual(err instanceof DatabaseError, true); callbackInvoked = true; setTimeout(resolve, 20); @@ -212,6 +221,17 @@ describe('Client Class', () => { ); }); + it('should handle multiple client.end() calls safely without error', async () => { + const client = new Client({ + project: 'p', + instance: 'i', + database: 'd', + }); + await client.connect(); + await client.end(); + await assert.doesNotReject(async () => client.end()); + }); + it('should clear pending query queue when client.end() is called', async () => { const client = new Client({ project: 'p', @@ -237,6 +257,59 @@ describe('Client Class', () => { } }); + it('should reject pending queries in queryQueue with Client was closed error when client.end() is called', async () => { + const client = new Client({ + project: 'p', + instance: 'i', + database: 'd', + }); + let finishConnect!: () => void; + const connectGate = new Promise(resolve => { + finishConnect = resolve; + }); + const origDoConnect = ( + client as unknown as {_doConnect: () => Promise} + )._doConnect.bind(client); + (client as unknown as {_doConnect: () => Promise})._doConnect = + async () => { + await connectGate; + return origDoConnect(); + }; + + const p1 = client.query('SELECT 1'); + const p2 = client.query('SELECT 2'); + const p3 = client.query('SELECT 3'); + + p1.catch(() => {}); + + await client.end(); + finishConnect(); + + assert.strictEqual( + (client as unknown as {queryQueue: unknown[]}).queryQueue.length, + 0, + ); + await assert.rejects(async () => p2, /Client was closed/); + await assert.rejects(async () => p3, /Client was closed/); + }); + + it('should reject connect() calls on an ended client and handle concurrent connect/end', async () => { + const client = new Client({ + project: 'p', + instance: 'i', + database: 'd', + }); + const connectPromise = client.connect(); + await client.end(); + try { + await connectPromise; + } catch { + // Ignored if race rejected + } + assert.strictEqual(client.isConnected, false); + await assert.rejects(async () => client.connect(), /Client was closed/); + }); + it('should emit end event on Query when query execution completes', done => { const client = new Client({ project: 'p', @@ -245,7 +318,7 @@ describe('Client Class', () => { }); let endEventEmitted = false; const q = client.query('SELECT 1'); - q.on('end', res => { + void q.on('end', res => { endEventEmitted = true; assert.strictEqual(res.command, 'SELECT'); void client.end().then(() => { @@ -274,10 +347,34 @@ describe('Client Class', () => { database: 'd', }); const q = new Query(''); - q.on('error', err => { + void q.on('error', err => { assert.strictEqual(err instanceof DatabaseError, true); void client.end().then(() => done()); }); void client.query(q).catch(() => {}); }); + + it('should delegate release(cb) to end(cb) using callback syntax', done => { + const client = new Client({ + project: 'p', + instance: 'i', + database: 'd', + }); + client.release(err => { + assert.strictEqual(err, null); + assert.strictEqual(client.isConnected, false); + done(); + }); + }); + + it('should emit error event on Query when client.query() connection fails without callback', done => { + const client = new Client({}); + const q = client.query('SELECT 1'); + void q.on('error', err => { + assert.strictEqual(err instanceof DatabaseError, true); + assert.match(err.message, /Invalid Spanner connection configuration/); + done(); + }); + void q.catch(() => {}); + }); }); diff --git a/handwritten/spanner-driver/test/unit/pool_test.ts b/handwritten/spanner-driver/test/unit/pool_test.ts index 69619170b48..b6fd2986ff4 100644 --- a/handwritten/spanner-driver/test/unit/pool_test.ts +++ b/handwritten/spanner-driver/test/unit/pool_test.ts @@ -131,7 +131,7 @@ describe('Pool Class', () => { let errorEventEmitted = false; const q = new Query(''); - q.on('error', () => { + void q.on('error', () => { errorEventEmitted = true; }); @@ -171,6 +171,22 @@ describe('Pool Class', () => { } }); + it('should reject pool.query() calls after pool.end() and invoke callback with error', done => { + const pool = new Pool({ + project: 'p', + instance: 'i', + database: 'd', + }); + void pool.end().then(() => { + void pool.query('SELECT 1', (err, res) => { + assert.strictEqual(res, undefined); + assert.strictEqual(err instanceof Error, true); + assert.match(err!.message, /Cannot acquire client from ending pool/); + done(); + }); + }); + }); + it('should ensure client is released before user callback executes in pool.query()', done => { const pool = new Pool({ project: 'p', @@ -180,16 +196,27 @@ describe('Pool Class', () => { let clientReleased = false; // Override _doConnect to track client.release call sequence - const originalDoConnect = (pool as unknown as {_doConnect: () => Promise<{release: () => Promise; query: (q: unknown, v?: unknown[]) => Promise<{command: string; rows: []; fields: []; rowCount: 0}>}>})._doConnect.bind(pool); - (pool as unknown as {_doConnect: () => Promise})._doConnect = async () => { - const client = await originalDoConnect(); - const originalRelease = client.release.bind(client); - client.release = async () => { - clientReleased = true; - await originalRelease(); + const originalDoConnect = ( + pool as unknown as { + _doConnect: () => Promise<{ + release: () => Promise; + query: ( + q: unknown, + v?: unknown[], + ) => Promise<{command: string; rows: []; fields: []; rowCount: 0}>; + }>; + } + )._doConnect.bind(pool); + (pool as unknown as {_doConnect: () => Promise})._doConnect = + async () => { + const client = await originalDoConnect(); + const originalRelease = client.release.bind(client); + client.release = async () => { + clientReleased = true; + await originalRelease(); + }; + return client; }; - return client; - }; void pool.query('SELECT 1', (err, res) => { assert.strictEqual(err, null); @@ -221,10 +248,20 @@ describe('Pool Class', () => { database: 'd', }); const q = new Query(''); - q.on('error', err => { + void q.on('error', err => { assert.strictEqual(err instanceof Error, true); void pool.end().then(() => done()); }); void pool.query(q).catch(() => {}); }); + + it('should emit error event on Query when pool.query() connection acquisition fails without callback', done => { + const pool = new Pool({}); + const q = new Query('SELECT 1'); + void q.on('error', err => { + assert.match(err.message, /Invalid Spanner connection configuration/); + done(); + }); + void pool.query(q).catch(() => {}); + }); }); diff --git a/handwritten/spanner-driver/test/unit/query_test.ts b/handwritten/spanner-driver/test/unit/query_test.ts index 1647b09fc27..6bd9664ee68 100644 --- a/handwritten/spanner-driver/test/unit/query_test.ts +++ b/handwritten/spanner-driver/test/unit/query_test.ts @@ -72,6 +72,22 @@ describe('Query Class', () => { assert.strictEqual(res.rowCount, 1); }); + it('should clear promiseResolver after setPromise is called to prevent memory retention on multiple calls', async () => { + const q = new Query<{rowCount: number}>('SELECT 1'); + assert.notStrictEqual( + (q as unknown as {promiseResolver: unknown}).promiseResolver, + undefined, + ); + q.setPromise(Promise.resolve({rowCount: 1})); + assert.strictEqual( + (q as unknown as {promiseResolver: unknown}).promiseResolver, + undefined, + ); + q.setPromise(Promise.resolve({rowCount: 2})); + const res = await q; + assert.strictEqual(res.rowCount, 2); + }); + it('should reject thenable promise on .catch()', async () => { const q = new Query('SELECT 1'); q.setPromise(Promise.reject(new Error('Query failed'))); From ad29394a1ba1bf47ff5f97e6f13d265e12df21be Mon Sep 17 00:00:00 2001 From: Surbhi Garg Date: Fri, 31 Jul 2026 15:22:01 +0530 Subject: [PATCH 5/5] removed cloud reference from comments --- handwritten/spanner-driver/.repo-metadata.json | 2 +- handwritten/spanner-driver/README.md | 4 ++-- handwritten/spanner-driver/package.json | 2 +- handwritten/spanner-driver/src/lib/client.ts | 6 +++--- handwritten/spanner-driver/src/lib/config.ts | 2 +- handwritten/spanner-driver/src/lib/constants.ts | 2 +- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/handwritten/spanner-driver/.repo-metadata.json b/handwritten/spanner-driver/.repo-metadata.json index cdb3c8f269c..cbdc9c5216b 100644 --- a/handwritten/spanner-driver/.repo-metadata.json +++ b/handwritten/spanner-driver/.repo-metadata.json @@ -1,6 +1,6 @@ { "name": "spanner-driver", - "name_pretty": "Cloud Spanner Driver", + "name_pretty": "Spanner Driver", "product_documentation": "https://cloud.google.com/spanner/docs/", "client_documentation": "https://cloud.google.com/nodejs/docs/reference/spanner/latest", "issue_tracker": "https://issuetracker.google.com/issues?q=componentid:190851%2B%20status:open", diff --git a/handwritten/spanner-driver/README.md b/handwritten/spanner-driver/README.md index 297a60c80f2..7e6b8c5dc0a 100644 --- a/handwritten/spanner-driver/README.md +++ b/handwritten/spanner-driver/README.md @@ -1,6 +1,6 @@ -# Google Cloud Spanner Node.js Driver (`@google-cloud/spanner-driver`) +# Google Spanner Node.js Driver (`@google-cloud/spanner-driver`) -The `@google-cloud/spanner-driver` package provides a high-performance, `node-postgres` (`pg`) compatible client and connection pool interface for Google Cloud Spanner. It bridges Node.js applications directly to Spanner using a native Go CGO engine, delivering full PostgreSQL dialect support. +The `@google-cloud/spanner-driver` package provides a high-performance, `node-postgres` (`pg`) compatible client and connection pool interface for Google Spanner. It bridges Node.js applications directly to Spanner using a native Go CGO engine, delivering full PostgreSQL dialect support. [![npm version](https://img.shields.io/npm/v/@google-cloud/spanner-driver.svg)](https://www.npmjs.com/package/@google-cloud/spanner-driver) [![License](https://img.shields.io/badge/license-Apache%202.0-blue.svg)](LICENSE) diff --git a/handwritten/spanner-driver/package.json b/handwritten/spanner-driver/package.json index 5413cb7b74d..00539eaa4c1 100644 --- a/handwritten/spanner-driver/package.json +++ b/handwritten/spanner-driver/package.json @@ -1,7 +1,7 @@ { "name": "@google-cloud/spanner-driver", "version": "0.1.0", - "description": "Node.js driver for Google Cloud Spanner", + "description": "Node.js driver for Google Spanner", "repository": { "type": "git", "url": "https://github.com/googleapis/google-cloud-node.git", diff --git a/handwritten/spanner-driver/src/lib/client.ts b/handwritten/spanner-driver/src/lib/client.ts index 05275b32c07..cbfc8ea5caa 100644 --- a/handwritten/spanner-driver/src/lib/client.ts +++ b/handwritten/spanner-driver/src/lib/client.ts @@ -30,7 +30,7 @@ interface QueryTask { } /** - * Client class representing a single database connection to Google Cloud Spanner. + * Client class representing a single database connection to Google Spanner. * * Handles DSN resolution, connection lifecycle (`connect`/`end`/`release`), sequential query * execution, transaction state tracking (`txStatus`), and dialect-aware error enrichment. @@ -83,7 +83,7 @@ export class Client extends EventEmitter { } /** - * Establishes a connection to Google Cloud Spanner using the resolved DSN. + * Establishes a connection to Google Spanner using the resolved DSN. * Supports both Promise (`await client.connect()`) and Node callback (`client.connect(cb)`) forms. * * @returns Promise resolving when connection is established, or void if callback is passed. @@ -141,7 +141,7 @@ export class Client extends EventEmitter { } /** - * Executes a SQL query against Google Cloud Spanner. + * Executes a SQL query against Google Spanner. * Supports Promises (`await client.query(sql)`), callbacks (`client.query(sql, cb)`), * and streaming row events (`client.query(sql).on('row', cb)`). * diff --git a/handwritten/spanner-driver/src/lib/config.ts b/handwritten/spanner-driver/src/lib/config.ts index 4170a1bd60c..bc256cc308c 100644 --- a/handwritten/spanner-driver/src/lib/config.ts +++ b/handwritten/spanner-driver/src/lib/config.ts @@ -13,7 +13,7 @@ // limitations under the License. /** - * Configuration options for establishing a connection to Google Cloud Spanner. + * Configuration options for establishing a connection to Google Spanner. */ export interface ClientConfig { /** Connection DSN string or postgresql:// URL. */ diff --git a/handwritten/spanner-driver/src/lib/constants.ts b/handwritten/spanner-driver/src/lib/constants.ts index 8a6528c45c9..735d928b774 100644 --- a/handwritten/spanner-driver/src/lib/constants.ts +++ b/handwritten/spanner-driver/src/lib/constants.ts @@ -13,7 +13,7 @@ // limitations under the License. /** - * Supported SQL dialects for Google Cloud Spanner. + * Supported SQL dialects for Google Spanner. * Note: Currently 'pg' (PostgreSQL) dialect is supported. */ export type Dialect = 'pg' | 'googlesql';