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

Error handler throws an exception on blocking retries #3409

Closed
lm231290 opened this issue Aug 7, 2024 · 6 comments · Fixed by #3421
Closed

Error handler throws an exception on blocking retries #3409

lm231290 opened this issue Aug 7, 2024 · 6 comments · Fixed by #3421

Comments

@lm231290
Copy link

lm231290 commented Aug 7, 2024

version 3.1.2 - 3.2.2

When using blocking retries an back-off (e.g new FixedBackOff(5000, 2)), there is an error message in the log:

...KafkaMessageListenerContainer    : Error handler threw an exception
org.springframework.kafka.KafkaException: Seek to current after exception
...

I see two confusing things here:

  1. the log states that it is an exception of Error Handler, and occurred while seeking. Even though it has a real exception in Caused by, the beginning is wrong in my opinion because the Error Handler didn't fail handling my errors
  2. the log is printed after the back-off interval (5s in my example), right before the next attempt

To reproduce:

  • Create DefaultErrorHandler as a Spring bean
  • Set a back-off with a notable interval
  • Set this error handler in KafkaListenerContainerFactory used for the listener

Expected behavior

Either no error message at all, or log it right after the attempt fails, before the back-off pause, and with less confusing message

Sample

https://github.com/lm231290/kafka-tests

There is a retry.log file there with my execution example
Or you can run ListenerTest.test_single and see the log messages and the timestamps

@artembilan
Copy link
Member

I didn’t look into code, but if you find it confusing then something is really wrong. We will be happy to review any contribution. Otherwise we will look into this when we can.
Thanks.

@sobychacko
Copy link
Contributor

I briefly looked at the issue by running your sample app. It looks like it is doing the right thing, but I agree that something is amiss, as the messages are confusing. The exception is thrown specifically from here: https://github.com/spring-projects/spring-kafka/blob/main/spring-kafka/src/main/java/org/springframework/kafka/listener/KafkaMessageListenerContainer.java#L2719
The exception from the listener is wrapped inside a KafkaException, and the default level there is ERROR. We will take a closer look.

@sobychacko
Copy link
Contributor

The normal expectation is that when you have enabled retry, you see the exceptions only after all the retries are exhausted and the record fails. So, we will have to debug more and see whats going on.

@lm231290
Copy link
Author

lm231290 commented Aug 8, 2024

@sobychacko Yeah, that is a good point too, to have the error logged only after the attempts are exhausted. Before that I'd prefer to see warnings or infos

@sobychacko
Copy link
Contributor

As a workaround, you can set the log level on the error handler to TRACE. That should remove the superfluous logging.

defaultErrorHandler.setLogLevel(KafkaException.Level.TRACE);

However, we will introduce a new custom KafkaException to mitigate this logging. The error handler must throw this exception when seeking the record to avoid any commits, but we agree that the user does not need to see them on the logs.

@lm231290
Copy link
Author

lm231290 commented Aug 8, 2024

@sobychacko, good idea, thank you.

I also noticed that for the listeners where I use retry topics along with blocking retries, there is no such a problem. Maybe it will give you some ideas how to fix it for the regular listeners

I configure blocking retries for retry topics like this:

public class KafkaRetryConfiguration extends RetryTopicConfigurationSupport {
...
@Override
    protected void configureBlockingRetries(RetryTopicConfigurationSupport.BlockingRetriesConfigurer blockingRetries) {
        blockingRetries
                .retryOn(RetryableException.class)
                .backOff(new FixedBackOff(delay, attempts));
    }
}

@sobychacko sobychacko added this to the 3.3.0-M2 milestone Aug 14, 2024
sobychacko added a commit to sobychacko/spring-kafka that referenced this issue Aug 14, 2024
Fixes: spring-projects#3409

- During error handling, records in retry are throwing `KafkaException` after seeking
  that causes the exception to get logged which maybe misleading. It seems to indicate
  that the error handling process itself thew the exception, while the exception was thrown
  in order to prevent accidental committing of a not-yet recovered record.
  Change this exception during seeks in retry while handling error from `KafkaException`
  to a new framework-only used exception - `RecordInRetryException` that simply logs the
  message at INFO level.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants