synapse-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Hiranya Jayathilaka <hiranya...@gmail.com>
Subject Re: svn commit: r1504950 - /synapse/trunk/java/modules/transports/core/nhttp/src/main/java/org/apache/synapse/transport/nhttp/HttpCoreNIOSender.java
Date Sat, 03 Aug 2013 20:10:51 GMT
I agree that catching all instances of IllegalStateException and treating it as an indicator
of client closing the connection is wrong. In fact I think getting an IllegalStateException
when the client closes the connection is wrong (especially since there's a separate catch
block for handling ConnectionClosedExceptions). This could have been the behavior in an old
http core version, but I assume it's fixed now. At least in the recent times, I haven't seen
this error getting thrown when the client closes the connection.

IMO IllegalStateException should indicate serious bugs and possible programming errors in
the code. Therefore I'm far more comfortable with not special casing this error, and handling
it as a general error. With this fix, at least we'll get a stack trace that we can look at
and try to understand why this error is actually thrown (if it's thrown at all). 

This is probably not the ideal solution, but I think it's better than what we had. Feel free
to further improve it, if you got any ideas.

Thanks,
Hiranya

On Aug 3, 2013, at 11:01 AM, Andreas Veithen <andreas.veithen@gmail.com> wrote:

> I don't think that this is the correct fix for SYNAPSE-866. My
> interpretation is that the original intent of the code you removed was
> to avoid logging an exception when the client closes the connection
> (which is not a very exceptional situation). The problem is that this
> code is triggered also in other situations.
> 
> Andreas
> 
> On Fri, Jul 19, 2013 at 8:01 PM,  <hiranya@apache.org> wrote:
>> Author: hiranya
>> Date: Fri Jul 19 18:01:47 2013
>> New Revision: 1504950
>> 
>> URL: http://svn.apache.org/r1504950
>> Log:
>> Fixing SYNAPSE-866; Logging a more general error message with the full stack trace
in case of IllegalStateExceptions
>> 
>> Modified:
>>    synapse/trunk/java/modules/transports/core/nhttp/src/main/java/org/apache/synapse/transport/nhttp/HttpCoreNIOSender.java
>> 
>> Modified: synapse/trunk/java/modules/transports/core/nhttp/src/main/java/org/apache/synapse/transport/nhttp/HttpCoreNIOSender.java
>> URL: http://svn.apache.org/viewvc/synapse/trunk/java/modules/transports/core/nhttp/src/main/java/org/apache/synapse/transport/nhttp/HttpCoreNIOSender.java?rev=1504950&r1=1504949&r2=1504950&view=diff
>> ==============================================================================
>> --- synapse/trunk/java/modules/transports/core/nhttp/src/main/java/org/apache/synapse/transport/nhttp/HttpCoreNIOSender.java
(original)
>> +++ synapse/trunk/java/modules/transports/core/nhttp/src/main/java/org/apache/synapse/transport/nhttp/HttpCoreNIOSender.java
Fri Jul 19 18:01:47 2013
>> @@ -554,11 +554,6 @@ public class HttpCoreNIOSender extends A
>>                 lstMetrics.incrementFaultsSending();
>>             }
>>             log.warn("Connection closed by client : " + worker.getRemoteAddress());
>> -        } catch (IllegalStateException e) {
>> -            if (lstMetrics != null) {
>> -                lstMetrics.incrementFaultsSending();
>> -            }
>> -            log.warn("Connection closed by client : " + worker.getRemoteAddress());
>>         } catch (IOException e) {
>>             if (lstMetrics != null) {
>>                 lstMetrics.incrementFaultsSending();
>> 
>> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@synapse.apache.org
> For additional commands, e-mail: dev-help@synapse.apache.org
> 

--
Hiranya Jayathilaka
Mayhem Lab/RACE Lab;
Dept. of Computer Science, UCSB;  http://cs.ucsb.edu
E-mail: hiranya@cs.ucsb.edu;  Mobile: +1 (805) 895-7443
Blog: http://techfeast-hiranya.blogspot.com


Mime
View raw message