Alexey,
ср, 14 апр. 2021 г. в 01:52, Alexey Kukushkin <kukushkinale...@gmail.com>:
> Just some points looking questionable to me, although I realize the error
> handling style may be very opinionated:
>
> - Would it make sense splitting the proposed composite error code
> (TBL-0001) into separate numeric code (0001) and scope/category ("TBL")
> to
> avoid parsing the code when an error handler needs to analyze only the
> category or the code?
>
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.
> - "*The cause - short string description of an issue, readable by
> user.*".
> This terminology sounds confusing to me since that "cause" sounds like
> Java
> Throwable's Message to me and the "Cause" is a lower level exception.
>
The string describes the cause of error, so the name. I'm ok to rename it
to a message. It will be stored in IgniteException.message field anyway.
> - "*The action - steps for a user to resolve error...*". The action is
> very important but do we want to make it part of the IgniteException? I
> do
> not think the recovery action text should be part of the exception.
> IgniteException may include a URL pointing to the corresponding
> documentation - this is discussable.
>
This will not be the part of the exception. A user should visit the
documentation page and read the action section by corresponding error code.
> - "*All public methods throw only unchecked IgniteException*" - I assume
> we still respect JCache specification and prefer using standard Java
> exception to signal about invalid parameters.
>
Using standard java exceptions whenever possible makes sense.
> - Why we do not follow the "classic" structured exception handling
> practices in Ignite:
>
Ignite 3 will be multi language, and other languages use other error
processing models. SQL for example uses error codes.
The single exception approach simplifies and unifies error handling across
platforms for me.
> - Why do we not allow using checked exceptions? It seems to me
> sometimes forcing the user to handle an error serves as a hint and
> thus
> improves usability. For example, handling an optimistic/pessimistic
> transaction conflict/deadlock. Or handling a timeout for operations
> with
> timeouts.
>
A valid point. Checked exceptions must be used for whose methods, where
error handling is enforced, for example tx optimistic failure.
Such errors will also have corresponding error codes.
> - Why single public IgniteException and no exception hierarchy? Java
> is optimized for structured exception handling instead of using
> IF-ELSE to
> analyze the codes.
>
Exception hierarchy is not required when using error codes and applicable
only to java API, so I would avoid spending efforts on it.
> - Why no nested exceptions? Sometimes an error handler is interested
> only in high level exceptions (like Invalid Configuration) and
> sometimes
> details are needed (like specific configuration parser exceptions).
>
Nested exceptions are not forbidden to use. They can provide additional
details on the error for debug purposes, but not strictly required, because
error code + message should provide enough information to the user.
> - For async methods returning a Future we may have a universal rule on
> how to handle exceptions. For example, we may specify that any async
> method
> can throw only invalid argument exceptions. All other errors are
> reported
> via the exceptionally(IgniteException -> {}) callback even if the async
> method was executed synchronously.
>
This is ok to me.
>
>
> вт, 13 апр. 2021 г. в 12:08, Alexei Scherbakov <
> alexey.scherbak...@gmail.com
> >:
>
> > Igniters,
> >
> > I would like to start the discussion about error handling in Ignite 3 and
> > how we can improve it compared to Ignite 2.
> >
> > The error handling in Ignite 2 was not very good because of generic
> > CacheException thrown on almost any occasion, having deeply nested root
> > cause and often containing no useful information on further steps to fix
> > the issue.
> >
> > I aim to fix it by introducing some rules on error handling.
> >
> > *Public exception structure.*
> >
> > A public exception must have an error code, a cause, and an action.
> >
> > * The code - the combination of 2 byte scope id and 2 byte error number
> > within the module. This allows up to 2^16 errors for each scope, which
> > should be enough. The error code string representation can look like
> > RFT-0001 or TBL-0001
> > * The cause - short string description of an issue, readable by user.
> This
> > can have dynamic parameters depending on the error type for better user
> > experience, like "Can't write a snapshot, no space left on device {0}"
> > * The action - steps for a user to resolve error situation described in
> the
> > documentation in the corresponding error section, for example "Clean up
> > disk space and retry the operation".
> >
> > Common errors should have their own scope, for example IGN-0001
> >
> > All public methods throw only unchecked
> > org.apache.ignite.lang.IgniteException containing aforementioned fields.
> > Each public method must have a section in the javadoc with a list of all
> > possible error codes for this method.
> >
> > A good example with similar structure can be found here [1]
> >
> > *Async timeouts.*
> >
> > Because almost all API methods in Ignite 3 are async, they all will have
> a
> > configurable default timeout and can complete with timeout error if a
> > computation is not finished in time, for example if a response has not
> been
> > yet received.
> > I suggest to complete the async op future with TimeoutException in this
> > case to make it on par with synchronous execution using future.get, which
> > will throw java.util.concurrent.TimeoutException on timeout.
> > For reference, see java.util.concurrent.CompletableFuture#orTimeout
> > No special error code should be used for this scenario.
> >
> > *Internal exceptions hierarchy.*
> >
> > All internal exceptions should extend
> > org.apache.ignite.internal.lang.IgniteInternalException for checked
> > exceptions and
> > org.apache.ignite.internal.lang.IgniteInternalCheckedException for
> > unchecked exceptions.
> >
> > Thoughts ?
> >
> > [1] https://docs.oracle.com/cd/B10501_01/server.920/a96525/preface.htm
> >
> > --
> >
> > Best regards,
> > Alexei Scherbakov
> >
>
>
> --
> Best regards,
> Alexey
>
--
Best regards,
Alexei Scherbakov
