mjsax commented on code in PR #16300:
URL: https://github.com/apache/kafka/pull/16300#discussion_r1696256477
##########
streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamTask.java:
##########
@@ -916,7 +920,17 @@ public void punctuate(final ProcessorNode<?, ?, ?, ?> node,
} catch (final StreamsException e) {
Review Comment:
Just catching up. For `process()` we currently don't handle
`ClassCastException` as well as `FailedProcessingException |
TaskCorruptedException | TaskMigratedException`.
However, only rethrow `ClassCastException` wrapped as `StreamsException()`
and thus might catch up "upstream" -- wondering if this is intended?
I also just looked into the call-trace and it seems we actually also have
dedicate
```
} catch (final StreamsException exception) {
record = null;
throw exception;
} catch (final RuntimeException e) {
```
inside `StreamTask#process` but we only handle `FailedProcessingException`
in the second catch for the `RuntimeException`. Is this by design? Atm
`FailedProcessingException extends KafkaException` so it still works, but as
pointed out on an already merged PR, I think it should actually extend
`StreamsException`
(https://github.com/apache/kafka/pull/16093/files#r1687232849) what implies we
would need some changes.
In general, `StreamsException` is actually not design to be thrown by users
(not to be catch by users), but it can still bubble up from `process()` -- So I
think it does make sense to catch and pass into the handler for both cases
`process()` and `punctuate()`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
