[infinispan-dev] Unwrapping exceptions

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

[infinispan-dev] Unwrapping exceptions

Radim Vansa
When enhancing functional API implementation, I've stumbled upon the way
the exceptions from lambdas should be thrown to user code.

Intuitively I've expected that when my lambda throws an exception, I'll
catch it from cache.eval(...). That may be the case when the lambda is
still executed locally, but when it goes remote, the exception is
wrapped into RemoteException etc.

In the past, exceptions being thrown were a matter of failure inside
Infinispan, but what about here? User does not need to know where the
functional expression has been executed, so it seems to me that he
should get the exception directly.

WDYT?

Radim

--
Radim Vansa <[hidden email]>
JBoss Performance Team

_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

Re: [infinispan-dev] Unwrapping exceptions

Dan Berindei
-1, I don't think we need to protect the user from knowing that his
lambda was executed on a remote node. On the contrary, it might help
him/her with debugging.

FWIW, I don't think the cache should ever throw the exact exception
that the lambda raised - I'd always wrap it in some kind of
CacheException.

Cheers
Dan
_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

Re: [infinispan-dev] Unwrapping exceptions

Radim Vansa
The intention was not to protect the user from knowing where the code
was executed, but rather simplify exception handling when he wants to
handle different exceptions from his code (though, throwing exception on
remote node is not too efficient). And the argument was that he does not
*need* to know it.

As for the debugging aid, it could make sense to add the remote stack
trace to suppressed exceptions, though I don't think that it will be of
any use to him.

R.

On 08/29/2016 06:07 PM, Dan Berindei wrote:

> -1, I don't think we need to protect the user from knowing that his
> lambda was executed on a remote node. On the contrary, it might help
> him/her with debugging.
>
> FWIW, I don't think the cache should ever throw the exact exception
> that the lambda raised - I'd always wrap it in some kind of
> CacheException.
>
> Cheers
> Dan
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev


--
Radim Vansa <[hidden email]>
JBoss Performance Team

_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

Re: [infinispan-dev] Unwrapping exceptions

Sanne Grinovero-3
Yes please make sure that the kind of end users exceptions are the
same for the client, regardless if the originator happens to be an
owner as well.

It's valuable to know that the exception happened on another node, but
the exception type (and primary message) should be the same.

Bonus points to not have exceptions at all :)
In Elasticsearch they developed a new scripting language for a use
case similar to our "lambda execution" which basically restricts in
the language itself what is safe to do vs what you can't do.
I'm not sure about developing a new language but from this point of
view it's brilliant..



On 29 August 2016 at 17:54, Radim Vansa <[hidden email]> wrote:

> The intention was not to protect the user from knowing where the code
> was executed, but rather simplify exception handling when he wants to
> handle different exceptions from his code (though, throwing exception on
> remote node is not too efficient). And the argument was that he does not
> *need* to know it.
>
> As for the debugging aid, it could make sense to add the remote stack
> trace to suppressed exceptions, though I don't think that it will be of
> any use to him.
>
> R.
>
> On 08/29/2016 06:07 PM, Dan Berindei wrote:
>> -1, I don't think we need to protect the user from knowing that his
>> lambda was executed on a remote node. On the contrary, it might help
>> him/her with debugging.
>>
>> FWIW, I don't think the cache should ever throw the exact exception
>> that the lambda raised - I'd always wrap it in some kind of
>> CacheException.
>>
>> Cheers
>> Dan
>> _______________________________________________
>> infinispan-dev mailing list
>> [hidden email]
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
>
> --
> Radim Vansa <[hidden email]>
> JBoss Performance Team
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

Re: [infinispan-dev] Unwrapping exceptions

Dan Berindei
On Tue, Aug 30, 2016 at 1:11 PM, Sanne Grinovero <[hidden email]> wrote:
> Yes please make sure that the kind of end users exceptions are the
> same for the client, regardless if the originator happens to be an
> owner as well.
>
> It's valuable to know that the exception happened on another node, but
> the exception type (and primary message) should be the same.
>

Is it really a problem if the local exceptions are wrapped in
CacheException, and the remote ones are wrapped in RemoteException?
RemoteException is a subtype of CacheException, so `catch
(CacheException e)` works with all of them.

Future.get() wraps all exceptions in ExecutionException,
CompletableFuture.join() wraps all exceptions in CompletionException.
So we'd be an outlier if we *didn't* wrap user exceptions.

> Bonus points to not have exceptions at all :)

Error codes FTW? ;)

> In Elasticsearch they developed a new scripting language for a use
> case similar to our "lambda execution" which basically restricts in
> the language itself what is safe to do vs what you can't do.
> I'm not sure about developing a new language but from this point of
> view it's brilliant..
>

I think you impose the same kind of restrictions at the bytecode
level, with something like JaQue [1]. Still, considering that you also
need a way to ship the lambda to the server, a DSL doesn't sound too
bad.

[1]: https://github.com/TrigerSoft/jaque

>
>
> On 29 August 2016 at 17:54, Radim Vansa <[hidden email]> wrote:
>> The intention was not to protect the user from knowing where the code
>> was executed, but rather simplify exception handling when he wants to
>> handle different exceptions from his code (though, throwing exception on
>> remote node is not too efficient). And the argument was that he does not
>> *need* to know it.
>>

But does the user really need to know that it was an exception in
their lambda vs an exception in Infinispan itself? Most of the time,
there's nothing you can do about it anyway...

>> As for the debugging aid, it could make sense to add the remote stack
>> trace to suppressed exceptions, though I don't think that it will be of
>> any use to him.
>>

If we throw the exact exception that the lambda raised on the remote
node, the user is going to see *only* the remote stack trace.

TBH we have the same problem with RemoteExceptions now: instead of
having a stack trace pointing to the user code calling into
Infinispan, our stack trace points to the Infinispan code handling the
response. But at least we have a chance to "fix" the stack trace of
the wrapper exception in AsyncInterceptorChainImpl.invoke to that it
has the caller's stack trace instead. If we don't have a wrapper,
that's no longer possible.

Cheers
Dan
_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

Re: [infinispan-dev] Unwrapping exceptions

Radim Vansa
On 08/30/2016 02:16 PM, Dan Berindei wrote:

> On Tue, Aug 30, 2016 at 1:11 PM, Sanne Grinovero <[hidden email]> wrote:
>> Yes please make sure that the kind of end users exceptions are the
>> same for the client, regardless if the originator happens to be an
>> owner as well.
>>
>> It's valuable to know that the exception happened on another node, but
>> the exception type (and primary message) should be the same.
>>
> Is it really a problem if the local exceptions are wrapped in
> CacheException, and the remote ones are wrapped in RemoteException?
> RemoteException is a subtype of CacheException, so `catch
> (CacheException e)` works with all of them.
>
> Future.get() wraps all exceptions in ExecutionException,
> CompletableFuture.join() wraps all exceptions in CompletionException.
> So we'd be an outlier if we *didn't* wrap user exceptions.

Okay, the exceptions can be wrapped in one (and always exactly one)
level of CacheException. Not as convenient for filtering (try-catch
block vs. instanceofs on e.getCause()), but makes (enough) sense. I'll
adapt the PR.

>
>> Bonus points to not have exceptions at all :)
> Error codes FTW? ;)
>
>> In Elasticsearch they developed a new scripting language for a use
>> case similar to our "lambda execution" which basically restricts in
>> the language itself what is safe to do vs what you can't do.
>> I'm not sure about developing a new language but from this point of
>> view it's brilliant..
>>
> I think you impose the same kind of restrictions at the bytecode
> level, with something like JaQue [1]. Still, considering that you also
> need a way to ship the lambda to the server, a DSL doesn't sound too
> bad.
>
> [1]: https://github.com/TrigerSoft/jaque
>
>>
>> On 29 August 2016 at 17:54, Radim Vansa <[hidden email]> wrote:
>>> The intention was not to protect the user from knowing where the code
>>> was executed, but rather simplify exception handling when he wants to
>>> handle different exceptions from his code (though, throwing exception on
>>> remote node is not too efficient). And the argument was that he does not
>>> *need* to know it.
>>>
> But does the user really need to know that it was an exception in
> their lambda vs an exception in Infinispan itself? Most of the time,
> there's nothing you can do about it anyway...
>
>>> As for the debugging aid, it could make sense to add the remote stack
>>> trace to suppressed exceptions, though I don't think that it will be of
>>> any use to him.
>>>
> If we throw the exact exception that the lambda raised on the remote
> node, the user is going to see *only* the remote stack trace.
>
> TBH we have the same problem with RemoteExceptions now: instead of
> having a stack trace pointing to the user code calling into
> Infinispan, our stack trace points to the Infinispan code handling the
> response. But at least we have a chance to "fix" the stack trace of
> the wrapper exception in AsyncInterceptorChainImpl.invoke to that it
> has the caller's stack trace instead. If we don't have a wrapper,
> that's no longer possible.
>
> Cheers
> Dan
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev


--
Radim Vansa <[hidden email]>
JBoss Performance Team

_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

Re: [infinispan-dev] Unwrapping exceptions

Radim Vansa
On 08/30/2016 02:47 PM, Radim Vansa wrote:

> On 08/30/2016 02:16 PM, Dan Berindei wrote:
>> On Tue, Aug 30, 2016 at 1:11 PM, Sanne Grinovero <[hidden email]> wrote:
>>> Yes please make sure that the kind of end users exceptions are the
>>> same for the client, regardless if the originator happens to be an
>>> owner as well.
>>>
>>> It's valuable to know that the exception happened on another node, but
>>> the exception type (and primary message) should be the same.
>>>
>> Is it really a problem if the local exceptions are wrapped in
>> CacheException, and the remote ones are wrapped in RemoteException?
>> RemoteException is a subtype of CacheException, so `catch
>> (CacheException e)` works with all of them.
>>
>> Future.get() wraps all exceptions in ExecutionException,
>> CompletableFuture.join() wraps all exceptions in CompletionException.
>> So we'd be an outlier if we *didn't* wrap user exceptions.
> Okay, the exceptions can be wrapped in one (and always exactly one)
> level of CacheException. Not as convenient for filtering (try-catch
> block vs. instanceofs on e.getCause()), but makes (enough) sense. I'll
> adapt the PR.

Hmm, it's not possible to use only one remote exception - if the command
fails on backup (before failing on primary), we should keep the hierarchy as
RemoteException (from primary) caused by RemoteException (from backup)
caused by (actual failure).

Radim

>>> Bonus points to not have exceptions at all :)
>> Error codes FTW? ;)
>>
>>> In Elasticsearch they developed a new scripting language for a use
>>> case similar to our "lambda execution" which basically restricts in
>>> the language itself what is safe to do vs what you can't do.
>>> I'm not sure about developing a new language but from this point of
>>> view it's brilliant..
>>>
>> I think you impose the same kind of restrictions at the bytecode
>> level, with something like JaQue [1]. Still, considering that you also
>> need a way to ship the lambda to the server, a DSL doesn't sound too
>> bad.
>>
>> [1]: https://github.com/TrigerSoft/jaque
>>
>>> On 29 August 2016 at 17:54, Radim Vansa <[hidden email]> wrote:
>>>> The intention was not to protect the user from knowing where the code
>>>> was executed, but rather simplify exception handling when he wants to
>>>> handle different exceptions from his code (though, throwing exception on
>>>> remote node is not too efficient). And the argument was that he does not
>>>> *need* to know it.
>>>>
>> But does the user really need to know that it was an exception in
>> their lambda vs an exception in Infinispan itself? Most of the time,
>> there's nothing you can do about it anyway...
>>
>>>> As for the debugging aid, it could make sense to add the remote stack
>>>> trace to suppressed exceptions, though I don't think that it will be of
>>>> any use to him.
>>>>
>> If we throw the exact exception that the lambda raised on the remote
>> node, the user is going to see *only* the remote stack trace.
>>
>> TBH we have the same problem with RemoteExceptions now: instead of
>> having a stack trace pointing to the user code calling into
>> Infinispan, our stack trace points to the Infinispan code handling the
>> response. But at least we have a chance to "fix" the stack trace of
>> the wrapper exception in AsyncInterceptorChainImpl.invoke to that it
>> has the caller's stack trace instead. If we don't have a wrapper,
>> that's no longer possible.
>>
>> Cheers
>> Dan
>> _______________________________________________
>> infinispan-dev mailing list
>> [hidden email]
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>


--
Radim Vansa <[hidden email]>
JBoss Performance Team

_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev