ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alexey Goncharuk <alexey.goncha...@gmail.com>
Subject Re: Trimming exceptions in IgniteUtils::cast
Date Thu, 05 Apr 2018 16:53:49 GMT
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.kulichenko@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 <
> stanlukyanov@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);
> >      }
> >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message