[jira] [Comment Edited] (DIRMINA-941) DefaultIoFilterChain (or any other class) should not catch Throwable without re-throwing

["Emmanuel Lecharny (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/DIRMINA-941?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13615211#comment-13615211 ] 

Emmanuel Lecharny edited comment on DIRMINA-941 at 9/5/14 10:44 AM:
--------------------------------------------------------------------

You are perfectly right, we should *not* catch exceptions.

Except that in MINA, when an exception occurs, we catch it to generate a ExceptionCaught event for the Handler to manage it the way it wants. So far, so good : the IoHandler always gets the required informations needed to manage the exception. 

Except that when the exception is generated by a bad log library, we can't rethrow the exception, otherwise you'll enter into an infinite loop, as the exception will be caught, generate a ExceptionCaught event, which will throw an excption, etc...

At this point, I don't know how to avoid such a rare scenario... May be something like :

{code}
    private void callNextExceptionCaught(Entry entry, IoSession session, Throwable cause) {
        ...
            try {
                IoFilter filter = entry.getFilter();
                NextFilter nextFilter = entry.getNextFilter();
                filter.exceptionCaught(nextFilter, session, cause);
            } catch (Throwable e) {
                try {
                    LOGGER.warn("Unexpected exception from exceptionCaught handler.", e);
                } catch (Throwable t) {       // <<----- Here we suppose we have a pb with the logger, and we simply dump a stackTrace
                    t.printStackTrace();
                }
            }
{code}

If you have any better idea, feel free to propose something that could be helpful. 


was (Author: elecharny):
You are perfectly right, we should *not* catch exceptions.

Except that in MINA, when an exception occurs, we catch it to generate a ExceptionCaught event for the Handler to manage it the way it wants. So far, so good : the IoHandler always gets the required informations needed to manage the exception. 

Except that when the exception is generated by a bad log library, we can't rethrow the exception, otherwise you'll enter into an infinite loop, as the exception will be caught, generate a ExceptionCaught event, which will throw an excption, etc...

At this point, I don't know how to avoid such a rare scenario... May be something like :

    private void callNextExceptionCaught(Entry entry, IoSession session, Throwable cause) {
        ...
            try {
                IoFilter filter = entry.getFilter();
                NextFilter nextFilter = entry.getNextFilter();
                filter.exceptionCaught(nextFilter, session, cause);
            } catch (Throwable e) {
                try {
                    LOGGER.warn("Unexpected exception from exceptionCaught handler.", e);
                } catch (Throwable t) {       // <<----- Here we suppose we have a pb with the logger, and we simply dump a stackTrace
                    t.printStackTrace();
                }
            }

If you have any better idea, feel free to propose something that could be helpful. 

> DefaultIoFilterChain (or any other class) should not catch Throwable without re-throwing
> ----------------------------------------------------------------------------------------
>
>                 Key: DIRMINA-941
>                 URL: https://issues.apache.org/jira/browse/DIRMINA-941
>             Project: MINA
>          Issue Type: Improvement
>          Components: Core
>    Affects Versions: 2.0.5, 2.0.7
>         Environment: N/A
>            Reporter: Chris Janicki
>             Fix For: 2.0.8
>
>
> I spent several hours tracking down a bug that was ultimately caused by an old slf4j*-api.jar in my encompassing project's classpath.  The critical error message ("... method not found... isTraceEnabled()...") was silenced by the "catch Throwable" behavior in the org.apache.mina.core.filterchain.DefaultIoFilterChain class.  Since the problem was related to the logging classes, the error wasn't logged, and unfortunately, the runtime exception was not allowed to propagate to the console... Instead it was consumed in DefaultIoFilterChain.  This left absolutely no clues for debugging why the SSH server just stopped in the middle of key negotiation.  
> The only evidence of the problem was that the SSHD server I was embedding just stopped in the middle of the key exchange.  In order to finally find the problem, I had to track down the last known working part of code in the ssh-core package, and edit your source code to catch and print Throwables.  While I appreciate that this was possible because of your open source, it was beyond the normal programmer's expectation for embedding a library.  In my google searches for the last printed SSH client debug state message ("debug1: SSH2_MSG_KEXINIT sent") , I found several people over the last few years who have struggled for answers at the same point in the code when using Apache's Mina/sshd.  There's no indication that many of them succeeded in tracking down the problem.  (I'll go back and leave some breadcrumbs for future travelers, where I have logins.)
> My overall point is that it's not nice to catch Throwables.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)