test: fix the flakey tests for bigtable readrows

[danieljbruce]

Description

Resolves flaky test timeouts and hanging streams in the Bigtable ReadRows test suite:

• Stream Lifecycle & Backpressure Handling: In handwritten/bigtable/test-common/utils/readRowsImpl.ts, updated the backpressure wait logic to listen for stream termination events (close, error, finish) in addition to drain. This ensures pending promises unblock immediately if the stream terminates while awaiting backpressure.
• Client Cancellation & Disconnection: Added an onStreamEnded handler to ReadRowsImpl to mark requests as cancelled and unblock waiting handlers on close and error events alongside cancelled, preventing chunk-generation loops from hanging when a client drops or finishes early.
• Test Timeouts & Setup: In handwritten/bigtable/test/readrows.ts, increased setWindowsTestTimeout from 60s to 200s and applied it to tests processing large chunk volumes under heavy CI load (such as Windows runners). Added explicit projectId: 'fake-project' during client initialization to avoid unnecessary metadata server lookups.
• Documentation: Added explanatory comments throughout both files clarifying stream lifecycle management, backpressure resolution, and test timeout rationale.

Impact

• Test Stability: Eliminates intermittent timeout failures in Bigtable unit and integration tests across CI environments.
• Resource Cleanup: Prevents leaked or hung promises in test mock servers when streaming requests are aborted or closed prematurely.
• Scope: Internal test utilities and test suite only; no breaking changes or impact to production runtime code.

[gemini-code-assist]

Code Review

This pull request improves stream termination and backpressure handling in the Bigtable test utilities to prevent hanging, and increases test timeouts to reduce flakiness on slow CI environments. Feedback highlights two critical issues: a potential event listener leak due to uncleaned .once() listeners on the stream, and a race condition where onStreamEnded could be invoked multiple times and prematurely resolve subsequent backpressure promises.

[gemini-code-assist]

Registering multiple .once() listeners on the same stream to resolve a single promise can lead to an event listener leak. When one of the events (e.g., 'drain') fires, the promise resolves, but the other listeners ('close', 'error', 'finish') remain registered on the stream. Over time, this can accumulate and trigger a MaxListenersExceededWarning or cause unexpected behavior.

To prevent this, we should clean up all registered listeners when the promise resolves.

          await new Promise<void>(resolve => {
            const onEvent = () => {
              stream.off('drain', onEvent);
              stream.off('close', onEvent);
              stream.off('error', onEvent);
              stream.off('finish', onEvent);
              resolve();
            };
            this.stopWaiting = onEvent;
            stream.once('drain', onEvent);
            stream.once('close', onEvent);
            stream.once('error', onEvent);
            stream.once('finish', onEvent);
          });

[danieljbruce]

Solved

[gemini-code-assist]

The onStreamEnded handler can be invoked multiple times. For example, when a 'cancelled' event is received, it calls onStreamEnded() and then synchronously emits an 'error' event (stream.emit('error', ...)), which triggers the 'error' listener and calls onStreamEnded() a second time.

If onStreamEnded is called multiple times, and the request handler loop has already moved on to a new backpressure promise (overwriting this.stopWaiting), the second invocation will prematurely resolve the new promise.

To prevent this race condition, use a boolean flag to ensure onStreamEnded only executes its logic once.

    let streamEnded = false;
    const onStreamEnded = () => {
      if (streamEnded) return;
      streamEnded = true;
      readRowsRequestHandler.cancelled = true;
      readRowsRequestHandler.stopWaiting();
    };

[danieljbruce]

Pushed a change to address this.

[shivanee-p]

/gemini review

[gemini-code-assist]

Code Review

This pull request improves stream handling and test stability in the Bigtable test utilities. Key changes include listening to termination events on backpressure to prevent hanging, cleaning up event listeners to avoid leaks, and increasing test timeouts to prevent flaky CI failures. Additionally, an optimization was added to skip proto compilation if the generated files already exist. The review feedback suggests resetting 'this.stopWaiting' to a no-op function to prevent potential memory leaks, and updating the proto existence check in 'compile-protos.sh' to also verify 'protos.json' to avoid incomplete builds.