Re: [DISCUSSION] Error handling in Ignite 3
Hi everyone,
I'd like to continue the discussion. I created IEP with the attempt to
summarize
all the information above, you can find it here [1]. What do you think?
[1]
https://cwiki.apache.org/confluence/display/IGNITE/IEP-84%3A+Error+handling
ср, 21 апр. 2021 г. в 20:46, Alexei Scherbakov :
> Alexei,
>
> I think it's ok to do error conversion if it makes sense, but better to
> preserve the root cause whenever possible.
> Another way to solve the described scenario is to introduce something like
> checked IgniteRetryAgainException, which forces the user to retry or ignore
> it.
> It's difficult to foresee exactly at this point what is the best solution
> without knowing the exact scenario.
>
> Andrey,
>
> I've just realized your proposal to add IGN to each string code. This is ok
> to me, for example IGN-TBL-0001
>
> ср, 21 апр. 2021 г. в 17:49, Alexey Goncharuk >:
>
> > Aleksei,
> >
> > > The method should always report root cause, in your example it will be
> > > B-, no matter which module API is called
> >
> > I may be wrong, but I doubt this will be usable for an end-user. Let's
> > imagine that the same root exception was raised in different contexts
> > resulting in two outcomes. The first one is safe to retry (say, the root
> > cause led to a transaction prepare failure), but the second outcome may
> be
> > a state in which no matter how many retries will be attempted, the
> > operation will never succeed. Only the upper-level layer can tell the
> > difference and return a proper message to the user, so I would say that
> > some error conversion/wrapping will be required no matter what.
> >
> > --AG
> >
> > пт, 16 апр. 2021 г. в 16:31, Alexei Scherbakov <
> > alexey.scherbak...@gmail.com
> > >:
> >
> > > чт, 15 апр. 2021 г. в 18:21, Andrey Mashenkov <
> > andrey.mashen...@gmail.com
> > > >:
> > >
> > > > Hi Alexey,
> > > > I like the idea.
> > > >
> > > > 1.
> > > >
> > > > > TBL-0001 is a *string representation* of the error. It is built
> > from
> > > 2
> > > > > byte scope id (mapped to name TBL) and 2 byte number (0001). Both
> > > > > internally packed in int. No any kind of parsing will be necessary
> to
> > > > read
> > > > > scope/category.
> > > >
> > > > I think Alexey mean if it will be possible to make smth like that
> > > >
> > > > catch (IgniteException e) {
> > > > if (e.getScope() == "TBL" && e.getCode() == 1234)
> > > > continue; // E.g. retry my TX
> > > > }
> > > >
> > > > It looks useful to me.
> > > >
> > >
> > > I have in mind something like this:
> > >
> > > public class IgniteException extends RuntimeException {
> > > private int errorCode;
> > >
> > > public IgniteException(ErrorScope scope, int code, String message,
> > > Throwable cause) {
> > > super(message, cause);
> > > this.errorCode = make(scope, code);
> > > }
> > >
> > > public boolean matches(ErrorScope scope, int code) {
> > > return errorCode == make(scope, code);
> > > }
> > >
> > > private int make(ErrorScope scope, int code) {
> > > return ((scope.ordinal() << 16) | code);
> > > }
> > >
> > > public ErrorScope scope() {
> > > return ErrorScope.values()[errorCode >> 16];
> > > }
> > >
> > > public int code() {
> > > return 0x & errorCode;
> > > }
> > >
> > > public static void main(String[] args) {
> > > IgniteException e = new IgniteException(ErrorScope.RAFT, 1,
> > "test",
> > > null);
> > >
> > > System.out.println(e.matches(ErrorScope.RAFT, 2));
> > > System.out.println(e.scope());
> > > System.out.println(e.code());
> > >
> > > try {
> > > throw e;
> > > }
> > > catch (IgniteException ee) {
> > > System.out.println(ee.matches(ErrorScope.RAFT, 1));
> > > }
> > > }
> > > }
> > >
> > >
> > > >
> > > > 2. How you see a cross-module exception throwing?
> > > > Assume, user call -> module A, which recursively call -> module B,
> > which
> > > > fails.
> > > > So, module A component calls a module B component and got an
> Exception
> > > with
> > > > "B-1234" exception.
> > > > Module A do not expect any exception here and doesn't take care of
> > > "B-xxx"
> > > > error codes, but only "A-.
> > > > Should it rethrow exception with "A-unknown" (maybe "UNK-0001") code
> > > > or reuse "B-" code with the own message, pointing original
> > exception
> > > as
> > > > a cause for both cases?
> > > >
> > > > The first approach may looks confusing, while the second one produces
> > too
> > > > many "UNK-" codes.
> > > > What code should get the user in that case?
> > > >
> > > >
> > > >
> > > The method should always report root cause, in your example it will be
> > > B-, no matter which module API is called.
> > >
> > >
> > > >
> > > >
> > > >
> > > > On Thu, Apr 15, 2021 at 5:36 PM Alexei Scherbakov <
> > > > alexey.scherbak...@gmail.com> wrote:
> > > >
> > > > > чт, 15 апр. 2021 г. в 14:26,
Re[2]: [DISCUSSION] Error handling in Ignite 3
Ivan, thanks for this effort, as for me - looks good.
>Hi everyone,
>
>I'd like to continue the discussion. I created IEP with the attempt to
>summarize
>all the information above, you can find it here [1]. What do you think?
>
>[1]
>https://cwiki.apache.org/confluence/display/IGNITE/IEP-84%3A+Error+handling
>
>ср, 21 апр. 2021 г. в 20:46, Alexei Scherbakov < alexey.scherbak...@gmail.com
>>:
>
>> Alexei,
>>
>> I think it's ok to do error conversion if it makes sense, but better to
>> preserve the root cause whenever possible.
>> Another way to solve the described scenario is to introduce something like
>> checked IgniteRetryAgainException, which forces the user to retry or ignore
>> it.
>> It's difficult to foresee exactly at this point what is the best solution
>> without knowing the exact scenario.
>>
>> Andrey,
>>
>> I've just realized your proposal to add IGN to each string code. This is ok
>> to me, for example IGN-TBL-0001
>>
>> ср, 21 апр. 2021 г. в 17:49, Alexey Goncharuk < alexey.goncha...@gmail.com
>> >:
>>
>> > Aleksei,
>> >
>> > > The method should always report root cause, in your example it will be
>> > > B-, no matter which module API is called
>> >
>> > I may be wrong, but I doubt this will be usable for an end-user. Let's
>> > imagine that the same root exception was raised in different contexts
>> > resulting in two outcomes. The first one is safe to retry (say, the root
>> > cause led to a transaction prepare failure), but the second outcome may
>> be
>> > a state in which no matter how many retries will be attempted, the
>> > operation will never succeed. Only the upper-level layer can tell the
>> > difference and return a proper message to the user, so I would say that
>> > some error conversion/wrapping will be required no matter what.
>> >
>> > --AG
>> >
>> > пт, 16 апр. 2021 г. в 16:31, Alexei Scherbakov <
>> > alexey.scherbak...@gmail.com
>> > >:
>> >
>> > > чт, 15 апр. 2021 г. в 18:21, Andrey Mashenkov <
>> > andrey.mashen...@gmail.com
>> > > >:
>> > >
>> > > > Hi Alexey,
>> > > > I like the idea.
>> > > >
>> > > > 1.
>> > > >
>> > > > > TBL-0001 is a *string representation* of the error. It is built
>> > from
>> > > 2
>> > > > > byte scope id (mapped to name TBL) and 2 byte number (0001). Both
>> > > > > internally packed in int. No any kind of parsing will be necessary
>> to
>> > > > read
>> > > > > scope/category.
>> > > >
>> > > > I think Alexey mean if it will be possible to make smth like that
>> > > >
>> > > > catch (IgniteException e) {
>> > > > if (e.getScope() == "TBL" && e.getCode() == 1234)
>> > > > continue; // E.g. retry my TX
>> > > > }
>> > > >
>> > > > It looks useful to me.
>> > > >
>> > >
>> > > I have in mind something like this:
>> > >
>> > > public class IgniteException extends RuntimeException {
>> > > private int errorCode;
>> > >
>> > > public IgniteException(ErrorScope scope, int code, String message,
>> > > Throwable cause) {
>> > > super(message, cause);
>> > > this.errorCode = make(scope, code);
>> > > }
>> > >
>> > > public boolean matches(ErrorScope scope, int code) {
>> > > return errorCode == make(scope, code);
>> > > }
>> > >
>> > > private int make(ErrorScope scope, int code) {
>> > > return ((scope.ordinal() << 16) | code);
>> > > }
>> > >
>> > > public ErrorScope scope() {
>> > > return ErrorScope.values()[errorCode >> 16];
>> > > }
>> > >
>> > > public int code() {
>> > > return 0x & errorCode;
>> > > }
>> > >
>> > > public static void main(String[] args) {
>> > > IgniteException e = new IgniteException(ErrorScope.RAFT, 1,
>> > "test",
>> > > null);
>> > >
>> > > System.out.println(e.matches(ErrorScope.RAFT, 2));
>> > > System.out.println(e.scope());
>> > > System.out.println(e.code());
>> > >
>> > > try {
>> > > throw e;
>> > > }
>> > > catch (IgniteException ee) {
>> > > System.out.println(ee.matches(ErrorScope.RAFT, 1));
>> > > }
>> > > }
>> > > }
>> > >
>> > >
>> > > >
>> > > > 2. How you see a cross-module exception throwing?
>> > > > Assume, user call -> module A, which recursively call -> module B,
>> > which
>> > > > fails.
>> > > > So, module A component calls a module B component and got an
>> Exception
>> > > with
>> > > > "B-1234" exception.
>> > > > Module A do not expect any exception here and doesn't take care of
>> > > "B-xxx"
>> > > > error codes, but only "A-.
>> > > > Should it rethrow exception with "A-unknown" (maybe "UNK-0001") code
>> > > > or reuse "B-" code with the own message, pointing original
>> > exception
>> > > as
>> > > > a cause for both cases?
>> > > >
>> > > > The first approach may looks confusing, while the second one produces
>> > too
>> > > > many "UNK-" codes.
>> > > > What code should get the user in that case?
>> > > >
>> > > >
>> > > >
>> > > The method should always report root cause, in your example it will be
>> > > B-, no matter which module API is called.
>> > >
>> > >
>> > > >
>> > > >
>> > > >
>> > > > On Thu, Apr 15, 2021 at 5:36 PM Alexei Scherbakov <
>> > > > ale
