Skip to content

fix(pubsub): align ClientConfig.port type with gax.GrpcClientOptions - #9539

Merged
danieljbruce merged 2 commits into
googleapis:mainfrom
danieljbruce:fix/pubsub-client-config-port
Oct 6, 2026
Merged

danieljbruce merged 2 commits into
googleapis:mainfrom
danieljbruce:fix/pubsub-client-config-port

Conversation

@danieljbruce

@danieljbruce danieljbruce commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes the @google-cloud/pubsub pack-and-install system test failure (should be able to use the d.ts):

node_modules/@google-cloud/pubsub/build/src/pubsub.d.ts:29:18 - error TS2430: Interface 'ClientConfig' incorrectly extends interface 'GrpcClientOptions'.
  Types of property 'port' are incompatible.
    Type 'string | number | undefined' is not assignable to type 'number | undefined'.
      Type 'string' is not assignable to type 'number'.

Root Cause

In #9496 (released in [email protected]), GrpcClientOptions added port?: number;. In @google-cloud/pubsub, ClientConfig extends gax.GrpcClientOptions but declared port?: string | number;, causing TypeScript compilation with skipLibCheck: false to fail when resolving [email protected]+.

Internally, PubSub#determineBaseUrl_ already parses port as a number (or undefined), matching gax.GrpcClientOptions and gax.ClientOptions.

Changes

  • Updated ClientConfig.port in handwritten/pubsub/src/pubsub.ts from string | number to number.

@danieljbruce
danieljbruce requested a review from a team as a code owner October 6, 2026 17:14
@product-auto-label product-auto-label Bot added the api: pubsub Issues related to the Pub/Sub API. label Oct 6, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the ClientConfig interface in handwritten/pubsub/src/pubsub.ts to restrict the port property to only accept a number instead of string or number. There are no review comments, and I have no feedback to provide.

@danieljbruce
danieljbruce marked this pull request as draft October 6, 2026 17:15
@github-actions
github-actions Bot requested a review from bshaffer October 6, 2026 17:25
@danieljbruce
danieljbruce marked this pull request as ready for review October 6, 2026 18:01
callback,
);
this.getClientAsync_(config)
// eslint-disable-next-line promise/no-callback-in-promise

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: We should probably look more into this callback instead of disabling it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. I put this in our backlog (https://b.corp.google.com/issues/570625459).

@danieljbruce
danieljbruce merged commit 4443d6d into googleapis:main Oct 6, 2026
47 checks passed
@release-please release-please Bot mentioned this pull request Oct 6, 2026
shivanee-p pushed a commit that referenced this pull request Oct 6, 2026
🤖 I have created a release *beep* *boop*
---


<details><summary>google-gax: 6.11.1</summary>

##
[6.11.1](google-gax-v6.11.0...google-gax-v6.11.1)
(2026-10-06)


### Bug Fixes

* **gax:** Only set resend_count on T4 attempt spans
([#9540](#9540))
([3b3f600](3b3f600))
* **gax:** Widen port to number | string in GrpcClientOptions
([#9542](#9542))
([17978e7](17978e7))
</details>

<details><summary>pubsub: 6.1.1</summary>

##
[6.1.1](pubsub-v6.1.0...pubsub-v6.1.1)
(2026-10-06)


### Bug Fixes

* **pubsub:** Align ClientConfig.port type with gax.GrpcClientOptions
([#9539](#9539))
([4443d6d](4443d6d))
* **test-utils:** Make google-test-utils a private workspace package
([#9525](#9525))
([be25b6f](be25b6f))
</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>
shivanee-p added a commit that referenced this pull request Oct 6, 2026
Reverts PR #9539 as to not break customers.

The `port` type definition was expanded in PR#3542 as a non-breaking
change.

---------

Co-authored-by: Santiago Quiroga <[email protected]>
danieljbruce added a commit that referenced this pull request Oct 7, 2026
…ent_ (#9544)

## 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: pubsub Issues related to the Pub/Sub API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants