[jira] [Comment Edited] (SSHD-625) Use catch(Throwable) where possible to avoid stopping performing queries and to log errors

["Eugene Petrenko (JIRA)" <ji...@apache.org>]

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

Eugene Petrenko edited comment on SSHD-625 at 1/25/16 10:23 AM:
----------------------------------------------------------------

Thanks for working on it :) 

It looks reasonable to me to include 
{code} catch(Throwable t) { logger.error("Some operation failed. " + t.getMessage(), t); DO APP RELATED FALLBACK } 
{code} 
in cases where I call some plugable part of code. The sole is I do not rely on the plugable part. It also mean I extend plugable part contract to allow any exceptions to be thrown. Still it's ok to simply deny execution of an activity that failed that way, but with logging and maybe with channel close or similar. It looks also OK to me to do {{log.error}} or {{log.warn}} in such cases. 
Actually, in the long run such logging may be helpful to everyone who is debugging issues. It makes the server itself robust to any incorrect behaviour from the plugable parts


was (Author: jonnyzzz):
Thanks for working on it :) 

It looks reasonable to me to include {{ catch(Throwable t) { logger.error("Some operation failed. " + t.getMessage(), t); DO APP RELATED FALLBACK } }} in cases where I call some plugable part of code. The sole is I do not rely on the plugable part. It also mean I extend plugable part contract to allow any exceptions to be thrown. Still it's ok to simply deny execution of an activity that failed that way, but with logging and maybe with channel close or similar. It looks also OK to me to do {{log.error}} or {{log.warn}} in such cases. 
Actually, in the long run such logging may be helpful to everyone who is debugging issues. It makes the server itself robust to any incorrect behaviour from the plugable parts

> Use catch(Throwable) where possible to avoid stopping performing queries and to log errors
> ------------------------------------------------------------------------------------------
>
>                 Key: SSHD-625
>                 URL: https://issues.apache.org/jira/browse/SSHD-625
>             Project: MINA SSHD
>          Issue Type: Bug
>    Affects Versions: 1.0.0
>            Reporter: Eugene Petrenko
>            Assignee: Goldstein Lyor
>            Priority: Minor
>
> There are several places in library source where you catch Exception to log it. Not Throwable. But it may easily turn out any some Error is thrown from user provided implementations.
> Here is incorrect catch: * org.apache.sshd.server.config.keys.AuthorizedKeysAuthenticator#authenticate
> There is no catch at all in 
> * org.apache.sshd.server.auth.UserAuthKeyboardInteractive#doAuth
> * org.apache.sshd.server.channel.ChannelSession#handleRequest
> The problem is that once my code throws a Error (e.g. VerifyError or OOM) the error goes strait into JVM internals without being explicitly logged. As a side effect of that SSHD server stops functioning. Connections are accepted but not being processed.
> My current workaround is to explicitly wrap all classes I implement with try/catch. I believe it should be done inside the library to avoid everyone from writing similar try-catch. I believe it's OK to forcibly close any connection once exception was caught and logged



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