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 <[email protected]> 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, <[email protected]> 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: [email protected] > For additional commands, e-mail: [email protected] > -- Hiranya Jayathilaka Mayhem Lab/RACE Lab; Dept. of Computer Science, UCSB; http://cs.ucsb.edu E-mail: [email protected]; Mobile: +1 (805) 895-7443 Blog: http://techfeast-hiranya.blogspot.com
