Expose user exception thrown during rebalance #2667
Conversation
As a user, I do not want exception my exception be effectively ignored by framework by logging them on debug level
| @@ -52,7 +52,7 @@ default void onPartitionsRevokedBeforeCommit(Consumer<?, ?> consumer, Collection | |||
| onPartitionsRevoked(partitions); | |||
| } | |||
| catch (Exception e) { // NOSONAR | |||
| LOGGER.debug(e, "User method threw exception"); | |||
| LOGGER.error(e, "User method threw exception"); | |||
There was a problem hiding this comment.
I don't think that this is what we want to have as a default behavior.
If you are not OK with debug, you always can override this method in your impl and error() it instead.
There was a problem hiding this comment.
A callback interface that the user can implement to trigger custom actions when the set of partitions assigned to the consumer changes.
(...)
There are many uses for this functionality. One common use is saving offsets in a custom store. By saving offsets in the onPartitionsRevoked(Collection) call we can ensure that any time partition assignment changes the offset gets saved.
Another use is flushing out any kind of cache of intermediate results the consumer may be keeping.
I can't imagine any scenario when I implement callback and is OK that framework silently ignores the exception thrown by callback.
I always prefer rely on framework default when unexpected error appears rather than reinvent the wheel. That's idea of framework: focus on business and leave all rest default.
Swallow user exception is not good default in any framework
There was a problem hiding this comment.
OK. Let's hear then back from @garyrussell who did that logic in the ConsumerAwareRebalanceListener!
There was a problem hiding this comment.
I have no trouble with this; user listeners should never throw exceptions; this code was added "just in case".
There was a problem hiding this comment.
Just to clarify; this was also to protect against a strange use case where the user implements ConsumerAwareRebalanceListener but actually overrides one or more of the ConsumerRebalanceListener methods (without a Consumer).
As a user, I do not want my exception be effectively ignored by framework by logging them on debug level