From 0e4e0d44a756ae194cc959e70f74d339937130ea Mon Sep 17 00:00:00 2001 From: feywind <57276408+feywind@users.noreply.github.com> Date: Mon, 9 Mar 2026 15:29:32 -0400 Subject: [PATCH 1/7] tests: roll cumulative test fixes from https://github.com/googleapis/nodejs-bigtable/tree/conformance-fixes-2 (manually due to merging errors) --- src/index.ts | 27 ++++++++++- src/row.ts | 5 +- src/tabular-api-surface.ts | 5 ++ testproxy/known_failures.txt | 20 -------- testproxy/services/close-client.ts | 11 +++++ testproxy/services/create-client.ts | 8 ++++ testproxy/services/mutate-row.ts | 29 +++++++---- testproxy/services/read-row.ts | 31 ++++++++---- testproxy/services/read-rows.ts | 3 +- testproxy/services/sample-row-keys.ts | 21 +++++--- testproxy/services/utils/bigtable-client.ts | 13 +++-- testproxy/services/utils/log.ts | 18 +++++++ .../services/utils/request/sampleRowKeys.ts | 48 ------------------- 13 files changed, 138 insertions(+), 101 deletions(-) create mode 100644 testproxy/services/utils/log.ts diff --git a/src/index.ts b/src/index.ts index 8f52ca98e..834cbc7c7 100644 --- a/src/index.ts +++ b/src/index.ts @@ -487,6 +487,7 @@ export class Bigtable { static Cluster: Cluster; _metricsConfigManager: ClientSideMetricsConfigManager; admin: admin.BigtableAdmin; + closed = false; constructor(options: BigtableOptions = {}) { // Determine what scopes are needed. @@ -904,6 +905,17 @@ export class Bigtable { let gaxStream: gax.CancellableStream; let stream: AbortableDuplex; + if (this.closed) { + callback?.({ + name: 'Closed', + message: 'Bigtable internal client is closed', + code: grpc.status.ABORTED, + details: 'Bigtable internal client is closed', + metadata: new grpc.Metadata(), + }); + return; + } + const prepareGaxRequest = ( callback: (err: Error | null, fn?: Function) => void, ) => { @@ -1021,11 +1033,22 @@ export class Bigtable { * Close all bigtable clients. New requests will be rejected but it will not * kill connections with pending requests. */ - close(): Promise { + async close(): Promise { + // Close all of the clients. const combined = Object.keys(this.api).map(clientType => this.api[clientType].close(), ); - return Promise.all(combined); + const results = await Promise.all(combined); + + // Clear them out of our cache. + Object.keys(this.api).forEach(clientType => { + delete this.api[clientType]; + }); + + // Mark as closed. + this.closed = true; + + return results; } /** diff --git a/src/row.ts b/src/row.ts index 94155211d..b97177110 100644 --- a/src/row.ts +++ b/src/row.ts @@ -30,7 +30,7 @@ import {CallOptions} from 'google-gax'; import {ServiceError} from 'google-gax'; import {google} from '../protos/protos'; import {RowDataUtils, RowProperties} from './row-data-utils'; -import {TabularApiSurface} from './tabular-api-surface'; +import {GetRowsOptions, TabularApiSurface} from './tabular-api-surface'; import {getRowsInternal} from './utils/getRowsInternal'; import { MethodName, @@ -667,9 +667,10 @@ export class Row { filter = arrify(filter).concat(options.filter); } - const getRowsOptions = Object.assign({}, options, { + const getRowsOptions: GetRowsOptions = Object.assign({}, options, { keys: [this.id], filter, + limit: 1, }); const metricsCollector = diff --git a/src/tabular-api-surface.ts b/src/tabular-api-surface.ts index f84a4d25a..7e2abd907 100644 --- a/src/tabular-api-surface.ts +++ b/src/tabular-api-surface.ts @@ -476,6 +476,11 @@ Please use the format 'prezzy' or '${instance.name}/tables/prezzy'.`); reqOpts, gaxOpts: Object.assign({}, gaxOptions), }); + if (!requestStream) { + throw new Error( + 'Failed to create request stream -- is the client already closed?', + ); + } metricsCollector.wrapRequest(requestStream); const stream = pumpify.obj([requestStream, rowKeysStream]); stream.on('end', () => { diff --git a/testproxy/known_failures.txt b/testproxy/known_failures.txt index 83ee9971e..e69de29bb 100644 --- a/testproxy/known_failures.txt +++ b/testproxy/known_failures.txt @@ -1,20 +0,0 @@ -TestMutateRow_Generic_DeadlineExceeded\| -TestMutateRow_Generic_CloseClient\| -TestMutateRow_Generic_DeadlineExceeded\| -TestReadModifyWriteRow_Generic_DeadlineExceeded\| -TestReadRow_Generic_DeadlineExceeded\| -TestMutateRows_Retry_WithRoutingCookie\| -TestReadRow_Generic_DeadlineExceeded\| -TestReadRow_Retry_WithRoutingCookie\| -TestReadRow_Retry_WithRetryInfo\| -TestReadRows_ReverseScans_FeatureFlag_Enabled\| -TestReadRows_NoRetry_OutOfOrderError_Reverse\| -TestReadRows_Retry_LastScannedRow_Reverse\| -TestReadRows_Retry_WithRoutingCookie\| -TestReadRows_Retry_WithRoutingCookie_MultipleErrorResponses\| -TestReadRows_Retry_WithRetryInfo\| -TestReadRows_Retry_WithRetryInfo_MultipleErrorResponse\| -TestSampleRowKeys_Retry_WithRoutingCookie\| -TestSampleRowKeys_Generic_CloseClient\| -TestSampleRowKeys_Generic_Headers\| -TestSampleRowKeys_NoRetry_NoEmptyKey\| diff --git a/testproxy/services/close-client.ts b/testproxy/services/close-client.ts index 6698f8509..72dfa9298 100644 --- a/testproxy/services/close-client.ts +++ b/testproxy/services/close-client.ts @@ -14,9 +14,12 @@ import {google} from '../protos/protos'; import {ClientImplMaker, normalizeCallback} from './utils'; +import {closeBigtableClient} from './utils/bigtable-client'; type ICloseClientRequest = google.bigtable.testproxy.ICloseClientRequest; type ICloseClientResponse = google.bigtable.testproxy.ICloseClientResponse; +import {log} from './utils/log'; + export const closeClient: ClientImplMaker< ICloseClientRequest, ICloseClientResponse @@ -26,8 +29,16 @@ export const closeClient: ClientImplMaker< const {clientId} = request; const bigtable = clientMap.get(clientId!); + log.info( + 'close client %s (%s)', + clientId, + bigtable ? 'exists' : "doesn't exist", + ); + if (bigtable) { + await closeBigtableClient(bigtable); await bigtable.close(); + log.info('client %s closed', clientId); } return {}; }); diff --git a/testproxy/services/create-client.ts b/testproxy/services/create-client.ts index f9b958ef3..c2a522956 100644 --- a/testproxy/services/create-client.ts +++ b/testproxy/services/create-client.ts @@ -18,6 +18,7 @@ import {google} from '../protos/protos'; import * as grpc from '@grpc/grpc-js'; import {Bigtable} from '../../src'; import {createBigtableClient} from './utils/bigtable-client'; +import {log} from './utils/log'; function durationToMilliseconds( duration: google.protobuf.Duration | google.protobuf.IDuration, @@ -68,6 +69,12 @@ export const createClient: ClientImplMaker< ); } + log.info( + 'create client %s (%s)', + clientId, + clientMap.has(clientId) ? 'exists' : "doesn't exist", + ); + if (clientMap.has(clientId)) { throw Object.assign(new Error(`Client ${clientId} already exists`), { code: grpc.status.ALREADY_EXISTS, @@ -100,6 +107,7 @@ export const createClient: ClientImplMaker< appProfileId: appProfileId!, clientConfig, }; + log.info('created bigtable client %s', clientId); const bigtable = new Bigtable(options); createBigtableClient(bigtable); clientMap.set(clientId!, bigtable); diff --git a/testproxy/services/mutate-row.ts b/testproxy/services/mutate-row.ts index daf8f50cc..a50a11e0b 100644 --- a/testproxy/services/mutate-row.ts +++ b/testproxy/services/mutate-row.ts @@ -34,14 +34,25 @@ export const mutateRow: ClientImplMaker< const appProfileId = bigtable.appProfileId; const client = getBigtableClient(bigtable); - await client.mutateRow({ - appProfileId, - mutations, - tableName, - rowKey, - }); + try { + await client.mutateRow({ + appProfileId, + mutations, + tableName, + rowKey, + }); - return { - status: {code: grpc.status.OK, details: []}, - }; + return { + status: {code: grpc.status.OK, details: []}, + }; + } catch (e) { + const error = e as GoogleError; + return { + status: { + code: error.code ? error.code : grpc.status.UNKNOWN, + message: error.message, + details: [], + }, + }; + } }); diff --git a/testproxy/services/read-row.ts b/testproxy/services/read-row.ts index cdbbd2ea6..de742d55e 100644 --- a/testproxy/services/read-row.ts +++ b/testproxy/services/read-row.ts @@ -24,6 +24,7 @@ import { getRowResponse, getTableInfo, } from './utils'; +import {GoogleError} from 'google-gax'; export const readRow: ClientImplMaker = ({ clientMap, @@ -32,15 +33,27 @@ export const readRow: ClientImplMaker = ({ const {clientId, rowKey, tableName} = rawRequest.request; const columns = {}; - const bigtable = clientMap.get(clientId!); - const table = getTableInfo(bigtable, tableName!); - const row = table.row(rowKey!); + try { + const bigtable = clientMap.get(clientId!); + const table = getTableInfo(bigtable, tableName!); + const row = table.row(rowKey!); - const res = await row.get(columns); - const firstRow = getRowResponse(res[0]); + const res = await row.get(columns); + const firstRow = getRowResponse(res[0]); - return { - status: {code: grpc.status.OK, details: []}, - row: firstRow, - }; + return { + status: {code: grpc.status.OK, details: []}, + row: firstRow, + }; + } catch (e) { + const error = e as GoogleError; + return { + status: { + code: error.code || grpc.status.FAILED_PRECONDITION, + // e.details must be in an empty array for the test runner to return the status. This is tracked in b/383096533. + details: [], + message: error.message, + }, + }; + } }); diff --git a/testproxy/services/read-rows.ts b/testproxy/services/read-rows.ts index 27b45f063..d1e9f02d3 100644 --- a/testproxy/services/read-rows.ts +++ b/testproxy/services/read-rows.ts @@ -87,7 +87,8 @@ export const readRows: ClientImplMaker = ({ const error = e as GoogleError; return { status: { - code: error.code, + // This might be zero/undefined if it's a disconnected client error. + code: error.code || grpc.status.FAILED_PRECONDITION, // e.details must be in an empty array for the test runner to return the status. This is tracked in b/383096533. details: [], message: error.message, diff --git a/testproxy/services/sample-row-keys.ts b/testproxy/services/sample-row-keys.ts index f4df2cf9b..12dfabc13 100644 --- a/testproxy/services/sample-row-keys.ts +++ b/testproxy/services/sample-row-keys.ts @@ -14,10 +14,10 @@ import * as grpc from '@grpc/grpc-js'; import {GoogleError} from 'google-gax'; -import {getSRKRequest} from './utils/request/sampleRowKeys'; import {ClientImplMaker, normalizeCallback} from './utils'; import {google} from '../protos/protos'; +import {log} from './utils/log'; type ISampleRowKeysRequest = google.bigtable.testproxy.ISampleRowKeysRequest; type ISampleRowKeysResult = google.bigtable.testproxy.ISampleRowKeysResult; @@ -28,25 +28,32 @@ export const sampleRowKeys: ClientImplMaker< normalizeCallback(async rawRequest => { const {request} = rawRequest; const {clientId, request: sampleRowKeysRequest} = request; - const {appProfileId, tableName} = sampleRowKeysRequest!; + const {tableName} = sampleRowKeysRequest!; const bigtable = clientMap.get(clientId!); - bigtable.appProfileId = appProfileId!; + + const [, , , instanceId, , tableId] = tableName!.split('/'); + const instance = bigtable.instance(instanceId); + const table = instance.table(tableId); try { - const response = await getSRKRequest(bigtable, {appProfileId, tableName}); + const response = await table.sampleRowKeys(); + log.info('sampleRowKeys response %o', response); return { status: {code: grpc.status.OK, details: []}, - response, + samples: response[0].map(sample => ({ + rowKey: sample.key, + offsetBytes: sample.offset, + })), }; } catch (e) { const error = e as GoogleError; - console.error('Error:', error.code); return { status: { - code: error.code, + // This might be zero/undefined if it's a disconnected client error. + code: error.code || grpc.status.FAILED_PRECONDITION, details: [], }, }; diff --git a/testproxy/services/utils/bigtable-client.ts b/testproxy/services/utils/bigtable-client.ts index 3ab7947e2..9a693f2af 100644 --- a/testproxy/services/utils/bigtable-client.ts +++ b/testproxy/services/utils/bigtable-client.ts @@ -37,17 +37,24 @@ export function createBigtableClient(bigtable: Bigtable) { bigtableAny[v2] = new BigtableClient(bigtable.options.BigtableClient); } -export function getBigtableClient(bigtable: Bigtable) { +export function getBigtableClient(bigtable: Bigtable): BigtableClient { // eslint-disable-next-line @typescript-eslint/no-explicit-any return (bigtable as any)[v2]; } -export async function deleteBigtableClient(bigtable: Bigtable) { +export async function closeBigtableClient(bigtable: Bigtable) { // eslint-disable-next-line @typescript-eslint/no-explicit-any const bigtableAny = bigtable as any; - const bigtableClient = bigtableAny[v2]; + const bigtableClient = bigtableAny[v2] as BigtableClient; await bigtableClient.close(); +} + +export async function deleteBigtableClient(bigtable: Bigtable) { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const bigtableAny = bigtable as any; + + await closeBigtableClient(bigtable); delete bigtableAny[v2]; } diff --git a/testproxy/services/utils/log.ts b/testproxy/services/utils/log.ts new file mode 100644 index 000000000..8493d2b6a --- /dev/null +++ b/testproxy/services/utils/log.ts @@ -0,0 +1,18 @@ +// Copyright 2025-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 +// +// https://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 {loggingUtils as logging} from 'google-gax'; + +// Debug logger to use with the testproxy. +export const log = logging.log('cbt-testproxy'); diff --git a/testproxy/services/utils/request/sampleRowKeys.ts b/testproxy/services/utils/request/sampleRowKeys.ts index a2423ed58..e69de29bb 100644 --- a/testproxy/services/utils/request/sampleRowKeys.ts +++ b/testproxy/services/utils/request/sampleRowKeys.ts @@ -1,48 +0,0 @@ -// Copyright 2025 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 -// -// https://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 protos from '../../../protos/protos'; - -type SRKRequest = protos.google.bigtable.v2.ISampleRowKeysRequest; -type SRKResponse = protos.google.bigtable.v2.ISampleRowKeysResponse; - -/** - * This function will create a request that can be passed into the - * handwritten ßsampleRowKeys method and calls the Gapic layer under the hood. - * - * @param {any} client Bigtable client - * @param {SRKRequest} request The sampleRowKeys request information - * that can be sent to the Gapic layer. - * @returns {Promise} A reponse that can be passed into the - * sampleRowKeys Gapic layer. - */ -export function getSRKRequest( - // eslint-disable-next-line @typescript-eslint/no-explicit-any - client: any, - request: SRKRequest, -): Promise { - const {tableName} = request; - - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const getTableInfo = (bigtable: any, tableName: any) => { - const [, , , instanceId, , tableId] = tableName.split('/'); - const instance = bigtable.instance(instanceId); - return instance.table(tableId); - }; - - const table = getTableInfo(client, tableName); - const sampleRowKeysResponse = table.sampleRowKeys(); - - return sampleRowKeysResponse; -} From 2ebaafb7d2b35fe43ac7383e362b48076a54e7ce Mon Sep 17 00:00:00 2001 From: feywind <57276408+feywind@users.noreply.github.com> Date: Mon, 9 Mar 2026 15:31:56 -0400 Subject: [PATCH 2/7] chore: remove empty file --- testproxy/services/utils/request/sampleRowKeys.ts | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 testproxy/services/utils/request/sampleRowKeys.ts diff --git a/testproxy/services/utils/request/sampleRowKeys.ts b/testproxy/services/utils/request/sampleRowKeys.ts deleted file mode 100644 index e69de29bb..000000000 From 0f4c86cb7f21ab9c09c9d8b2a569738cd87ab98e Mon Sep 17 00:00:00 2001 From: feywind <57276408+feywind@users.noreply.github.com> Date: Mon, 9 Mar 2026 17:38:14 -0400 Subject: [PATCH 3/7] tests: change 'closed' wording to match previously expected wording --- src/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/index.ts b/src/index.ts index 834cbc7c7..2ed30c2dc 100644 --- a/src/index.ts +++ b/src/index.ts @@ -908,9 +908,9 @@ export class Bigtable { if (this.closed) { callback?.({ name: 'Closed', - message: 'Bigtable internal client is closed', + message: 'The client has already been closed.', code: grpc.status.ABORTED, - details: 'Bigtable internal client is closed', + details: 'The client has already been closed.', metadata: new grpc.Metadata(), }); return; From ceab26269b65dc6fc1199b69310b5b024a98712b Mon Sep 17 00:00:00 2001 From: feywind <57276408+feywind@users.noreply.github.com> Date: Tue, 10 Mar 2026 17:35:30 -0400 Subject: [PATCH 4/7] tests: also handle the stream mode being closed already --- src/index.ts | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/index.ts b/src/index.ts index 2ed30c2dc..8a989a98f 100644 --- a/src/index.ts +++ b/src/index.ts @@ -906,14 +906,24 @@ export class Bigtable { let stream: AbortableDuplex; if (this.closed) { - callback?.({ - name: 'Closed', - message: 'The client has already been closed.', - code: grpc.status.ABORTED, - details: 'The client has already been closed.', - metadata: new grpc.Metadata(), - }); - return; + const error = Object.assign( + new Error('The client has already been closed.'), + { + name: 'Closed', + code: grpc.status.ABORTED, + details: 'The client has already been closed.', + metadata: new grpc.Metadata(), + } + ); + if (isStreamMode) { + stream = streamEvents(new PassThrough({objectMode: true})); + stream.abort = () => {}; + setImmediate(() => stream.destroy(error)); + return stream; + } else { + callback?.(error as ServiceError); + return; + } } const prepareGaxRequest = ( From fb13b0d2eea6220508d2ff8671182d587721e939 Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Tue, 10 Mar 2026 21:41:41 +0000 Subject: [PATCH 5/7] =?UTF-8?q?=F0=9F=A6=89=20Updates=20from=20OwlBot=20po?= =?UTF-8?q?st-processor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --- src/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/index.ts b/src/index.ts index 8a989a98f..35e8f5d4a 100644 --- a/src/index.ts +++ b/src/index.ts @@ -913,7 +913,7 @@ export class Bigtable { code: grpc.status.ABORTED, details: 'The client has already been closed.', metadata: new grpc.Metadata(), - } + }, ); if (isStreamMode) { stream = streamEvents(new PassThrough({objectMode: true})); From 8f206b56777fb6086a7c6ef8ebc8eea2ea521c23 Mon Sep 17 00:00:00 2001 From: feywind <57276408+feywind@users.noreply.github.com> Date: Wed, 11 Mar 2026 14:44:15 -0400 Subject: [PATCH 6/7] tests: remove extraneous bigtable.close() call --- testproxy/services/close-client.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/testproxy/services/close-client.ts b/testproxy/services/close-client.ts index 72dfa9298..76a2131e2 100644 --- a/testproxy/services/close-client.ts +++ b/testproxy/services/close-client.ts @@ -37,7 +37,6 @@ export const closeClient: ClientImplMaker< if (bigtable) { await closeBigtableClient(bigtable); - await bigtable.close(); log.info('client %s closed', clientId); } return {}; From 171443bad75724339abb52bad4bba8959fabf9e7 Mon Sep 17 00:00:00 2001 From: feywind <57276408+feywind@users.noreply.github.com> Date: Wed, 11 Mar 2026 14:54:12 -0400 Subject: [PATCH 7/7] tests: put back the other close line, and add a comment about why it looks duplicate --- testproxy/services/close-client.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/testproxy/services/close-client.ts b/testproxy/services/close-client.ts index 76a2131e2..1d19bb205 100644 --- a/testproxy/services/close-client.ts +++ b/testproxy/services/close-client.ts @@ -36,7 +36,10 @@ export const closeClient: ClientImplMaker< ); if (bigtable) { + // closeBigtableClient closes the BigtableClient, but not the Bigtable + // object itself. We need to close the Bigtable object as well. await closeBigtableClient(bigtable); + await bigtable.close(); log.info('client %s closed', clientId); } return {};