[jira] [Commented] (SSHD-1058) ChannelOutputStream#flush should not log at error level

["Lyor Goldstein (Jira)" <ji...@apache.org>]

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

Lyor Goldstein commented on SSHD-1058:
--------------------------------------

We apply a consistent policy of logging exception in our code which basically states that exception "summaries" are logged at ERROR level and their details at INFO if DEBUG is enabled. I am not convinced that the use-case presented in this issue is compelling enough to make an exception in this case - though I do understand the reasoning behind it and can relate to it.

{quote}
The caller should decide how to deal with the exception and log at error level if needed.
{quote}

The same may be said for +any+ exception thrown by the code - not just here. For your use-case, this particular exception logging is "annoying" - for others there may be other use-cases that "annoy" them. We need to choose a certain policy that we think will be acceptable enough by the vast majority of our users - hence the choice we made.

> ChannelOutputStream#flush should not log at error level
> -------------------------------------------------------
>
>                 Key: SSHD-1058
>                 URL: https://issues.apache.org/jira/browse/SSHD-1058
>             Project: MINA SSHD
>          Issue Type: Bug
>    Affects Versions: 2.5.0, 2.5.1
>         Environment: Linux, Java 11
>            Reporter: Markus Spann
>            Assignee: Lyor Goldstein
>            Priority: Minor
>
> When calling flush on ChannelOutputStream, Window.waitForSpace may throw a WindowClosedException. This exception is always logged at error level, then rethrown. If Debug is enabled on the logger, the error log is issued twice (second time with stacktrace).
> During deinitialization it is common practice to call flush on output streams. Due to the asynchronous nature of the code in this library, the call may fail. The caller should decide how to deal with the exception and log at error level if needed.
> The implementation should not log at error level regardless.
> I would suggest INFO level instead (if logging at all).
>  
> {code:java}
> // lines 202 ff.
> if (log.isDebugEnabled()) {
>     log.info("flush({}) failed ({}) to wait for space of len={}: {}: ", this, e.getClass().getSimpleName(), total, e.getMessage(), e); // with stacktrace
> } else {
>     log.info("flush({}) failed ({}) to wait for space of len={}: {}",
>  this, e.getClass().getSimpleName(), total, e.getMessage());
> }
> throw e;
> {code}
> Thanks,
> Markus
>  
>  



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org