On 11/5/2012 6:36 AM, Sergey Beryozkin wrote:
> Hi
> On 04/11/12 20:36, Marek Potociar wrote:
>>
>> On Nov 4, 2012, at 8:16 PM, Sergey Beryozkin<sberyozkin_at_talend.com>
>> wrote:
>>
>>> On 02/11/12 18:51, Marek Potociar wrote:
>>>>
>>>> On Nov 2, 2012, at 6:13 PM, Sergey
>>>> Beryozkin<sberyozkin_at_talend.com> wrote:
>>>>
>>>>> Hi
>>>>> On 02/11/12 16:36, Marek Potociar wrote:
>>>>>> I'm still not sure what's the big picture in your proposal:
>>>>>>
>>>>>> - You indicated earlier that we should drop ClientException, now
>>>>>> you're
>>>>>> adding new ClientException subtypes.
>>>>>> - ResponseProcessingException seems like something that should be
>>>>>> better
>>>>>> derived from MessageProcessingException, but you're again using
>>>>>> ClientException
>>>>>>
>>>>>> Also, why should we create new connection-related exceptions if
>>>>>> Java SE
>>>>>> already providers a hierarchy of IOException subclasses in java.net
>>>>>> <http://java.net> package that cover these failures
>>>>>> (ConnectException,
>>>>>> UnknownHostException, NoRouteToHostException, HttpRetryException...)?
>>>>>>
>>>>>> It seems to me that, if we really want to do anything about improving
>>>>>> the current processing exception hierarchy, we could do e.g. this:
>>>>>>
>>>>>> 1. Drop ClientException
>>>>>> 2. Define a common ProcessingException instead and update client APIs
>>>>>> to throw ProcessingException
>>>>>> * On client side this exception would wrap any exceptions not
>>>>>> related to message processing in filters or interceptors,
>>>>>> including IOExceptions thrown by the transport layer or
>>>>>> MBR/MBW.
>>>>>> 3. Make MessageProcessingException a subclass of ProcessingException
>>>>>> 4. Add new client-side ResponseProcessingException subclass of
>>>>>> MessageProcessingException
>>>>>>
>>>>>> I know we did not get rid of getCause() in case of IOExceptions,
>>>>>> but I
>>>>>> it still seems to address most of the other issues you had (wrapping
>>>>>> filter/interceptor exceptions, missing response processing
>>>>>> exception).
>>>>>> Would that work for you? Sergey, others would that work for you too?
>>>>>>
>>>>>
>>>>> I just like the expression "ClientException". What does this
>>>>> proposal improve on the server side of things ? I think I'm more
>>>>> comfortable with the latest proposal from Bill, but I'd like a
>>>>> single catch for "ClientException" be sufficient, at the bare
>>>>> level, with respect to managing client-centric exception of all sorts
>>>>
>>>> You could have a single try-catch for a ProcessingException. That
>>>> would fully cover what ClientException covers and additionally
>>>> remove the need to invoke getCause for addressing message processing
>>>> exception sub-types. Additionally, that would also cover exception
>>>> thrown from Response.readEntity().
>>>>
>>>> Isn't that worth considering?
>>>>
>>>
>>> Sounds like a good idea. I'm just sensing something is not right.
>>> Purely refactoring wise it is spot on. I think there has to be
>>> construct there dedicated to representing "client" centric exceptions.
>>> We are going to lose it...
>>
>> I think I know what you mean - I had the same feeling. But facing the
>> facts I could not find a valid reason why the above would not work.
>> The only problematic use case I could think of is covered by a
>> client-specific ResponseProcessingException. So I really don't could
>> not find a solid argument why the above would not work. Plus, the
>> hierarchy really seems to make a lot of sense.
>>
>
> And how we'd represent client-side exceptions to do with the connection
> failures ? It is not a ProcessingException.
>
Why do we care if it is a processing exception or a connection
exception? If the name "ProcessingException" doesn't fit for the base
class, change it.
> With ClientException one can consider creating
> ConnectionTimeoutException, etc.
>
> It appears the refactoring is proposed is mostly because
> ClientException.getCause() is not considered a good programming
> practice...May be in some cases that is the case. But I disagree with
> this opinion leading to the refactoring which 'loses' the client-centric
> exception class.
>
> Exception.getCause() is there for a reason, it is when the underlying
> exception has to be visible to the catch block.
>
> When we do "client.get(Book.class)" is it that case exactly, as opposed
> to "Response r = client.get()". I can see why the refactoring can make
> ClientException.getCause() not necessary, but it really has its cons too.
>
> As far as various compromises are concerned, IMHO the current approach wins
>
I want to avoid getCause() logic on the client. If you can meet that
criteria, I'm ok with whatever you come up with.
--
Bill Burke
JBoss, a division of Red Hat
http://bill.burkecentral.com