Skip to content

Shutdown option WaitForProcessing never entered on subscription close #2117

@nico-kn

Description

@nico-kn

Please make sure you have searched for information in the following guides.

A screenshot that you have tested with "Try this API".

Not available, this bug is part of the client shutdown behavior and not part of the pubsub API.

Link to the code that reproduces this issue. A link to a public Github Repository or gist with a minimal reproduction.

https://github.com/googleapis/nodejs-pubsub/blob/main/samples/closeSubscriptionWithTimeout.js#L60-L69

A step-by-step description of how to reproduce the issue, based on the linked reproduction.

  1. Create a new subscription with
closeOptions: {
      behavior: SubscriptionCloseBehaviors.WaitForProcessing,
      timeout: Duration.from({seconds: 10}),
    },
  1. Let the message processing callback of that subscription perform work that takes a couple of seconds to finalize. Does not need to exceed the closeOption timeout.
  2. Publish messages to the topic that this subscription is bound to.
  3. Call await subscription.close() during message processing
  4. You will see that the promise is returned immediately and the WaitForProcessing closeOption was never entered

A clear and concise description of what the bug is, and what you expected to happen.

There seems to be a bug in the implementation of the condition during subscription close that decides to enter the waitForProcessing behavior in

if (
behavior === SubscriberCloseBehaviors.WaitForProcessing &&
!this._inventory.isEmpty
) {
. The access to this._inventory.isEmpty seems to be undefined and would need to be changed to this._inventory.isEmpty() as also suggested in the comment of the PR originally introducing this functionality: #2068 (review)

It seems that with this bug, the waitForProcessing graceful shutdown feature introduced with #2068 is not working as expected and the shutdown behavior is never initiated.

A clear and concise description WHY you expect this behavior, i.e., was it a recent change, there is documentation that points to this behavior, etc. **

The WaitForProcessing closeOption was recently introduced in version 5.2.0 with #2068. In the supplied tests, the isEmpty field check is set on test level, hence letting the tests pass. However, in the source code of

Metadata

Metadata

Assignees

No one assigned

    Labels

    api: pubsubIssues related to the googleapis/nodejs-pubsub API.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions