Skip to content

Commit

Permalink
KTOR-7139 Fux exception thrown in onCallRespond makes the client wait…
Browse files Browse the repository at this point in the history
… for response indefinitely
  • Loading branch information
e5l committed Jan 16, 2025
1 parent 01abaa9 commit e22a125
Show file tree
Hide file tree
Showing 5 changed files with 108 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ public abstract class BaseApplicationEngine(
val pipeline = pipeline

BaseApplicationResponse.setupSendPipeline(pipeline.sendPipeline)
BaseApplicationResponse.setupFallbackResponse(pipeline)

monitor.subscribe(ApplicationStarting) {
if (!info.isFirstLoading) {
Expand All @@ -58,6 +59,7 @@ public abstract class BaseApplicationEngine(
it.installDefaultInterceptors()
it.installDefaultTransformationChecker()
}

monitor.subscribe(ApplicationStarted) {
val finishedAt = getTimeMillis()
val elapsedTimeInSeconds = (finishedAt - info.initializedStartAt) / 1_000.0
Expand Down Expand Up @@ -110,7 +112,7 @@ private fun Application.installDefaultTransformationChecker() {
intercept(ApplicationCallPipeline.Plugins) {
try {
proceed()
} catch (e: CannotTransformContentToTypeException) {
} catch (_: CannotTransformContentToTypeException) {
call.respond(HttpStatusCode.UnsupportedMediaType)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,14 @@ import io.ktor.util.*
import io.ktor.util.cio.*
import io.ktor.util.internal.*
import io.ktor.utils.io.*
import kotlinx.coroutines.*
import kotlinx.coroutines.CopyableThrowable
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.withContext

private val ERROR_CONTENT = object : OutgoingContent.NoContent() {
override val status: HttpStatusCode = HttpStatusCode.InternalServerError
}

public abstract class BaseApplicationResponse(
final override val call: PipelineCall
Expand Down Expand Up @@ -80,10 +87,12 @@ public abstract class BaseApplicationResponse(
// TODO: What should we do if TransferEncoding was set and length is present?
headers.append(HttpHeaders.ContentLength, contentLength.toStringFast(), safeOnly = false)
}

!transferEncodingSet -> {
when (content) {
is OutgoingContent.ProtocolUpgrade -> {
}

is OutgoingContent.NoContent -> headers.append(HttpHeaders.ContentLength, "0", safeOnly = false)
else -> headers.append(HttpHeaders.TransferEncoding, "chunked", safeOnly = false)
}
Expand Down Expand Up @@ -319,5 +328,25 @@ public abstract class BaseApplicationResponse(
response.respondOutgoingContent(body)
}
}

public fun setupFallbackResponse(application: EnginePipeline) {
val inDevMode = application.developmentMode
application.intercept(ApplicationCallPipeline.Setup) {
try {
proceed()
} catch (cause: Throwable) {
if (call.isHandled) return@intercept

val response = call.response as? BaseApplicationResponse
?: call.attributes[EngineResponseAttributeKey]

if (inDevMode) {
response.respondOutgoingContent(ExceptionPageContent(call, cause))
} else {
response.respondNoContent(ERROR_CONTENT)
}
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/*
* Copyright 2014-2025 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license.
*/

package io.ktor.server.engine.internal

import io.ktor.http.content.OutgoingContent
import io.ktor.server.application.ApplicationCall
import io.ktor.server.plugins.origin
import io.ktor.server.request.PipelineRequest
import io.ktor.server.request.httpMethod
import io.ktor.server.request.path
import io.ktor.utils.io.ByteReadChannel

internal class ExceptionPageContent(call: ApplicationCall, cause: Throwable) : OutgoingContent.ReadChannelContent() {


private val responsePage: String = buildString {
val request = call.request
append("<html><body><h1>Internal Server Error</h1><h2>Request Information:</h2><pre>")
append("Method: ${request.httpMethod}\n")
append("Path: ${request.path()}\n")
append("Parameters: ${request.rawQueryParameters}\n")
append("From origin: ${request.origin}\n")
append("</pre><h2>Stack Trace:</h2><pre>")

val stackTrace = cause.stackTraceToString().lines()
stackTrace.forEach { element ->
append("<span style=\"color:blue;\">$element</span><br>")
}
var currentCause = cause.cause
while (currentCause != null) {
append("<br>Caused by:<br>")
val causeStack = currentCause.stackTraceToString().lines()
causeStack.forEach { element ->
append("<span style=\"color:green;\">$element</span><br>")
}
currentCause = currentCause.cause
}
append("</pre></body></html>")
}


override fun readFrom(): ByteReadChannel = ByteReadChannel(responsePage)
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ internal class NettyApplicationCallHandler(
else ->
try {
enginePipeline.execute(call)
} catch (error: Exception) {
} catch (error: Throwable) {
handleFailure(call, error)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,18 @@ import io.ktor.utils.io.core.*
import io.ktor.utils.io.jvm.javaio.*
import io.ktor.utils.io.streams.*
import kotlinx.coroutines.*
import kotlinx.coroutines.debug.*
import org.slf4j.*
import java.io.*
import java.net.*
import kotlinx.coroutines.debug.DebugProbes
import org.slf4j.Logger
import org.slf4j.LoggerFactory
import java.io.File
import java.io.IOException
import java.net.HttpURLConnection
import java.net.Proxy
import java.net.URL
import java.util.*
import java.util.concurrent.*
import java.util.concurrent.atomic.*
import java.util.concurrent.atomic.AtomicInteger
import kotlin.test.*
import kotlin.use

abstract class SustainabilityTestSuite<TEngine : ApplicationEngine, TConfiguration : ApplicationEngine.Configuration>(
hostFactory: ApplicationEngineFactory<TEngine, TConfiguration>
Expand Down Expand Up @@ -918,6 +921,26 @@ abstract class SustainabilityTestSuite<TEngine : ApplicationEngine, TConfigurati
assertTrue(failCause != null)
assertIs<IOException>(failCause)
}

@Test
fun `fail in onCallRespond does not freeze request`() = runTest {
createAndStartServer {
application.install(createApplicationPlugin("MyPlugin") {
onCallRespond { call ->
error("oh nooooo")
}
})

get {
call.respondText("hello world")
}
}

withUrl("") {
assertEquals(HttpStatusCode.InternalServerError, status)
println("$this done")
}
}
}

internal inline fun assertFails(block: () -> Unit) {
Expand Down

0 comments on commit e22a125

Please sign in to comment.