You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by Mohammad arshad <mo...@huawei.com> on 2016/10/03 07:54:23 UTC

ZooKeeper clients does not handle new error codes properly

Hi All,
In Zookeeper rolling upgrade scenario where server is new but client is old, when sever sends error code which is not understood by a client,  client throws IllegalArgumentException. Generally IllegalArgumentException is not handled by any of the ZK applications. It is too generic. How to handle this scenario in ZK applications?
My understanding is instead of throwing IllegalArgumentException we should throw a subclass of KeeperException, for example InvalidErrorCodeException, so that zk apps can take more specific action.
Any thoughts?

Thanks
-Arshad


Re: ZooKeeper clients does not handle new error codes properly

Posted by Flavio Junqueira <fp...@apache.org>.
Yeah, unfortunately, I don't think we make use of the protocol version in ConnectRequest and Response. When we instantiate ConnectRequest in ClientCnxn, we simply set it to zero. 

-Flavio

> On 04 Oct 2016, at 16:21, Mohammad arshad <mo...@huawei.com> wrote:
> 
> Thanks Benjamin Reed,
> Client protocol version is 0, it is not changed. Also there is no logic executed at the server side based on the client protocol version. Increasing the client protocol version and converting the errors at server may be the ideal and more precise solution but to handle it in a generic way can we convert the unknown error to KeeperException.SystemErrorException at client side? Do you foresee any problem in this solution? 
> 
> Thanks
> -Arshad
> 
> 
> -----Original Message-----
> From: Benjamin Reed [mailto:breed@apache.org] 
> Sent: 04 October 2016 10:27
> To: user@zookeeper.apache.org
> Cc: DevZooKeeper
> Subject: Re: ZooKeeper clients does not handle new error codes properly
> 
> did we bump the protocol version when we added the new errors? the server could do the conversion when it responds to older clients.
> 
> On Mon, Oct 3, 2016 at 3:05 AM, Flavio Junqueira <fp...@apache.org> wrote:
> 
>> Hi Arshad,
>> 
>> It makes sense to me. What if we convert unknown server errors to 
>> KeeperException.SystemErrorException? This is a generic error and it 
>> extends KeeperException.
>> 
>> I don't see it as a big issue to make this change, but others may feel 
>> differently. If we do it, then we will need a release note pointing 
>> out the change of behavior.
>> 
>> -Flavio
>> 
>>> On 03 Oct 2016, at 08:54, Mohammad arshad 
>>> <mo...@huawei.com>
>> wrote:
>>> 
>>> Hi All,
>>> In Zookeeper rolling upgrade scenario where server is new but client 
>>> is
>> old, when sever sends error code which is not understood by a client, 
>> client throws IllegalArgumentException. Generally 
>> IllegalArgumentException is not handled by any of the ZK applications. 
>> It is too generic. How to handle this scenario in ZK applications?
>>> My understanding is instead of throwing IllegalArgumentException we
>> should throw a subclass of KeeperException, for example 
>> InvalidErrorCodeException, so that zk apps can take more specific action.
>>> Any thoughts?
>>> 
>>> Thanks
>>> -Arshad
>>> 
>> 
>> 


Re: ZooKeeper clients does not handle new error codes properly

Posted by Flavio Junqueira <fp...@apache.org>.
Yeah, unfortunately, I don't think we make use of the protocol version in ConnectRequest and Response. When we instantiate ConnectRequest in ClientCnxn, we simply set it to zero. 

-Flavio

> On 04 Oct 2016, at 16:21, Mohammad arshad <mo...@huawei.com> wrote:
> 
> Thanks Benjamin Reed,
> Client protocol version is 0, it is not changed. Also there is no logic executed at the server side based on the client protocol version. Increasing the client protocol version and converting the errors at server may be the ideal and more precise solution but to handle it in a generic way can we convert the unknown error to KeeperException.SystemErrorException at client side? Do you foresee any problem in this solution? 
> 
> Thanks
> -Arshad
> 
> 
> -----Original Message-----
> From: Benjamin Reed [mailto:breed@apache.org] 
> Sent: 04 October 2016 10:27
> To: user@zookeeper.apache.org
> Cc: DevZooKeeper
> Subject: Re: ZooKeeper clients does not handle new error codes properly
> 
> did we bump the protocol version when we added the new errors? the server could do the conversion when it responds to older clients.
> 
> On Mon, Oct 3, 2016 at 3:05 AM, Flavio Junqueira <fp...@apache.org> wrote:
> 
>> Hi Arshad,
>> 
>> It makes sense to me. What if we convert unknown server errors to 
>> KeeperException.SystemErrorException? This is a generic error and it 
>> extends KeeperException.
>> 
>> I don't see it as a big issue to make this change, but others may feel 
>> differently. If we do it, then we will need a release note pointing 
>> out the change of behavior.
>> 
>> -Flavio
>> 
>>> On 03 Oct 2016, at 08:54, Mohammad arshad 
>>> <mo...@huawei.com>
>> wrote:
>>> 
>>> Hi All,
>>> In Zookeeper rolling upgrade scenario where server is new but client 
>>> is
>> old, when sever sends error code which is not understood by a client, 
>> client throws IllegalArgumentException. Generally 
>> IllegalArgumentException is not handled by any of the ZK applications. 
>> It is too generic. How to handle this scenario in ZK applications?
>>> My understanding is instead of throwing IllegalArgumentException we
>> should throw a subclass of KeeperException, for example 
>> InvalidErrorCodeException, so that zk apps can take more specific action.
>>> Any thoughts?
>>> 
>>> Thanks
>>> -Arshad
>>> 
>> 
>> 


RE: ZooKeeper clients does not handle new error codes properly

Posted by Mohammad arshad <mo...@huawei.com>.
Created ZOOKEEPER-2695 to handle this scenario by throwing KeeperException.SystemErrorException instead of IllegalArgumentException

Best Regards
Mohammad Arshad
HUAWEI TECHNOLOGIES CO.LTD.    
Huawei Tecnologies India Pvt. Ltd.
Near EPIP Industrial Area, Kundalahalli Village
Whitefield, Bangalore-560066
www.huawei.com
-----------------------------------------------------------------------------------------------------------------
This e-mail and its attachments contain confidential information from HUAWEI, which 
is intended only for the person or entity whose address is listed above. Any use of the 
information contained herein in any way (including, but not limited to, total or partial 
disclosure, reproduction, or dissemination) by persons other than the intended 
recipient(s) is prohibited. If you receive this e-mail in error, please notify the sender by 
phone or email immediately and delete it!

-----Original Message-----
From: Mohammad arshad 
Sent: 05 October 2016 19:34
To: dev@zookeeper.apache.org; UserZooKeeper
Subject: RE: ZooKeeper clients does not handle new error codes properly

Thanks Michael Han for the information.
It is the same place in my case as well.

-----Original Message-----
From: Michael Han [mailto:hanm@cloudera.com]
Sent: 04 October 2016 23:08
To: UserZooKeeper
Cc: dev@zookeeper.apache.org
Subject: Re: ZooKeeper clients does not handle new error codes properly

Hi Arshad,

I am curious where the IllegalArgumentException was thrown in code in your use case - I am asking because I spotted that in KeeperException.create, we have a default fall through [1] that throws IllegalArgumentException, and if this is not the place where you observed the IllegalArgumentException in your upgrade use case, then we may also need think to change the default fall through here from throwing IllegalArgumentException to return something like SystemErrorException, because it seems possible that unknown / not mapped error codes can be passed to KeeperException.create that will lead to IllegalArgumentException that client can't handle well?

[1] https://github.com/apache/zookeeper/blob/master/src/
java/main/org/apache/zookeeper/KeeperException.java#L144

On Tue, Oct 4, 2016 at 8:21 AM, Mohammad arshad <mo...@huawei.com>
wrote:

> Thanks Benjamin Reed,
> Client protocol version is 0, it is not changed. Also there is no 
> logic executed at the server side based on the client protocol version.
> Increasing the client protocol version and converting the errors at 
> server may be the ideal and more precise solution but to handle it in 
> a generic way can we convert the unknown error to 
> KeeperException.SystemErrorException
> at client side? Do you foresee any problem in this solution?
>
> Thanks
> -Arshad
>
>
> -----Original Message-----
> From: Benjamin Reed [mailto:breed@apache.org]
> Sent: 04 October 2016 10:27
> To: user@zookeeper.apache.org
> Cc: DevZooKeeper
> Subject: Re: ZooKeeper clients does not handle new error codes 
> properly
>
> did we bump the protocol version when we added the new errors? the 
> server could do the conversion when it responds to older clients.
>
> On Mon, Oct 3, 2016 at 3:05 AM, Flavio Junqueira <fp...@apache.org> wrote:
>
> > Hi Arshad,
> >
> > It makes sense to me. What if we convert unknown server errors to 
> > KeeperException.SystemErrorException? This is a generic error and it 
> > extends KeeperException.
> >
> > I don't see it as a big issue to make this change, but others may 
> > feel differently. If we do it, then we will need a release note 
> > pointing out the change of behavior.
> >
> > -Flavio
> >
> > > On 03 Oct 2016, at 08:54, Mohammad arshad 
> > > <mo...@huawei.com>
> > wrote:
> > >
> > > Hi All,
> > > In Zookeeper rolling upgrade scenario where server is new but 
> > > client is
> > old, when sever sends error code which is not understood by a 
> > client, client throws IllegalArgumentException. Generally 
> > IllegalArgumentException is not handled by any of the ZK applications.
> > It is too generic. How to handle this scenario in ZK applications?
> > > My understanding is instead of throwing IllegalArgumentException 
> > > we
> > should throw a subclass of KeeperException, for example 
> > InvalidErrorCodeException, so that zk apps can take more specific action.
> > > Any thoughts?
> > >
> > > Thanks
> > > -Arshad
> > >
> >
> >
>



--
Cheers
Michael.

RE: ZooKeeper clients does not handle new error codes properly

Posted by Mohammad arshad <mo...@huawei.com>.
Thanks Michael Han for the information.
It is the same place in my case as well.

-----Original Message-----
From: Michael Han [mailto:hanm@cloudera.com] 
Sent: 04 October 2016 23:08
To: UserZooKeeper
Cc: dev@zookeeper.apache.org
Subject: Re: ZooKeeper clients does not handle new error codes properly

Hi Arshad,

I am curious where the IllegalArgumentException was thrown in code in your use case - I am asking because I spotted that in KeeperException.create, we have a default fall through [1] that throws IllegalArgumentException, and if this is not the place where you observed the IllegalArgumentException in your upgrade use case, then we may also need think to change the default fall through here from throwing IllegalArgumentException to return something like SystemErrorException, because it seems possible that unknown / not mapped error codes can be passed to KeeperException.create that will lead to IllegalArgumentException that client can't handle well?

[1] https://github.com/apache/zookeeper/blob/master/src/
java/main/org/apache/zookeeper/KeeperException.java#L144

On Tue, Oct 4, 2016 at 8:21 AM, Mohammad arshad <mo...@huawei.com>
wrote:

> Thanks Benjamin Reed,
> Client protocol version is 0, it is not changed. Also there is no 
> logic executed at the server side based on the client protocol version.
> Increasing the client protocol version and converting the errors at 
> server may be the ideal and more precise solution but to handle it in 
> a generic way can we convert the unknown error to 
> KeeperException.SystemErrorException
> at client side? Do you foresee any problem in this solution?
>
> Thanks
> -Arshad
>
>
> -----Original Message-----
> From: Benjamin Reed [mailto:breed@apache.org]
> Sent: 04 October 2016 10:27
> To: user@zookeeper.apache.org
> Cc: DevZooKeeper
> Subject: Re: ZooKeeper clients does not handle new error codes 
> properly
>
> did we bump the protocol version when we added the new errors? the 
> server could do the conversion when it responds to older clients.
>
> On Mon, Oct 3, 2016 at 3:05 AM, Flavio Junqueira <fp...@apache.org> wrote:
>
> > Hi Arshad,
> >
> > It makes sense to me. What if we convert unknown server errors to 
> > KeeperException.SystemErrorException? This is a generic error and it 
> > extends KeeperException.
> >
> > I don't see it as a big issue to make this change, but others may 
> > feel differently. If we do it, then we will need a release note 
> > pointing out the change of behavior.
> >
> > -Flavio
> >
> > > On 03 Oct 2016, at 08:54, Mohammad arshad 
> > > <mo...@huawei.com>
> > wrote:
> > >
> > > Hi All,
> > > In Zookeeper rolling upgrade scenario where server is new but 
> > > client is
> > old, when sever sends error code which is not understood by a 
> > client, client throws IllegalArgumentException. Generally 
> > IllegalArgumentException is not handled by any of the ZK applications.
> > It is too generic. How to handle this scenario in ZK applications?
> > > My understanding is instead of throwing IllegalArgumentException 
> > > we
> > should throw a subclass of KeeperException, for example 
> > InvalidErrorCodeException, so that zk apps can take more specific action.
> > > Any thoughts?
> > >
> > > Thanks
> > > -Arshad
> > >
> >
> >
>



--
Cheers
Michael.

RE: ZooKeeper clients does not handle new error codes properly

Posted by Mohammad arshad <mo...@huawei.com>.
Created ZOOKEEPER-2695 to handle this scenario by throwing KeeperException.SystemErrorException instead of IllegalArgumentException

Best Regards
Mohammad Arshad
HUAWEI TECHNOLOGIES CO.LTD.    
Huawei Tecnologies India Pvt. Ltd.
Near EPIP Industrial Area, Kundalahalli Village
Whitefield, Bangalore-560066
www.huawei.com
-----------------------------------------------------------------------------------------------------------------
This e-mail and its attachments contain confidential information from HUAWEI, which 
is intended only for the person or entity whose address is listed above. Any use of the 
information contained herein in any way (including, but not limited to, total or partial 
disclosure, reproduction, or dissemination) by persons other than the intended 
recipient(s) is prohibited. If you receive this e-mail in error, please notify the sender by 
phone or email immediately and delete it!

-----Original Message-----
From: Mohammad arshad 
Sent: 05 October 2016 19:34
To: dev@zookeeper.apache.org; UserZooKeeper
Subject: RE: ZooKeeper clients does not handle new error codes properly

Thanks Michael Han for the information.
It is the same place in my case as well.

-----Original Message-----
From: Michael Han [mailto:hanm@cloudera.com]
Sent: 04 October 2016 23:08
To: UserZooKeeper
Cc: dev@zookeeper.apache.org
Subject: Re: ZooKeeper clients does not handle new error codes properly

Hi Arshad,

I am curious where the IllegalArgumentException was thrown in code in your use case - I am asking because I spotted that in KeeperException.create, we have a default fall through [1] that throws IllegalArgumentException, and if this is not the place where you observed the IllegalArgumentException in your upgrade use case, then we may also need think to change the default fall through here from throwing IllegalArgumentException to return something like SystemErrorException, because it seems possible that unknown / not mapped error codes can be passed to KeeperException.create that will lead to IllegalArgumentException that client can't handle well?

[1] https://github.com/apache/zookeeper/blob/master/src/
java/main/org/apache/zookeeper/KeeperException.java#L144

On Tue, Oct 4, 2016 at 8:21 AM, Mohammad arshad <mo...@huawei.com>
wrote:

> Thanks Benjamin Reed,
> Client protocol version is 0, it is not changed. Also there is no 
> logic executed at the server side based on the client protocol version.
> Increasing the client protocol version and converting the errors at 
> server may be the ideal and more precise solution but to handle it in 
> a generic way can we convert the unknown error to 
> KeeperException.SystemErrorException
> at client side? Do you foresee any problem in this solution?
>
> Thanks
> -Arshad
>
>
> -----Original Message-----
> From: Benjamin Reed [mailto:breed@apache.org]
> Sent: 04 October 2016 10:27
> To: user@zookeeper.apache.org
> Cc: DevZooKeeper
> Subject: Re: ZooKeeper clients does not handle new error codes 
> properly
>
> did we bump the protocol version when we added the new errors? the 
> server could do the conversion when it responds to older clients.
>
> On Mon, Oct 3, 2016 at 3:05 AM, Flavio Junqueira <fp...@apache.org> wrote:
>
> > Hi Arshad,
> >
> > It makes sense to me. What if we convert unknown server errors to 
> > KeeperException.SystemErrorException? This is a generic error and it 
> > extends KeeperException.
> >
> > I don't see it as a big issue to make this change, but others may 
> > feel differently. If we do it, then we will need a release note 
> > pointing out the change of behavior.
> >
> > -Flavio
> >
> > > On 03 Oct 2016, at 08:54, Mohammad arshad 
> > > <mo...@huawei.com>
> > wrote:
> > >
> > > Hi All,
> > > In Zookeeper rolling upgrade scenario where server is new but 
> > > client is
> > old, when sever sends error code which is not understood by a 
> > client, client throws IllegalArgumentException. Generally 
> > IllegalArgumentException is not handled by any of the ZK applications.
> > It is too generic. How to handle this scenario in ZK applications?
> > > My understanding is instead of throwing IllegalArgumentException 
> > > we
> > should throw a subclass of KeeperException, for example 
> > InvalidErrorCodeException, so that zk apps can take more specific action.
> > > Any thoughts?
> > >
> > > Thanks
> > > -Arshad
> > >
> >
> >
>



--
Cheers
Michael.

RE: ZooKeeper clients does not handle new error codes properly

Posted by Mohammad arshad <mo...@huawei.com>.
Thanks Michael Han for the information.
It is the same place in my case as well.

-----Original Message-----
From: Michael Han [mailto:hanm@cloudera.com] 
Sent: 04 October 2016 23:08
To: UserZooKeeper
Cc: dev@zookeeper.apache.org
Subject: Re: ZooKeeper clients does not handle new error codes properly

Hi Arshad,

I am curious where the IllegalArgumentException was thrown in code in your use case - I am asking because I spotted that in KeeperException.create, we have a default fall through [1] that throws IllegalArgumentException, and if this is not the place where you observed the IllegalArgumentException in your upgrade use case, then we may also need think to change the default fall through here from throwing IllegalArgumentException to return something like SystemErrorException, because it seems possible that unknown / not mapped error codes can be passed to KeeperException.create that will lead to IllegalArgumentException that client can't handle well?

[1] https://github.com/apache/zookeeper/blob/master/src/
java/main/org/apache/zookeeper/KeeperException.java#L144

On Tue, Oct 4, 2016 at 8:21 AM, Mohammad arshad <mo...@huawei.com>
wrote:

> Thanks Benjamin Reed,
> Client protocol version is 0, it is not changed. Also there is no 
> logic executed at the server side based on the client protocol version.
> Increasing the client protocol version and converting the errors at 
> server may be the ideal and more precise solution but to handle it in 
> a generic way can we convert the unknown error to 
> KeeperException.SystemErrorException
> at client side? Do you foresee any problem in this solution?
>
> Thanks
> -Arshad
>
>
> -----Original Message-----
> From: Benjamin Reed [mailto:breed@apache.org]
> Sent: 04 October 2016 10:27
> To: user@zookeeper.apache.org
> Cc: DevZooKeeper
> Subject: Re: ZooKeeper clients does not handle new error codes 
> properly
>
> did we bump the protocol version when we added the new errors? the 
> server could do the conversion when it responds to older clients.
>
> On Mon, Oct 3, 2016 at 3:05 AM, Flavio Junqueira <fp...@apache.org> wrote:
>
> > Hi Arshad,
> >
> > It makes sense to me. What if we convert unknown server errors to 
> > KeeperException.SystemErrorException? This is a generic error and it 
> > extends KeeperException.
> >
> > I don't see it as a big issue to make this change, but others may 
> > feel differently. If we do it, then we will need a release note 
> > pointing out the change of behavior.
> >
> > -Flavio
> >
> > > On 03 Oct 2016, at 08:54, Mohammad arshad 
> > > <mo...@huawei.com>
> > wrote:
> > >
> > > Hi All,
> > > In Zookeeper rolling upgrade scenario where server is new but 
> > > client is
> > old, when sever sends error code which is not understood by a 
> > client, client throws IllegalArgumentException. Generally 
> > IllegalArgumentException is not handled by any of the ZK applications.
> > It is too generic. How to handle this scenario in ZK applications?
> > > My understanding is instead of throwing IllegalArgumentException 
> > > we
> > should throw a subclass of KeeperException, for example 
> > InvalidErrorCodeException, so that zk apps can take more specific action.
> > > Any thoughts?
> > >
> > > Thanks
> > > -Arshad
> > >
> >
> >
>



--
Cheers
Michael.

Re: ZooKeeper clients does not handle new error codes properly

Posted by Michael Han <ha...@cloudera.com>.
Hi Arshad,

I am curious where the IllegalArgumentException was thrown in code in your
use case - I am asking because I spotted that in KeeperException.create, we
have a default fall through [1] that throws IllegalArgumentException, and
if this is not the place where you observed the IllegalArgumentException in
your upgrade use case, then we may also need think to change the default
fall through here from throwing IllegalArgumentException to return
something like SystemErrorException, because it seems possible that unknown
/ not mapped error codes can be passed to KeeperException.create that will
lead to IllegalArgumentException that client can't handle well?

[1] https://github.com/apache/zookeeper/blob/master/src/
java/main/org/apache/zookeeper/KeeperException.java#L144

On Tue, Oct 4, 2016 at 8:21 AM, Mohammad arshad <mo...@huawei.com>
wrote:

> Thanks Benjamin Reed,
> Client protocol version is 0, it is not changed. Also there is no logic
> executed at the server side based on the client protocol version.
> Increasing the client protocol version and converting the errors at server
> may be the ideal and more precise solution but to handle it in a generic
> way can we convert the unknown error to KeeperException.SystemErrorException
> at client side? Do you foresee any problem in this solution?
>
> Thanks
> -Arshad
>
>
> -----Original Message-----
> From: Benjamin Reed [mailto:breed@apache.org]
> Sent: 04 October 2016 10:27
> To: user@zookeeper.apache.org
> Cc: DevZooKeeper
> Subject: Re: ZooKeeper clients does not handle new error codes properly
>
> did we bump the protocol version when we added the new errors? the server
> could do the conversion when it responds to older clients.
>
> On Mon, Oct 3, 2016 at 3:05 AM, Flavio Junqueira <fp...@apache.org> wrote:
>
> > Hi Arshad,
> >
> > It makes sense to me. What if we convert unknown server errors to
> > KeeperException.SystemErrorException? This is a generic error and it
> > extends KeeperException.
> >
> > I don't see it as a big issue to make this change, but others may feel
> > differently. If we do it, then we will need a release note pointing
> > out the change of behavior.
> >
> > -Flavio
> >
> > > On 03 Oct 2016, at 08:54, Mohammad arshad
> > > <mo...@huawei.com>
> > wrote:
> > >
> > > Hi All,
> > > In Zookeeper rolling upgrade scenario where server is new but client
> > > is
> > old, when sever sends error code which is not understood by a client,
> > client throws IllegalArgumentException. Generally
> > IllegalArgumentException is not handled by any of the ZK applications.
> > It is too generic. How to handle this scenario in ZK applications?
> > > My understanding is instead of throwing IllegalArgumentException we
> > should throw a subclass of KeeperException, for example
> > InvalidErrorCodeException, so that zk apps can take more specific action.
> > > Any thoughts?
> > >
> > > Thanks
> > > -Arshad
> > >
> >
> >
>



-- 
Cheers
Michael.

Re: ZooKeeper clients does not handle new error codes properly

Posted by Michael Han <ha...@cloudera.com>.
Hi Arshad,

I am curious where the IllegalArgumentException was thrown in code in your
use case - I am asking because I spotted that in KeeperException.create, we
have a default fall through [1] that throws IllegalArgumentException, and
if this is not the place where you observed the IllegalArgumentException in
your upgrade use case, then we may also need think to change the default
fall through here from throwing IllegalArgumentException to return
something like SystemErrorException, because it seems possible that unknown
/ not mapped error codes can be passed to KeeperException.create that will
lead to IllegalArgumentException that client can't handle well?

[1] https://github.com/apache/zookeeper/blob/master/src/
java/main/org/apache/zookeeper/KeeperException.java#L144

On Tue, Oct 4, 2016 at 8:21 AM, Mohammad arshad <mo...@huawei.com>
wrote:

> Thanks Benjamin Reed,
> Client protocol version is 0, it is not changed. Also there is no logic
> executed at the server side based on the client protocol version.
> Increasing the client protocol version and converting the errors at server
> may be the ideal and more precise solution but to handle it in a generic
> way can we convert the unknown error to KeeperException.SystemErrorException
> at client side? Do you foresee any problem in this solution?
>
> Thanks
> -Arshad
>
>
> -----Original Message-----
> From: Benjamin Reed [mailto:breed@apache.org]
> Sent: 04 October 2016 10:27
> To: user@zookeeper.apache.org
> Cc: DevZooKeeper
> Subject: Re: ZooKeeper clients does not handle new error codes properly
>
> did we bump the protocol version when we added the new errors? the server
> could do the conversion when it responds to older clients.
>
> On Mon, Oct 3, 2016 at 3:05 AM, Flavio Junqueira <fp...@apache.org> wrote:
>
> > Hi Arshad,
> >
> > It makes sense to me. What if we convert unknown server errors to
> > KeeperException.SystemErrorException? This is a generic error and it
> > extends KeeperException.
> >
> > I don't see it as a big issue to make this change, but others may feel
> > differently. If we do it, then we will need a release note pointing
> > out the change of behavior.
> >
> > -Flavio
> >
> > > On 03 Oct 2016, at 08:54, Mohammad arshad
> > > <mo...@huawei.com>
> > wrote:
> > >
> > > Hi All,
> > > In Zookeeper rolling upgrade scenario where server is new but client
> > > is
> > old, when sever sends error code which is not understood by a client,
> > client throws IllegalArgumentException. Generally
> > IllegalArgumentException is not handled by any of the ZK applications.
> > It is too generic. How to handle this scenario in ZK applications?
> > > My understanding is instead of throwing IllegalArgumentException we
> > should throw a subclass of KeeperException, for example
> > InvalidErrorCodeException, so that zk apps can take more specific action.
> > > Any thoughts?
> > >
> > > Thanks
> > > -Arshad
> > >
> >
> >
>



-- 
Cheers
Michael.

RE: ZooKeeper clients does not handle new error codes properly

Posted by Mohammad arshad <mo...@huawei.com>.
Thanks Benjamin Reed,
Client protocol version is 0, it is not changed. Also there is no logic executed at the server side based on the client protocol version. Increasing the client protocol version and converting the errors at server may be the ideal and more precise solution but to handle it in a generic way can we convert the unknown error to KeeperException.SystemErrorException at client side? Do you foresee any problem in this solution? 

Thanks
-Arshad


-----Original Message-----
From: Benjamin Reed [mailto:breed@apache.org] 
Sent: 04 October 2016 10:27
To: user@zookeeper.apache.org
Cc: DevZooKeeper
Subject: Re: ZooKeeper clients does not handle new error codes properly

did we bump the protocol version when we added the new errors? the server could do the conversion when it responds to older clients.

On Mon, Oct 3, 2016 at 3:05 AM, Flavio Junqueira <fp...@apache.org> wrote:

> Hi Arshad,
>
> It makes sense to me. What if we convert unknown server errors to 
> KeeperException.SystemErrorException? This is a generic error and it 
> extends KeeperException.
>
> I don't see it as a big issue to make this change, but others may feel 
> differently. If we do it, then we will need a release note pointing 
> out the change of behavior.
>
> -Flavio
>
> > On 03 Oct 2016, at 08:54, Mohammad arshad 
> > <mo...@huawei.com>
> wrote:
> >
> > Hi All,
> > In Zookeeper rolling upgrade scenario where server is new but client 
> > is
> old, when sever sends error code which is not understood by a client, 
> client throws IllegalArgumentException. Generally 
> IllegalArgumentException is not handled by any of the ZK applications. 
> It is too generic. How to handle this scenario in ZK applications?
> > My understanding is instead of throwing IllegalArgumentException we
> should throw a subclass of KeeperException, for example 
> InvalidErrorCodeException, so that zk apps can take more specific action.
> > Any thoughts?
> >
> > Thanks
> > -Arshad
> >
>
>

RE: ZooKeeper clients does not handle new error codes properly

Posted by Mohammad arshad <mo...@huawei.com>.
Thanks Benjamin Reed,
Client protocol version is 0, it is not changed. Also there is no logic executed at the server side based on the client protocol version. Increasing the client protocol version and converting the errors at server may be the ideal and more precise solution but to handle it in a generic way can we convert the unknown error to KeeperException.SystemErrorException at client side? Do you foresee any problem in this solution? 

Thanks
-Arshad


-----Original Message-----
From: Benjamin Reed [mailto:breed@apache.org] 
Sent: 04 October 2016 10:27
To: user@zookeeper.apache.org
Cc: DevZooKeeper
Subject: Re: ZooKeeper clients does not handle new error codes properly

did we bump the protocol version when we added the new errors? the server could do the conversion when it responds to older clients.

On Mon, Oct 3, 2016 at 3:05 AM, Flavio Junqueira <fp...@apache.org> wrote:

> Hi Arshad,
>
> It makes sense to me. What if we convert unknown server errors to 
> KeeperException.SystemErrorException? This is a generic error and it 
> extends KeeperException.
>
> I don't see it as a big issue to make this change, but others may feel 
> differently. If we do it, then we will need a release note pointing 
> out the change of behavior.
>
> -Flavio
>
> > On 03 Oct 2016, at 08:54, Mohammad arshad 
> > <mo...@huawei.com>
> wrote:
> >
> > Hi All,
> > In Zookeeper rolling upgrade scenario where server is new but client 
> > is
> old, when sever sends error code which is not understood by a client, 
> client throws IllegalArgumentException. Generally 
> IllegalArgumentException is not handled by any of the ZK applications. 
> It is too generic. How to handle this scenario in ZK applications?
> > My understanding is instead of throwing IllegalArgumentException we
> should throw a subclass of KeeperException, for example 
> InvalidErrorCodeException, so that zk apps can take more specific action.
> > Any thoughts?
> >
> > Thanks
> > -Arshad
> >
>
>

Re: ZooKeeper clients does not handle new error codes properly

Posted by Benjamin Reed <br...@apache.org>.
did we bump the protocol version when we added the new errors? the server
could do the conversion when it responds to older clients.

On Mon, Oct 3, 2016 at 3:05 AM, Flavio Junqueira <fp...@apache.org> wrote:

> Hi Arshad,
>
> It makes sense to me. What if we convert unknown server errors to
> KeeperException.SystemErrorException? This is a generic error and it
> extends KeeperException.
>
> I don't see it as a big issue to make this change, but others may feel
> differently. If we do it, then we will need a release note pointing out the
> change of behavior.
>
> -Flavio
>
> > On 03 Oct 2016, at 08:54, Mohammad arshad <mo...@huawei.com>
> wrote:
> >
> > Hi All,
> > In Zookeeper rolling upgrade scenario where server is new but client is
> old, when sever sends error code which is not understood by a client,
> client throws IllegalArgumentException. Generally IllegalArgumentException
> is not handled by any of the ZK applications. It is too generic. How to
> handle this scenario in ZK applications?
> > My understanding is instead of throwing IllegalArgumentException we
> should throw a subclass of KeeperException, for example
> InvalidErrorCodeException, so that zk apps can take more specific action.
> > Any thoughts?
> >
> > Thanks
> > -Arshad
> >
>
>

Re: ZooKeeper clients does not handle new error codes properly

Posted by Benjamin Reed <br...@apache.org>.
did we bump the protocol version when we added the new errors? the server
could do the conversion when it responds to older clients.

On Mon, Oct 3, 2016 at 3:05 AM, Flavio Junqueira <fp...@apache.org> wrote:

> Hi Arshad,
>
> It makes sense to me. What if we convert unknown server errors to
> KeeperException.SystemErrorException? This is a generic error and it
> extends KeeperException.
>
> I don't see it as a big issue to make this change, but others may feel
> differently. If we do it, then we will need a release note pointing out the
> change of behavior.
>
> -Flavio
>
> > On 03 Oct 2016, at 08:54, Mohammad arshad <mo...@huawei.com>
> wrote:
> >
> > Hi All,
> > In Zookeeper rolling upgrade scenario where server is new but client is
> old, when sever sends error code which is not understood by a client,
> client throws IllegalArgumentException. Generally IllegalArgumentException
> is not handled by any of the ZK applications. It is too generic. How to
> handle this scenario in ZK applications?
> > My understanding is instead of throwing IllegalArgumentException we
> should throw a subclass of KeeperException, for example
> InvalidErrorCodeException, so that zk apps can take more specific action.
> > Any thoughts?
> >
> > Thanks
> > -Arshad
> >
>
>

Re: ZooKeeper clients does not handle new error codes properly

Posted by Flavio Junqueira <fp...@apache.org>.
Hi Arshad,

It makes sense to me. What if we convert unknown server errors to KeeperException.SystemErrorException? This is a generic error and it extends KeeperException.

I don't see it as a big issue to make this change, but others may feel differently. If we do it, then we will need a release note pointing out the change of behavior.

-Flavio

> On 03 Oct 2016, at 08:54, Mohammad arshad <mo...@huawei.com> wrote:
> 
> Hi All,
> In Zookeeper rolling upgrade scenario where server is new but client is old, when sever sends error code which is not understood by a client,  client throws IllegalArgumentException. Generally IllegalArgumentException is not handled by any of the ZK applications. It is too generic. How to handle this scenario in ZK applications?
> My understanding is instead of throwing IllegalArgumentException we should throw a subclass of KeeperException, for example InvalidErrorCodeException, so that zk apps can take more specific action.
> Any thoughts?
> 
> Thanks
> -Arshad
> 


Re: ZooKeeper clients does not handle new error codes properly

Posted by Flavio Junqueira <fp...@apache.org>.
Hi Arshad,

It makes sense to me. What if we convert unknown server errors to KeeperException.SystemErrorException? This is a generic error and it extends KeeperException.

I don't see it as a big issue to make this change, but others may feel differently. If we do it, then we will need a release note pointing out the change of behavior.

-Flavio

> On 03 Oct 2016, at 08:54, Mohammad arshad <mo...@huawei.com> wrote:
> 
> Hi All,
> In Zookeeper rolling upgrade scenario where server is new but client is old, when sever sends error code which is not understood by a client,  client throws IllegalArgumentException. Generally IllegalArgumentException is not handled by any of the ZK applications. It is too generic. How to handle this scenario in ZK applications?
> My understanding is instead of throwing IllegalArgumentException we should throw a subclass of KeeperException, for example InvalidErrorCodeException, so that zk apps can take more specific action.
> Any thoughts?
> 
> Thanks
> -Arshad
>