-
Notifications
You must be signed in to change notification settings - Fork 620
subscription.close() does not flush pending acks. #2558
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
Comments
Thanks for reporting. cc @callmehiphop for help. |
I had originally decided to not add callback/promise support to ack and nack because now we write those requests to a stream and there isn't a reliable way to catch errors. However I think it's probably worth adding support for use cases like this, @stephenplusplus wdyt? I think if you were to set a timeout to call close after the ack, as opposed to after calling close, that would provide you with a temporary solution. return new Promise((resolve, reject) => {
messages.forEach(message => message.ack())
setTimeout(() => subscription.close().then(resolve, reject), 3000)
}) |
Worth noting: acks in Pub/Sub are best effort and Pub/Sub does not reliably tell you if they error. So, error checking will be incomplete at best (which is why we were told to just have |
@callmehiphop That said, subscription closure should just automatically flush pending acks. |
It currently does, but it sounds like we are maybe hitting a race condition
where the process ends before they can flush completely
…On Aug 25, 2017 10:49 AM, "Luke Sneeringer" ***@***.***> wrote:
@callmehiphop <https://github.com/callmehiphop> That said, subscription
closure should just automatically flush pending acks.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2558 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABoNA2WvNHIyrANlvYAe5KwOtRQ_2q33ks5sbwlBgaJpZM4PCdJT>
.
|
Thanks very much for that more succinct code. I'm happy with it being best
effort, the occasional duplicate when the ACK fails is fine, as you know
PubSub can send duplicates so we already handle that.
|
Got it. Probably related to the GCF function ending. |
Uh oh!
There was an error while loading. Please reload this page.
#2380
The new api has made my code simpler so thanks. However I find that now there is no ACK which returns a promise then in a Cloud Function I have to write code like this i.e. the call to 'subscription.close()' does not flush the pending ACKs before resolving:
The text was updated successfully, but these errors were encountered: