zhijiangW commented on a change in pull request #11486: [FLINK-16712][task]
Refactor StreamTask to construct final fields
URL: https://github.com/apache/flink/pull/11486#discussion_r396890985
##########
File path:
flink-streaming-java/src/main/java/org/apache/flink/streaming/runtime/tasks/StreamTask.java
##########
@@ -260,17 +258,28 @@ protected StreamTask(
@Nullable TimerService timerService,
Thread.UncaughtExceptionHandler
uncaughtExceptionHandler,
StreamTaskActionExecutor actionExecutor,
- TaskMailbox mailbox) {
+ TaskMailbox mailbox) throws Exception {
Review comment:
Good catch. I double checked the codes and it seems not break any existing
behaviors, because we would not explicitly match the `FlinkException` in `Task`
or `JobMaster` ATM.
This wrapped exception for `statelessCtor.newInstance(environment)` is
redundant before this PR, because any instance of `AbstractInvokable` would not
throw exceptions during construction.
I am a bit torn whether it should be wrapped into `FlinkException` now
because it would hide the essential exception to confuse users sometimes. But
this is a common issue also existing in other wrapped exceptions. E.g. some
users were always confused when seeing the message "Could not forward element
to next operator" from wrapped `ExceptionInChainedOperatorException` which
causes task failure, but the real exception might come from specific UDF. And I
also plan to improve this issue a bit by appending the real exception message
with the custom message, but it should be a separate ticket.
Do you have some further concerns for this case?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
