fix(pubsub): avoid invoking callback inside promise handler in getClient_ by danieljbruce · Pull Request #9544 · googleapis/google-cloud-node

danieljbruce · 2026-10-06T19:53:36Z

Description

Replaces the .then().catch() promise chain and promise/no-callback-in-promise ESLint disable comments in PubSub.prototype.getClient_ with an async IIFE and try / catch block.

Impact

Prevents synchronous errors thrown inside the getClient_ callback from being caught by .catch(callback) and invoking the callback a second time, while satisfying eslint-plugin-promise rules without lint suppressions (follow-up to #9539).

Changes

Refactored PubSub.prototype.getClient_ in handwritten/pubsub/src/pubsub.ts to await this.getClientAsync_(config) inside a try / catch block within a void (async () => { ... })() IIFE and invoke callback(null, client) outside the try / catch block.
Removed the two // eslint-disable-next-line promise/no-callback-in-promise comments in handwritten/pubsub/src/pubsub.ts.

Testing

Verified existing getClient_ and request unit tests in handwritten/pubsub/test/pubsub.ts pass (105 passing).
Verified GIT_DIFF_ARG=upstream/main node ./bin/linter.mjs --strict passes with zero ESLint errors or warnings and passes TypeScript type checking.

Alternatives

Keep the .then().catch() chain with eslint-disable-next-line promise/no-callback-in-promise comments: rejected because if callback(null, client) throws synchronously, .catch(callback) catches that error and invokes callback a second time.

…ent_

gemini-code-assist

Code Review

This pull request refactors the getClient_ method in pubsub.ts to use an asynchronous IIFE with a try/catch block instead of promise chaining. The review feedback recommends reverting this change to use the two-argument form of .then() with a single ESLint disable comment, as the IIFE introduces unnecessary runtime complexity, extra promise allocations, and potential issues with error propagation if the callback itself throws an error.

gemini-code-assist · 2026-10-06T19:55:32Z

+    void (async () => {
+      let client: gax.ClientStub;
+      try {
+        client = await this.getClientAsync_(config);
+      } catch (err) {
+        callback(err as Error);
+        return;
+      }
+      callback(null, client);
+    })();


While using an async IIFE avoids the promise/no-callback-in-promise ESLint rule without disable comments, it introduces unnecessary complexity, extra promise allocations, and microtask overhead. Furthermore, any synchronous error thrown by the callback itself will be caught by the async IIFE and turned into an unhandled promise rejection, which can make debugging harder.

A cleaner, more standard, and highly performant way to solve the double-callback bug is to use the two-argument form of .then(onFulfilled, onRejected). Since the second argument (onRejected) only handles rejections from the original promise (getClientAsync_) and not errors thrown by the onFulfilled handler, the callback is guaranteed to be called at most once.

Using a single eslint-disable-next-line comment to satisfy the linter is much preferred over adding runtime complexity and changing error propagation behavior.

// eslint-disable-next-line promise/no-callback-in-promise this.getClientAsync_(config).then( client => callback(null, client), err => callback(err) );

References

Do not wrap callback executions in try-catch blocks to handle synchronous errors without an idempotency guard, as this can cause the callback to be called multiple times if the callback itself throws an error.

Without adding another // eslint line of code, is there another way to make the debugging easier.

🤖 I have created a release *beep* *boop* --- <details><summary>google-gax: 6.12.0</summary> ## [6.12.0](google-gax-v6.11.1...google-gax-v6.12.0) (2026-10-07) ### Features * **tools:** Add individual CLI flags for Bun monkey patches ([#9533](#9533)) ([a5eec89](a5eec89)) </details> <details><summary>gaxios: 8.2.0</summary> ## [8.2.0](gaxios-v8.1.0...gaxios-v8.2.0) (2026-10-07) ### Features * **tools:** Add individual CLI flags for Bun monkey patches ([#9533](#9533)) ([a5eec89](a5eec89)) </details> <details><summary>gcp-metadata: 9.1.0</summary> ## [9.1.0](gcp-metadata-v9.0.4...gcp-metadata-v9.1.0) (2026-10-07) ### Features * **tools:** Add individual CLI flags for Bun monkey patches ([#9533](#9533)) ([a5eec89](a5eec89)) </details> <details><summary>google-auth-library: 11.2.0</summary> ## [11.2.0](google-auth-library-v11.1.0...google-auth-library-v11.2.0) (2026-10-07) ### Features * **tools:** Add individual CLI flags for Bun monkey patches ([#9533](#9533)) ([a5eec89](a5eec89)) </details> <details><summary>googleapis-common: 9.2.0</summary> ## [9.2.0](googleapis-common-v9.1.0...googleapis-common-v9.2.0) (2026-10-07) ### Features * **tools:** Add individual CLI flags for Bun monkey patches ([#9533](#9533)) ([a5eec89](a5eec89)) </details> <details><summary>test-utils: 0.2.1</summary> ## [0.2.1](test-utils-v0.2.0...test-utils-v0.2.1) (2026-10-07) ### Bug Fixes * **test-utils:** Make google-test-utils a private workspace package ([#9525](#9525)) ([be25b6f](be25b6f)) </details> <details><summary>bigquery: 9.2.0</summary> ## [9.2.0](bigquery-v9.1.0...bigquery-v9.2.0) (2026-10-07) ### Features * **tools:** Add individual CLI flags for Bun monkey patches ([#9533](#9533)) ([a5eec89](a5eec89)) </details> <details><summary>bigtable: 7.4.0</summary> ## [7.4.0](bigtable-v7.3.0...bigtable-v7.4.0) (2026-10-07) ### Features * **tools:** Add individual CLI flags for Bun monkey patches ([#9533](#9533)) ([a5eec89](a5eec89)) </details> <details><summary>datastore: 11.2.0</summary> ## [11.2.0](datastore-v11.1.0...datastore-v11.2.0) (2026-10-07) ### Features * **tools:** Add individual CLI flags for Bun monkey patches ([#9533](#9533)) ([a5eec89](a5eec89)) </details> <details><summary>error-reporting: 4.2.0</summary> ## [4.2.0](error-reporting-v4.1.0...error-reporting-v4.2.0) (2026-10-07) ### Features * **tools:** Add individual CLI flags for Bun monkey patches ([#9533](#9533)) ([a5eec89](a5eec89)) </details> <details><summary>dns: 6.2.0</summary> ## [6.2.0](dns-v6.1.0...dns-v6.2.0) (2026-10-07) ### Features * **tools:** Add individual CLI flags for Bun monkey patches ([#9533](#9533)) ([a5eec89](a5eec89)) </details> <details><summary>logging: 12.2.0</summary> ## [12.2.0](logging-v12.1.0...logging-v12.2.0) (2026-10-07) ### Features * **tools:** Add individual CLI flags for Bun monkey patches ([#9533](#9533)) ([a5eec89](a5eec89)) </details> <details><summary>logging-bunyan: 6.2.0</summary> ## [6.2.0](logging-bunyan-v6.1.0...logging-bunyan-v6.2.0) (2026-10-07) ### Features * **tools:** Add individual CLI flags for Bun monkey patches ([#9533](#9533)) ([a5eec89](a5eec89)) </details> <details><summary>logging-winston: 7.2.0</summary> ## [7.2.0](logging-winston-v7.1.0...logging-winston-v7.2.0) (2026-10-07) ### Features * **tools:** Add individual CLI flags for Bun monkey patches ([#9533](#9533)) ([a5eec89](a5eec89)) </details> <details><summary>pubsub: 6.2.0</summary> ## [6.2.0](pubsub-v6.1.1...pubsub-v6.2.0) (2026-10-07) ### Features * **tools:** Add individual CLI flags for Bun monkey patches ([#9533](#9533)) ([a5eec89](a5eec89)) ### Bug Fixes * **pubsub:** Avoid invoking callback inside promise handler in getClient_ ([#9544](#9544)) ([752d1d7](752d1d7)) * **pubsub:** Pause underlying pull streams and skip keepalive teardown while paused ([#9521](#9521)) ([f863ecc](f863ecc)) </details> <details><summary>translate: 10.2.0</summary> ## [10.2.0](translate-v10.1.1...translate-v10.2.0) (2026-10-07) ### Features * **tools:** Add individual CLI flags for Bun monkey patches ([#9533](#9533)) ([a5eec89](a5eec89)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Co-authored-by: Shivanee Persaud <[email protected]>

fix(pubsub): avoid invoking callback inside promise handler in getCli…

774a964

…ent_

product-auto-label Bot added the api: pubsub Issues related to the Pub/Sub API. label Oct 6, 2026

gemini-code-assist Bot reviewed Oct 6, 2026

View reviewed changes

fix(pubsub): schedule getClient_ callback via process.nextTick

88cf25b

danieljbruce marked this pull request as ready for review October 7, 2026 13:43

danieljbruce requested a review from a team as a code owner October 7, 2026 13:43

github-actions Bot requested a review from shivanee-p October 7, 2026 13:43

quirogas approved these changes Oct 7, 2026

View reviewed changes

danieljbruce merged commit 752d1d7 into googleapis:main Oct 7, 2026
47 checks passed

release-please Bot mentioned this pull request Oct 7, 2026

chore: release main #9547

Merged

Navigation Menu

Repository navigation

fix(pubsub): avoid invoking callback inside promise handler in getClient_ - #9544

fix(pubsub): avoid invoking callback inside promise handler in getClient_#9544
danieljbruce merged 2 commits into
googleapis:mainfrom
danieljbruce:fix/pubsub-no-callback-in-promise

danieljbruce commented Oct 6, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot Oct 6, 2026

Uh oh!

danieljbruce Oct 6, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

danieljbruce commented Oct 6, 2026

Description

Impact

Changes

Testing

Alternatives

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Oct 6, 2026

Choose a reason for hiding this comment

Uh oh!

danieljbruce Oct 6, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants