feat: add timeout option and graceful shutdown to Subscription.close() by feywind · Pull Request #2068 · googleapis/nodejs-pubsub

feywind · 2025-07-16T17:48:26Z

Implements a new timeout and behavior option (using Duration) for the Subscription.close() method. These options are on the SubscriberOptions/SubscriptionOptions passed in when opening a subscriber.

This provides more control over the shutdown process:

The subscriber stream is closed immediately.
If the behaviour is set to NackImmediately, then all buffered messages are nacked, and the timeout specifies how long to wait for them to go through.
If the behaviour is set to WaitForProcessing, then the remaining message machinery continues until the timeout is almost up, and then it switches to NackImmediately.
If the timeout is zero, everything closes down immediately, as quickly as possible.

(This is a re-open of #2037 to make it from the main repo.)

Implements a new `timeout` option (using `Duration`) for the `Subscription.close()` method. This provides more control over the shutdown process: - If `timeout` is zero, the subscription closes as quickly as possible without nacking buffered messages. - If `timeout` is positive, the subscription attempts to nack any buffered messages (in the lease manager) and waits up to the specified duration for pending acknowledgements and nacks to be sent to the server. - If no timeout is provided, the behavior remains as before (waits indefinitely for pending acks/modacks, no nacking). The core logic is implemented in `Subscriber.close()`. `PubSub.close()` documentation is updated to clarify its scope and recommend using `Subscription.close()` directly for this feature. Includes: - Unit tests for the new timeout behavior in `Subscriber.close()`. - A TypeScript sample (`samples/closeSubscriptionWithTimeout.ts`) demonstrating usage. - Updated JSDoc documentation for relevant methods.

…ions

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

…dejs-pubsub into feat-close-timeout

src/exponential-retry.ts

sofisl · 2025-07-16T21:48:58Z

src/subscriber.ts

-    const remaining = this._inventory.clear();

-    await this._waitForFlush();
+    const options = this._options.closeOptions;


optional chaining?

this._options is guaranteed to be non-undefined, so I don't think it's necessary here. The lines that use options do optional chaining, since closeOptions may be undefined.

sofisl · 2025-07-16T21:50:03Z

src/subscriber.ts

+    // The timeout can't realistically be longer than the longest time we're willing
+    // to lease messages.
+    let timeout = durationAtMost(
+      options?.timeout ?? this.maxExtensionTime,


is maxExtensionTime a const? How do we know it will be available? If the former, maybe worth uppercasing?

It's a config option with a default. So it's set by the constructor:

this.maxExtensionTime = defaultOptions.subscription.maxExtensionTime;

And then if the user passed a value, it overwrites the default.

src/subscriber.ts

src/temporal.ts

miguelvelezsa

I would highly recommend to add types in all the new code :)

src/lease-manager.ts

src/subscriber.ts

miguelvelezsa · 2025-07-18T16:22:55Z

src/subscriber.ts

+   *
+   * @private
+   */
+  dispatched(): void {


should be dispatch()?

It's less of a command, more of an event. Basically there is something else doing dispatching, and this method is called to notify telemetry and such. So I don't think I'd call it dispatch, but I'm open to other ideas.

src/subscriber.ts

miguelvelezsa · 2025-07-18T16:58:20Z

src/subscriber.ts

+      const err = e as [unknown, boolean];
+      if (err[1] === false) {


for my own knowledge, how err[1] === true means was timeout? probably adding type for 'e' here will help :)

Yeah, this has to do with implementing the time-limited Promise wait... it's super annoying to make graceful. I'll look again.

Not sure why it didn't give me a separate box for comments on the one above, but check out Sofia's comment on why I did this like I did.

I agree with you on the second one. I'll add an interface.

src/subscriber.ts

feywind · 2025-07-21T21:11:16Z

I would highly recommend to add types in all the new code :)

Can you elaborate here? The only time I tend to omit them is when they're super obvious (like making a class member like foo = new Map<string, string>()). I might've missed some : void on function returns.

Edit: it didn't tag you :) @miguelvelezsa

feywind · 2025-07-22T19:43:36Z

I need to look at a few more review comments before merging anything.

…imeout' into feat-close-timeout

…lose-timeout

generated-files-bot · 2025-08-06T21:02:40Z

Warning: This pull request is touching the following templated files:

README.md - README.md is managed by synthtool. However, a partials file can be used to update the README, e.g.: https://github.com/googleapis/nodejs-storage/blob/main/.readme-partials.yaml
samples/README.md - samples/README.md is managed by synthtool. However, a partials file can be used to update the README, e.g.: https://github.com/googleapis/nodejs-storage/blob/main/.readme-partials.yaml

nico-kn · 2025-12-05T15:30:29Z

src/subscriber.ts

+    if (
+      behavior === SubscriberCloseBehaviors.WaitForProcessing &&
+      !this._inventory.isEmpty
+    ) {


Was just debugging the WaitForProcessing functionality locally and stumbled upon this check.

@feywind wouldn't this need to be the following? isEmpty seems to be undefined, instead this might have to be a method call?

Suggested change

if (

behavior === SubscriberCloseBehaviors.WaitForProcessing &&

!this._inventory.isEmpty

) {

if (

behavior === SubscriberCloseBehaviors.WaitForProcessing &&

!this._inventory.isEmpty()

) {

Created #2117 accordingly

google-labs-jules bot and others added 16 commits May 6, 2025 22:15

docs: revert README change so release-please can do it

687212d

feat: jules' vibin' is too lo-fi, fix some bad assumptions

da726d0

samples: typeless a JS sample for close with timeout

629aa43

feat: add awaitWithTimeout and test

a0ba234

tests: also test error results without timeout

13e58f2

feat: update for the current spec, test updates coming

5db179e

tests: misc fixes before further additions

62afe56

feat: update Temporal shims to match latest standards

b904884

chore: linter fix

2cf219f

feat: update to latest spec doc info, finish unit tests

6e41e4d

Merge branch 'main' into feat-close-timeout

49003e9

feat: also move the options from close() parameters to subscriber opt…

ca50c3c

…ions

chore: fix linter errors

ce5e917

samples: update to latest API changes

f2898a3

🦉 Updates from OwlBot post-processor

073c6d7

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

feywind requested a review from a team July 16, 2025 17:48

feywind requested a review from a team as a code owner July 16, 2025 17:48

product-auto-label bot added size: xl Pull request size is extra large. api: pubsub Issues related to the googleapis/nodejs-pubsub API. labels Jul 16, 2025

feywind mentioned this pull request Jul 16, 2025

feat: add timeout option and graceful shutdown to Subscription.close() #2037

Closed

feywind added the owlbot:run Add this label to trigger the Owlbot post processor. label Jul 16, 2025

gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Jul 16, 2025

gcf-owl-bot bot added 3 commits July 16, 2025 17:53

🦉 Updates from OwlBot post-processor

bcad5e7

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

🦉 Updates from OwlBot post-processor

f28be03

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

Merge branch 'feat-close-timeout' of https://github.com/googleapis/no…

9aabe08

…dejs-pubsub into feat-close-timeout

sofisl previously approved these changes Jul 16, 2025

View reviewed changes

miguelvelezsa reviewed Jul 18, 2025

View reviewed changes

feywind added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jul 22, 2025

feywind mentioned this pull request Aug 1, 2025

feat: Attempted implemented graceful shutdown #1855

Closed

4 tasks

feywind added 7 commits August 6, 2025 16:01

fix: no need to check isEmpty on remove

8866c98

chore: merge remote-tracking branch 'refs/remotes/origin/feat-close-t…

82e0c81

…imeout' into feat-close-timeout

chore: merge remote-tracking branch 'remotes/origin/main' into feat-c…

f08ae6d

…lose-timeout

chore: remove unneeded promise skip code

791a7f0

fix: substantially clarify the awaitWithTimeout interface

36e5361

chore: hoist timeout logic into its own method

d3a3489

fix: tests were leaking EventEmitter handlers

a285935

feywind dismissed sofisl’s stale review via a285935 August 6, 2025 21:02

chore: change constant to CONSTANT_CASE

abeeb6c

feywind added owlbot:run Add this label to trigger the Owlbot post processor. and removed do not merge Indicates a pull request not ready for merge, due to either quality or timing. labels Aug 6, 2025

gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Aug 6, 2025

sofisl approved these changes Aug 7, 2025

View reviewed changes

feywind merged commit 8dee024 into main Aug 7, 2025
20 of 21 checks passed

feywind deleted the feat-close-timeout branch August 7, 2025 17:57

release-please bot mentioned this pull request Jul 22, 2025

chore(main): release 5.2.0 #2073

Merged

nico-kn reviewed Dec 5, 2025

View reviewed changes

Conversation

feywind commented Jul 16, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

miguelvelezsa left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

feywind Aug 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

feywind commented Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

feywind commented Jul 22, 2025

Uh oh!

generated-files-bot bot commented Aug 6, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

feywind Aug 6, 2025 •

edited

Loading

feywind commented Jul 21, 2025 •

edited

Loading