Java thin client errors handling

[Aleksandr Shapkin]

Hello!



I just noticed that the Java thin client throws the following internal
exceptions:

ClientProtocolError

ClientError



Since the classes are not public, there is no way to catch them properly in
user code.

Consider the recent changes, introduced by IGNITE-9410:



throw new ClientProtocolError(String.format("Transactions have not
supported by the server's " +

"protocol version %s, required version %s",
req.clientChannel().serverVersion(), V1_5_0));



The code above correctly verifies the server version against the current
client and throws an exception

In case of an outdated server. The only way to catch it in user code is by
RuntimeException

that feels too broad to use.



I’d like to discuss what’d be the best option to handle this scenario:

-            Should we make the ClientError public?

-            Should we introduce a new public error for every particular
exception?

-            Just ignore this?



Thoughts?

--
Alex.

[Igor Sapego-2]

Hi,

I believe, we definitely should not ignore this.

Alexey, you are the author of this code. What do you think?

Best Regards,
Igor


On Thu, Feb 27, 2020 at 3:56 PM Aleksandr Shapkin <[hidden email]> wrote:

> Hello!
>
>
>
> I just noticed that the Java thin client throws the following internal
> exceptions:
>
> ClientProtocolError
>
> ClientError
>
>
>
> Since the classes are not public, there is no way to catch them properly in
> user code.
>
> Consider the recent changes, introduced by IGNITE-9410:
>
>
>
> throw new ClientProtocolError(String.format("Transactions have not
> supported by the server's " +
>
> "protocol version %s, required version %s",
> req.clientChannel().serverVersion(), V1_5_0));
>
>
>
> The code above correctly verifies the server version against the current
> client and throws an exception
>
> In case of an outdated server. The only way to catch it in user code is by
> RuntimeException
>
> that feels too broad to use.
>
>
>
> I’d like to discuss what’d be the best option to handle this scenario:
>
> -            Should we make the ClientError public?
>
> -            Should we introduce a new public error for every particular
> exception?
>
> -            Just ignore this?
>
>
>
> Thoughts?
>
> --
> Alex.
>

[Alexey Kukushkin]

Hi Guys,

Most likely I just forgot to remove the "internal" ClientError and
ClientProtocolError when I was implementing a review comment to merge the
Java thin client into ignite-core (originally I developed the Java thin
client as a separate module). I see that the ClientProtocolError is not
used in the code at all and the ClientError is used to report some SSL
initialisation failures.

We cannot throw "internal" exceptions from public API. I would open a
"newbie" ticket to get rid of the "internal" ClientError and
ClientProtocolError.

The public ClientException and its subclasses are used to report the Java
thin client errors now.

As I understand we are reviewing some new feature that utilises the
internal "ClientProtocolError". I would create a new public
org.apache.ignite.client.ClientProtocolException extending ClientException
instead.



On Tue, Mar 3, 2020 at 2:50 PM Igor Sapego <[hidden email]> wrote:

> Hi,
>
> I believe, we definitely should not ignore this.
>
> Alexey, you are the author of this code. What do you think?
>
> Best Regards,
> Igor
>
>
> On Thu, Feb 27, 2020 at 3:56 PM Aleksandr Shapkin <[hidden email]>
> wrote:
>
> > Hello!
> >
> >
> >
> > I just noticed that the Java thin client throws the following internal
> > exceptions:
> >
> > ClientProtocolError
> >
> > ClientError
> >
> >
> >
> > Since the classes are not public, there is no way to catch them properly
> in
> > user code.
> >
> > Consider the recent changes, introduced by IGNITE-9410:
> >
> >
> >
> > throw new ClientProtocolError(String.format("Transactions have not
> > supported by the server's " +
> >
> > "protocol version %s, required version %s",
> > req.clientChannel().serverVersion(), V1_5_0));
> >
> >
> >
> > The code above correctly verifies the server version against the current
> > client and throws an exception
> >
> > In case of an outdated server. The only way to catch it in user code is
> by
> > RuntimeException
> >
> > that feels too broad to use.
> >
> >
> >
> > I’d like to discuss what’d be the best option to handle this scenario:
> >
> > -            Should we make the ClientError public?
> >
> > -            Should we introduce a new public error for every particular
> > exception?
> >
> > -            Just ignore this?
> >
> >
> >
> > Thoughts?
> >
> > --
> > Alex.
> >
>

--
Best regards,
Alexey