Skip to content

fix(pubsub): nack messages when handler is dropped#4548

Merged
haphungw merged 2 commits intogoogleapis:mainfrom
haphungw:fix-auto-nack-on-drop
Feb 4, 2026
Merged

fix(pubsub): nack messages when handler is dropped#4548
haphungw merged 2 commits intogoogleapis:mainfrom
haphungw:fix-auto-nack-on-drop

Conversation

@haphungw
Copy link
Copy Markdown
Contributor

@haphungw haphungw commented Feb 4, 2026

Implement Drop for AtLeastOnce to provide a safety net nack.

Previously, if a handler went out of scope without being processed, the message would remain leased. Now, they are automatically rejected if the application fails to explicitly acknowledge or reject them.

Note: The next PR will refactor by removing the explicit nack() in favor of this automatic behavior.

Fix #4514

Previously, if a handler went out of scope without being processed, the message would remain leased. Now, they are automatically rejected if the application fails to explicitly acknowledge or reject them.
Fix googleapis#4514
@haphungw haphungw requested a review from a team February 4, 2026 01:36
@product-auto-label product-auto-label bot added the api: pubsub Issues related to the Pub/Sub API. label Feb 4, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Feb 4, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.02%. Comparing base (058b4f6) to head (6aa803f).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4548   +/-   ##
=======================================
  Coverage   95.01%   95.02%           
=======================================
  Files         195      195           
  Lines        7444     7456   +12     
=======================================
+ Hits         7073     7085   +12     
  Misses        371      371           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Member

@dbolduc dbolduc left a comment

Choose a reason for hiding this comment

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

Nice, can you update the drop comment before merging?

}

impl Drop for AtLeastOnce {
/// Automatically rejects the message if it hasn't been acknowledged yet.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: I think we should copy the documentation from the existing nack() here, especially if we are going to get rid of fn nack() altogether (which I think we should do).

}
}

#[cfg(test)]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good thinking.

@haphungw haphungw enabled auto-merge (squash) February 4, 2026 02:58
@haphungw haphungw merged commit b96b407 into googleapis:main Feb 4, 2026
34 checks passed
@haphungw haphungw deleted the fix-auto-nack-on-drop branch April 4, 2026 00:16
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.

Pub/Sub - Nack messages on drop of handler

2 participants