Stanislav, Thanks for the patch. It is a bit complex change, I will try to review it over the next weekend.
--AG 2018-03-12 21:34 GMT+03:00 Valentin Kulichenko < valentin.kuliche...@gmail.com>: > Generally, I think we should not trim any exceptions, because this way we > can unexpectedly remove useful information. Do we know what was the > original reasoning behind this logic? > > -Val > > On Mon, Mar 12, 2018 at 3:57 AM, Stanislav Lukyanov < > stanlukya...@gmail.com> > wrote: > > > Hi, > > > > IgniteUtils::cast method is used to cast an exception to > > IgniteCheckedException. It is done by trimming the top exceptions in the > > trace until we find IgniteCheckedException, or until we’ve found the last > > one. As a result, IgniteUtils::cast generally can trim the whole stack > > trace but the root exception. > > > > One problem caused by that is https://issues.apache.org/ > > jira/browse/IGNITE-7904. > > ComputeTask::result may throw IgniteException which is to be rethrown by > > the ComputeTaskFuture::get, but in fact the IgniteException and all its > > causes (but the root one) are trimmed, and the exception that comes from > > the ComputeTaskFuture::get is different than excepted. > > > > To fix that I want to change the IgniteUtils::cast not to remove any > > exceptions from the stack trace and just wrap all arguments with > > IgniteCheckException. > > Of course, this will affect all places where IgniteUtils::cast is used > > (mostly various futures completion). > > Does anyone have concerns about this? > > > > To illustrate, a patch (untested!) for IgniteUtils is below. > > > > Thanks, > > Stan > > > > diff --git a/modules/core/src/main/java/org/apache/ignite/internal/ > util/IgniteUtils.java > > b/modules/core/src/main/java/org/apache/ignite/internal/ > > util/IgniteUtils.java > > index cbe64cd97af..5e8d9c8f4d9 100755 > > --- a/modules/core/src/main/java/org/apache/ignite/internal/ > > util/IgniteUtils.java > > +++ b/modules/core/src/main/java/org/apache/ignite/internal/ > > util/IgniteUtils.java > > @@ -7256,26 +7256,16 @@ public abstract class IgniteUtils { > > public static IgniteCheckedException cast(Throwable t) { > > assert t != null; > > > > - while (true) { > > - if (t instanceof Error) > > - throw (Error)t; > > + while (t instanceof GridClosureException) > > + t = ((GridClosureException)t).unwrap(); > > > > - if (t instanceof GridClosureException) { > > - t = ((GridClosureException)t).unwrap(); > > + if (t instanceof Error) > > + throw (Error)t; > > > > - continue; > > - } > > - > > - if (t instanceof IgniteCheckedException) > > - return (IgniteCheckedException)t; > > - > > - if (!(t instanceof IgniteException) || t.getCause() == null) > > - return new IgniteCheckedException(t); > > + if (t instanceof IgniteCheckedException) > > + return (IgniteCheckedException)t; > > > > - assert t.getCause() != null; // ...and it is > IgniteException. > > - > > - t = t.getCause(); > > - } > > + return new IgniteCheckedException(t); > > } > > > > >
