Skip to content
This repository has been archived by the owner on Jan 19, 2022. It is now read-only.

Suppress exception in Pub/Sub adapter in AUTO_ACK and MANUAL modes #2319

Merged
merged 3 commits into from
Apr 15, 2020

Conversation

elefeint
Copy link
Contributor

The current behavior of the Pub/Sub Spring Integration inbound adapter throwing an exception in AUTO_ACK and MANUAL modes results in client library reacting to the exception by nacking in its code.

This PR removes the exception and logs a warning instead, allowing the message to be redelivered according to Subscription ackDeadline setting and the client library's automatic deadline extension setting.

Fixes #2250.
Follow-up to #2074.

@elefeint elefeint requested review from meltsufin and dzou April 14, 2020 19:31
@codecov
Copy link

codecov bot commented Apr 14, 2020

Codecov Report

Merging #2319 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #2319   +/-   ##
=========================================
  Coverage     73.54%   73.54%           
  Complexity     2078     2078           
=========================================
  Files           258      258           
  Lines          7468     7469    +1     
  Branches        772      772           
=========================================
+ Hits           5492     5493    +1     
  Misses         1620     1620           
  Partials        356      356           
Flag Coverage Δ Complexity Δ
#unittests 73.54% <100.00%> (+<0.01%) 2078.00 <0.00> (ø)
Impacted Files Coverage Δ Complexity Δ
...tegration/inbound/PubSubInboundChannelAdapter.java 65.38% <100.00%> (+0.67%) 9.00 <0.00> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1db1071...65d7636. Read the comment docs.

dzou
dzou previously approved these changes Apr 14, 2020
Copy link
Contributor

@dzou dzou left a comment

Choose a reason for hiding this comment

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

Maybe add docs to clarify the behavior?

Thanks for getting to the bottom of this complicated issue; I know that users have always periodically posted questions asking about this so it is very helpful to clarify expected behavior.

@elefeint
Copy link
Contributor Author

Yes, I am going to whip #2078 into shape next.

@elefeint elefeint requested a review from meltsufin April 14, 2020 21:32
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

Copy link
Contributor

@meltsufin meltsufin left a comment

Choose a reason for hiding this comment

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

Nice! I like the at least and at most check! :)

@elefeint elefeint merged commit ac639ce into master Apr 15, 2020
@elefeint elefeint deleted the pubsub-autoack branch April 15, 2020 16:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

In AUTO_ACK mode message is immediately nack when exception occurs.
3 participants