Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Message.ack() doesn't work as expected #1648

Closed
lushu opened this issue Oct 25, 2022 · 4 comments
Closed

Message.ack() doesn't work as expected #1648

lushu opened this issue Oct 25, 2022 · 4 comments
Assignees
Labels
api: pubsub Issues related to the googleapis/nodejs-pubsub API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@lushu
Copy link

lushu commented Oct 25, 2022

Problem:
When Message.ack was called and immediately followed by clean up subscription, the ack message actually won't be sent out.

Possible Reason:
The reason is: the Message.ack method calls an async method Subscriber.ack without handling the Promise or returning it. This makes Message.ack quit before the ack message being sent out. Does this make sense?

@lushu lushu added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Oct 25, 2022
@product-auto-label product-auto-label bot added the api: pubsub Issues related to the googleapis/nodejs-pubsub API. label Oct 25, 2022
@feywind
Copy link
Collaborator

feywind commented Nov 9, 2022

@lushu Yeah, that makes sense, and I was actually just looking at that code for another question. :) The intention is that those methods are "fire and forget". If exactly once delivery isn't enabled, there won't be a confirmation or failure for the ack.

I hate to call it "as intended", but at the moment, it is. There has been some talk of providing a way for users to explicitly ask for everything to be finished before close() returns, but at the moment, it just drops anything that hasn't sent yet. (Again, unless you are using exactly once delivery, but I wouldn't recommend that just to fix this issue. :)

@feywind feywind closed this as completed Nov 9, 2022
@feywind feywind added type: question Request for information or clarification. Not an issue. and removed type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Nov 9, 2022
@lushu
Copy link
Author

lushu commented Nov 9, 2022

Thank you @feywind for your explanation. Do you know why this happened only recently? From the git log, it looks like that the ack function has been like this for a long time, but only recently, it couldn't work if followed by a cleaning up. Why in a very long time, it did acknowledge the messages?

Cheers,

@feywind
Copy link
Collaborator

feywind commented Nov 10, 2022

We discussed this a bit today, and it sounds like we're intending to cover this case with future updates on close(). I'll reopen this.

@feywind feywind reopened this Nov 10, 2022
@feywind feywind added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. and removed type: question Request for information or clarification. Not an issue. labels Nov 10, 2022
@feywind
Copy link
Collaborator

feywind commented May 2, 2024

Closed in favour of #1917

@feywind feywind closed this as completed May 2, 2024
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 googleapis/nodejs-pubsub API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

2 participants