From 417583ccaa6bcad739056b1ce2536ccf3b128743 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois-Xavier=20Thomas?= Date: Sun, 10 Mar 2019 21:40:24 +0100 Subject: [PATCH 1/4] Do not show stack trace for client-side connection errors The `ClientAbortException` exception indicates that the connection was closed by the client, usually for something the server can do nothing about (e.g. navigating outside of the page while it's loading). Since this error happens often, this commit displays shorter error messages when it does, without a large stack trace. All other exceptions are handled just as before. --- .../player/controller/StreamController.java | 5 ++--- .../player/spring/LoggingExceptionResolver.java | 11 ++++++++--- .../main/java/org/airsonic/player/util/Util.java | 15 +++++++++++++++ 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/airsonic-main/src/main/java/org/airsonic/player/controller/StreamController.java b/airsonic-main/src/main/java/org/airsonic/player/controller/StreamController.java index 7a038637..fc97b95c 100644 --- a/airsonic-main/src/main/java/org/airsonic/player/controller/StreamController.java +++ b/airsonic-main/src/main/java/org/airsonic/player/controller/StreamController.java @@ -249,8 +249,8 @@ public class StreamController { } } } - } catch (ClientAbortException err) { - LOG.info("org.apache.catalina.connector.ClientAbortException: Connection reset"); + } catch (ClientAbortException e) { + LOG.info("{}: Client unexpectedly closed connection while loading {} ({})", request.getRemoteAddr(), Util.getURLForRequest(request), e.getCause().toString()); return; } finally { if (status != null) { @@ -426,5 +426,4 @@ public class StreamController { out.write(buf); out.flush(); } - } diff --git a/airsonic-main/src/main/java/org/airsonic/player/spring/LoggingExceptionResolver.java b/airsonic-main/src/main/java/org/airsonic/player/spring/LoggingExceptionResolver.java index f374b74d..6ee5b55a 100644 --- a/airsonic-main/src/main/java/org/airsonic/player/spring/LoggingExceptionResolver.java +++ b/airsonic-main/src/main/java/org/airsonic/player/spring/LoggingExceptionResolver.java @@ -1,5 +1,6 @@ package org.airsonic.player.spring; +import org.airsonic.player.util.Util; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.core.Ordered; @@ -11,13 +12,17 @@ import javax.servlet.http.HttpServletResponse; public class LoggingExceptionResolver implements HandlerExceptionResolver, Ordered { - private static final Logger logger = LoggerFactory.getLogger(LoggingExceptionResolver.class); + private static final Logger LOG = LoggerFactory.getLogger(LoggingExceptionResolver.class); @Override public ModelAndView resolveException( - HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse, Object o, Exception e + HttpServletRequest request, HttpServletResponse response, Object o, Exception e ) { - logger.error("Exception occurred", e); + if (e instanceof org.apache.catalina.connector.ClientAbortException) { + LOG.info("{}: Client unexpectedly closed connection while loading {} ({})", request.getRemoteAddr(), Util.getURLForRequest(request), e.getCause().toString()); + } else { + LOG.error("{}: An exception occurred while loading {}", request.getRemoteAddr(), Util.getURLForRequest(request), e); + } return null; } diff --git a/airsonic-main/src/main/java/org/airsonic/player/util/Util.java b/airsonic-main/src/main/java/org/airsonic/player/util/Util.java index 294d3fb4..b5c07da9 100644 --- a/airsonic-main/src/main/java/org/airsonic/player/util/Util.java +++ b/airsonic-main/src/main/java/org/airsonic/player/util/Util.java @@ -25,6 +25,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import javax.servlet.ServletResponse; +import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import java.util.ArrayList; @@ -117,4 +118,18 @@ public final class Util { return ""; } } + + /** + * Return a complete URL for the given HTTP request, + * including the query string. + * + * @param request An HTTP request instance + * @return The associated URL + */ + public static String getURLForRequest(HttpServletRequest request) { + String url = request.getRequestURL().toString(); + String queryString = request.getQueryString(); + if (queryString != null && queryString.length() > 0) url += "?" + queryString; + return url; + } } From ec96b9711debdd6c52f7dbb21556ff0004eea73f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois-Xavier=20Thomas?= Date: Sun, 10 Mar 2019 21:40:41 +0100 Subject: [PATCH 2/4] Show more informative messages while streaming When streaming, log messages now show the URL and IP of the originating request, so that it's easier to determine what client is listening to something on the server. --- .../java/org/airsonic/player/controller/StreamController.java | 4 ++-- .../java/org/airsonic/player/io/PlayQueueInputStream.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/airsonic-main/src/main/java/org/airsonic/player/controller/StreamController.java b/airsonic-main/src/main/java/org/airsonic/player/controller/StreamController.java index fc97b95c..f804dd78 100644 --- a/airsonic-main/src/main/java/org/airsonic/player/controller/StreamController.java +++ b/airsonic-main/src/main/java/org/airsonic/player/controller/StreamController.java @@ -108,7 +108,7 @@ public class StreamController { playQueue.addFiles(false, playlistService.getFilesInPlaylist(playlistId)); player.setPlayQueue(playQueue); Util.setContentLength(response, playQueue.length()); - LOG.info("Incoming Podcast request for playlist " + playlistId); + LOG.info("{}: Incoming Podcast request for playlist {}", request.getRemoteAddr(), playlistId); } response.setHeader("Access-Control-Allow-Origin", "*"); @@ -165,7 +165,7 @@ public class StreamController { range = getRange(request, file); if (settingsService.isEnableSeek() && range != null && !file.isVideo()) { - LOG.info("Got HTTP range: " + range); + LOG.info("{}: Got HTTP range: {}", request.getRemoteAddr(), range); response.setStatus(HttpServletResponse.SC_PARTIAL_CONTENT); Util.setContentLength(response, range.isClosed() ? range.size() : fileLength - range.getFirstBytePos()); long lastBytePos = range.getLastBytePos() != null ? range.getLastBytePos() : fileLength - 1; diff --git a/airsonic-main/src/main/java/org/airsonic/player/io/PlayQueueInputStream.java b/airsonic-main/src/main/java/org/airsonic/player/io/PlayQueueInputStream.java index b48f1e14..af799c62 100644 --- a/airsonic-main/src/main/java/org/airsonic/player/io/PlayQueueInputStream.java +++ b/airsonic-main/src/main/java/org/airsonic/player/io/PlayQueueInputStream.java @@ -117,7 +117,7 @@ public class PlayQueueInputStream extends InputStream { close(); } else if (!file.equals(currentFile)) { close(); - LOG.info(player.getUsername() + " listening to \"" + FileUtil.getShortPath(file.getFile()) + "\""); + LOG.info("{}: {} listening to {}", player.getIpAddress(), player.getUsername(), FileUtil.getShortPath(file.getFile())); mediaFileService.incrementPlayCount(file); // Don't scrobble REST players (except Sonos) From 51b738053f00bc9239f5e34275a26405d0feac2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois-Xavier=20Thomas?= Date: Tue, 2 Apr 2019 20:29:58 +0200 Subject: [PATCH 3/4] Make it work even if Tomcat-specific exceptions are not available When Tomcat is not available (for example, when using Jetty), the ClientAbortException is not available either, causing an error when starting the server. This commit fixes that, and instead catches that exception (or its Jetty equivalent) via reflection. --- .../player/controller/StreamController.java | 20 +++++++++++++++---- .../spring/LoggingExceptionResolver.java | 14 ++++++++++--- .../java/org/airsonic/player/util/Util.java | 12 +++++++++++ 3 files changed, 39 insertions(+), 7 deletions(-) diff --git a/airsonic-main/src/main/java/org/airsonic/player/controller/StreamController.java b/airsonic-main/src/main/java/org/airsonic/player/controller/StreamController.java index f804dd78..83f0e6c7 100644 --- a/airsonic-main/src/main/java/org/airsonic/player/controller/StreamController.java +++ b/airsonic-main/src/main/java/org/airsonic/player/controller/StreamController.java @@ -29,7 +29,6 @@ import org.airsonic.player.service.sonos.SonosHelper; import org.airsonic.player.util.HttpRange; import org.airsonic.player.util.StringUtil; import org.airsonic.player.util.Util; -import org.apache.catalina.connector.ClientAbortException; import org.apache.commons.io.IOUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -249,9 +248,22 @@ public class StreamController { } } } - } catch (ClientAbortException e) { - LOG.info("{}: Client unexpectedly closed connection while loading {} ({})", request.getRemoteAddr(), Util.getURLForRequest(request), e.getCause().toString()); - return; + } catch (IOException e) { + + // This happens often and outside of the control of the server, so + // we catch Tomcat/Jetty "connection aborted by client" exceptions + // and display a short error message. + boolean shouldCatch = false; + shouldCatch |= Util.isInstanceOfClassName(e, "org.apache.catalina.connector.ClientAbortException"); + shouldCatch |= Util.isInstanceOfClassName(e, "org.eclipse.jetty.io.EofException"); + if (shouldCatch) { + LOG.info("{}: Client unexpectedly closed connection while loading {} ({})", request.getRemoteAddr(), Util.getURLForRequest(request), e.getCause().toString()); + return; + } + + // Rethrow the exception in all other cases + throw e; + } finally { if (status != null) { securityService.updateUserByteCounts(user, status.getBytesTransfered(), 0L, 0L); diff --git a/airsonic-main/src/main/java/org/airsonic/player/spring/LoggingExceptionResolver.java b/airsonic-main/src/main/java/org/airsonic/player/spring/LoggingExceptionResolver.java index 6ee5b55a..0dcb193d 100644 --- a/airsonic-main/src/main/java/org/airsonic/player/spring/LoggingExceptionResolver.java +++ b/airsonic-main/src/main/java/org/airsonic/player/spring/LoggingExceptionResolver.java @@ -18,11 +18,19 @@ public class LoggingExceptionResolver implements HandlerExceptionResolver, Order public ModelAndView resolveException( HttpServletRequest request, HttpServletResponse response, Object o, Exception e ) { - if (e instanceof org.apache.catalina.connector.ClientAbortException) { + // This happens often and outside of the control of the server, so + // we catch Tomcat/Jetty "connection aborted by client" exceptions + // and display a short error message. + boolean shouldCatch = false; + shouldCatch |= Util.isInstanceOfClassName(e, "org.apache.catalina.connector.ClientAbortException"); + shouldCatch |= Util.isInstanceOfClassName(e, "org.eclipse.jetty.io.EofException"); + if (shouldCatch) { LOG.info("{}: Client unexpectedly closed connection while loading {} ({})", request.getRemoteAddr(), Util.getURLForRequest(request), e.getCause().toString()); - } else { - LOG.error("{}: An exception occurred while loading {}", request.getRemoteAddr(), Util.getURLForRequest(request), e); + return null; } + + // Display a full stack trace in all other cases + LOG.error("{}: An exception occurred while loading {}", request.getRemoteAddr(), Util.getURLForRequest(request), e); return null; } diff --git a/airsonic-main/src/main/java/org/airsonic/player/util/Util.java b/airsonic-main/src/main/java/org/airsonic/player/util/Util.java index b5c07da9..0a758f03 100644 --- a/airsonic-main/src/main/java/org/airsonic/player/util/Util.java +++ b/airsonic-main/src/main/java/org/airsonic/player/util/Util.java @@ -132,4 +132,16 @@ public final class Util { if (queryString != null && queryString.length() > 0) url += "?" + queryString; return url; } + + /** + * Return true if the given object is an instance of the class name in argument. + * If the class doesn't exist, returns false. + */ + public static boolean isInstanceOfClassName(Object o, String className) { + try { + return Class.forName(className).isInstance(o); + } catch (ClassNotFoundException e) { + return false; + } + } } From 3f4a49c95a086c846e2dd35388020d13032bfceb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois-Xavier=20Thomas?= Date: Tue, 2 Apr 2019 21:42:06 +0200 Subject: [PATCH 4/4] Fix dependency error with org.eclipse.jetty.jetty This is only used by reflection, and should be provided by the servlet container (Tomcat or Jetty). --- airsonic-main/pom.xml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/airsonic-main/pom.xml b/airsonic-main/pom.xml index 408e9b0f..66889d60 100755 --- a/airsonic-main/pom.xml +++ b/airsonic-main/pom.xml @@ -492,6 +492,13 @@ provided + + + org.eclipse.jetty + jetty-io + provided + + com.fasterxml.jackson.core jackson-core