korlov42 commented on code in PR #3264:
URL: https://github.com/apache/ignite-3/pull/3264#discussion_r1524595954
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ExecutionContext.java:
##########
@@ -323,7 +325,18 @@ public void execute(RunnableX task, Consumer<Throwable>
onError) {
task.run();
}
} catch (Throwable e) {
- onError.accept(e);
+ Throwable unwrappedException = ExceptionUtils.unwrapCause(e);
+ onError.accept(unwrappedException);
+
+ if (unwrappedException instanceof IgniteException) {
+ return;
+ }
+
+ if (unwrappedException instanceof AssertionError) {
+ // Just print assertion error, prevent to handle by
failhandler on higher level.
+ LOG.warn("Unexpected exception", e);
+ return;
+ }
throw new IgniteInternalException(INTERNAL_ERR, "Unexpected
exception", e);
Review Comment:
why do we need all this post handling? I mean, we've handled the exception
by propagating it to the root of the query, so why do we need to print anything
to log here and even re-throw it?
##########
modules/failure-handler/src/main/java/org/apache/ignite/internal/failure/FailureProcessor.java:
##########
@@ -145,4 +154,14 @@ private static boolean failureTypeIgnored(FailureContext
failureCtx, FailureHand
return handler instanceof AbstractFailureHandler
&& ((AbstractFailureHandler)
handler).ignoredFailureTypes().contains(failureCtx.type());
}
+
+ /**
+ * Set FailHander interceptor to provide ability tщ test scenarios related
to fail handler.
+ *
+ * @param interceptor Interceptor of fails.
+ */
+ @TestOnly
+ public synchronized void setInterceptor(@Nullable FailureHandler
interceptor) {
Review Comment:
does it make to add TODO here to remove this method when failure handler
becomes configurable?
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ExchangeServiceImpl.java:
##########
@@ -176,6 +174,10 @@ private void onMessage(String nodeName, QueryBatchMessage
msg) {
} catch (Throwable e) {
inbox.onError(e);
+ if (e instanceof IgniteException) {
+ return;
+ }
+
throw new IgniteInternalException(INTERNAL_ERR, "Unexpected
exception", e);
Review Comment:
again, we've just sent the error to the root, so why do we need to rethrow
it here?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
