[External] : Re: Future.resultNow / exceptionNow

[Paul Bjorkstrand]

To me, it sounds like the ask is this:

"If the Future completed with an exception, why would the exception not
be attached as the cause of the ISE thrown by resultNow()?"

The answer Ron gave was:

"The exception is not attached as the cause, because the API is being used
incorrectly in the first place."

I would argue that not attaching the cause is incorrect as well. I have
seen many times in my career where catch blocks do not attach the cause
when throwing again, which makes it rather difficult to track down the root
problem. I agree that the method may have been used improperly, but I
wouldn't want to hide the root cause.

When you throw an exception *because of another exception* it is far better
to add it as the cause, than to throw one with no cause. If you don't
include the cause, you are hiding the root issue that needs to be solved.
Yes, we may be using the API improperly, but are trying to teach people to
use the API properly, or are you just trying to make it annoying if they
_don't_ use it properly?

I can't count how many times I have seen a catch-block throw a
RuntimeException for a checked one, and didn't include the cause. In my
book, that is a cardinal sin, akin to logging an exception like this in
SLF4J:
log.error("An exception happened: {}", e);
Using either pattern, you lose the benefit of the stack trace and cause
chain in the first place.

For me, this goes beyond reducing boilerplate, and is more about good
programming practices. The more information you can provide to the
developer about what _exactly_ went wrong, the easier it is for that
developer to find the issue.

I agree, 100% with the other cases - task cancelled or thread interrupted
or future not done yet - should throw as-is. All that is being asked for
is, add the cause to the ISE when the future completed exceptionally. You
will save people like me, hours of troubleshooting (and helping teach
others to troubleshoot) errors that don't have useful stacktraces.

-Paul


On Sun, Nov 21, 2021 at 9:57 AM Alex Otenko <oleksandr.otenko at gmail.com>
wrote:

> On Sun, 21 Nov 2021, 14:56 Ron Pressler, <ron.pressler at oracle.com> wrote:
>
> >
> >
> > > On 21 Nov 2021, at 14:00, Alex Otenko <oleksandr.otenko at gmail.com>
> > wrote:
> > >
> > > No, you don't know what went wrong. You only know something did.
> >
> > The methods must only be called when the Future is in a certain state,
> and
> > the exception tells you exactly what the actual, unexpected state is. You
> > have all the information you could ever need to fix the bug. Suppose you
> > call resultNow without having called join first. The error you’ll get
> will
> > tell you exactly that this is the bug, and that adding join is necessary.
> > And suppose you call resultNow after join but before handling exception,
> > the error you’ll get will tell you exactly that that’s what missing.
> >
>
> No, because, to be precise, NPE un the task is the root cause. If not for
> it, the task never throws, and in all the testing we never discovered the
> missing exception handling in the code that calls resultNow.
>
> Now in production NPE got triggered. We look through the logs, and see one
> bug, but not the other. To fix it, you need one patch just to make the NPE
> discoverable. So you will lose time to resolution.
>
>
>
> > >
> > > So, you see the code is broken. You get to deploy two patches: one to
> > start using a different method to observe what has lead to the problem
> > (likely a Runtime Exception or Error - say, NPE in the task), then the
> fix
> > for the actual problem.
> >
> > The exception tells you *which* incorrect state has led to the problem
> > (maybe the message should include precisely which State enum value is
> > expected and which is found).
> >
>
> Not the full state, though, we learn just a single bit of information.
>
>
>
> > >
> > > As for maybeResultNow - I haven't warmed up to the idea that throwing
> by
> > resultNow is saving me from a really bad problem.
> >
> > The problem that resultNow was created to solve is that in the common
> > case, in which you already know the state of the future and have already
> > handled exceptions, anything other than returning the result would be an
> > unnecessary complication. Write down that code and see for yourself.
> >
>
> Sure. Here it is:
>
> exe.join();
> Throwable th = future.exceptionNow();
> if (th != null) throw rewrap(th);
> // here resultNow is guaranteed to be ok
> // but exceptionNow will throw
>
>
>
> > >
> > > I am looking at the code that joined on executor, and don't see why it
> > is more correct for (albeit indirect) state introspection to punish me
> with
> > hidden throws.
> >
> > I don’t understand. The method for state introspection, Future.state,
> > never fails. resultNow, which is *not* for state introspection, never
> > punishes you. It just cannot continue when a bug in your program makes
> its
> > precondition false, and it helps you by telling you what your bug is.
> >
> > > I am looking at another example that polls for result, and again don't
> > see why throwing is better for something that is like a non-blocking get.
> >
> > The purpose of resultNow is not to be a non-blocking get (although you
> > could write a non-blocking get with a combination of state and
> resultNow).
> > The purpose of resultNow is to obtain the result in the common case, in
> > which you already know that it’s there, and are *guaranteed* that the
> > method will never fail.
> >
>
> People will use it as a get() that doesn't require try-catch.
>
>
> > > Reminds me of NIO Selector API, where almost anything can result in a
> > bunch of Runtime Exceptions.
> >
> > If we could panic, maybe we’d panic. The exception is not the point. It
> > just indicates a bug in the program.
> >
>
> Well, and my contention is that you declare it a bug. What's the horrible
> sin of the above check? A throwing exceptionNow doesn't really save me from
> anything.
>
>
> Alex
>
>
> > I understand that you’d like a somewhat different feature, but please
> > start with a motivation: take the current API and show what *problem you
> > run into* that you’d like to solve.
> >
> > If your problem is unrelated to the JEP but is just about a non-blocking
> > polling mechanism for Future, I’d still argue that having both state and
> > resultNow is superior to a single maybeGetResult, because the former is
> > more general: you can use it just as efficiently for polling or for
> > obtaining the result in cases where the correct state is guaranteed.
> >
> > — Ron
>