[exceptions] Add eof/timeout handling in main exception handler path (#…

…1845)
javalin · Apr 3, 2023 · dbbebde · dbbebde
1 parent 6473347
commit dbbebde
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 6 deletions.
diff --git a/javalin/src/main/java/io/javalin/http/servlet/ExceptionMapper.kt b/javalin/src/main/java/io/javalin/http/servlet/ExceptionMapper.kt
@@ -27,6 +27,7 @@ class ExceptionMapper(val cfg: JavalinConfig) {
             return handle(ctx, t.cause as Exception)
         }
         when {
+            JettyUtil.isSomewhatExpectedException(t) -> JettyUtil.logDebugAndSetError(t, ctx.res())
             t is Exception && HttpResponseExceptionMapper.canHandle(t) && noUserHandler(t) -> HttpResponseExceptionMapper.handle(t as HttpResponseException, ctx)
             t is Exception -> Util.findByClass(handlers, t.javaClass)?.handle(t, ctx) ?: uncaughtException(ctx, t)
             else -> handleUnexpectedThrowable(ctx.res(), t)
@@ -39,13 +40,11 @@ class ExceptionMapper(val cfg: JavalinConfig) {
     }
 
     internal fun handleUnexpectedThrowable(res: HttpServletResponse, throwable: Throwable): Nothing? {
-        val unwrapped = (throwable as? CompletionException)?.cause ?: throwable
-        if (JettyUtil.isClientAbortException(unwrapped) || JettyUtil.isJettyTimeoutException(unwrapped)) {
-            JavalinLogger.debug("Client aborted or timed out", throwable)
-            return null // jetty aborts and timeouts happen when clients disconnect, they are not actually unexpected
-        }
         res.status = HttpStatus.INTERNAL_SERVER_ERROR.code
-        JavalinLogger.error("Exception occurred while servicing http-request", throwable)
+        when (JettyUtil.isSomewhatExpectedException(throwable)) {
+            true -> JettyUtil.logDebugAndSetError(throwable, res)
+            false -> JavalinLogger.error("Exception occurred while servicing http-request", throwable)
+        }
         return null
     }
 

diff --git a/javalin/src/main/java/io/javalin/jetty/JettyUtil.kt b/javalin/src/main/java/io/javalin/jetty/JettyUtil.kt
@@ -1,11 +1,14 @@
 package io.javalin.jetty
 
+import io.javalin.http.HttpStatus
 import io.javalin.util.ConcurrencyUtil
 import io.javalin.util.JavalinLogger
+import jakarta.servlet.http.HttpServletResponse
 import org.eclipse.jetty.server.LowResourceMonitor
 import org.eclipse.jetty.server.Server
 import org.eclipse.jetty.server.handler.StatisticsHandler
 import java.io.IOException
+import java.util.concurrent.CompletionException
 import java.util.concurrent.TimeoutException
 
 object JettyUtil {
@@ -36,4 +39,13 @@ object JettyUtil {
     // This is rare, but intended (see issues #163 and #1277)
     fun isJettyTimeoutException(t: Throwable) = t is IOException && t.cause is TimeoutException
 
+    fun isSomewhatExpectedException(t: Throwable): Boolean {
+        val unwrapped = (t as? CompletionException)?.cause ?: t
+        return isClientAbortException(unwrapped) || isJettyTimeoutException(unwrapped)
+    }
+    fun logDebugAndSetError(t: Throwable, res: HttpServletResponse) {
+        JavalinLogger.debug("Client aborted or timed out", t)
+        res.status = HttpStatus.INTERNAL_SERVER_ERROR.code
+    }
+
 }
diff --git a/javalin/src/test/java/io/javalin/TestExceptionMapper.kt b/javalin/src/test/java/io/javalin/TestExceptionMapper.kt
@@ -18,6 +18,7 @@ import io.javalin.testing.TestUtil
 import io.javalin.testing.TypedException
 import io.javalin.testing.httpCode
 import org.assertj.core.api.Assertions.assertThat
+import org.eclipse.jetty.io.EofException
 import org.junit.jupiter.api.Test
 import kotlin.reflect.full.allSuperclasses
 
@@ -103,4 +104,9 @@ class TestExceptionMapper {
         assertThat(http.jsonGet("/").body).contains("""MY MESSAGE WITH \nNEWLINES\n""")
     }
 
+    @Test
+    fun `jetty eof exceptions are caught and handled as errors`() = TestUtil.test { app, http ->
+        app.get("/") { throw EofException() }
+        assertThat(http.get("/").httpCode()).isEqualTo(INTERNAL_SERVER_ERROR)
+    }
 }