Skip to content

Commit

Permalink
KTOR-4457 Ignore request timeout for WebSocket requests
Browse files Browse the repository at this point in the history
  • Loading branch information
e5l committed Jun 20, 2022
1 parent 3aded41 commit 32dd431
Show file tree
Hide file tree
Showing 6 changed files with 80 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import io.ktor.network.selector.*
import io.ktor.util.*
import io.ktor.util.collections.*
import io.ktor.util.network.*
import io.ktor.utils.io.*
import kotlinx.coroutines.*
import kotlinx.coroutines.channels.*
import kotlin.coroutines.*
Expand All @@ -24,7 +23,7 @@ internal class CIOEngine(
override val config: CIOEngineConfig
) : HttpClientEngineBase("ktor-cio") {

override val dispatcher by lazy {
override val dispatcher: CoroutineDispatcher by lazy {
Dispatchers.clientDispatcher(config.threadsCount, "ktor-cio-dispatcher")
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import io.ktor.client.network.sockets.*
import io.ktor.client.plugins.*
import io.ktor.client.request.*
import io.ktor.client.utils.*
import io.ktor.http.*
import io.ktor.network.sockets.*
import io.ktor.network.tls.*
import io.ktor.util.*
Expand Down Expand Up @@ -114,7 +115,7 @@ internal class Endpoint(
}
}

val timeout = getRequestTimeout(request.getCapabilityOrNull(HttpTimeout), config)
val timeout = getRequestTimeout(request, config)
setupTimeout(callContext, request, timeout)

val requestTime = GMTDate()
Expand Down Expand Up @@ -256,7 +257,18 @@ public class FailToConnectException : Exception("Connect timed out or retry atte

internal expect fun Throwable.mapToKtor(request: HttpRequestData): Throwable

@OptIn(InternalAPI::class)
internal fun getRequestTimeout(
requestConfig: HttpTimeout.HttpTimeoutCapabilityConfiguration?,
request: HttpRequestData,
engineConfig: CIOEngineConfig
): Long = requestConfig?.requestTimeoutMillis ?: engineConfig.requestTimeout
): Long {
/**
* The request timeout is handled by the plugin and disabled for the WebSockets.
*/
val isWebSocket = request.url.protocol.isWebsocket()
if (request.getCapabilityOrNull(HttpTimeout) != null || isWebSocket || request.isUpgradeRequest()) {
return HttpTimeout.INFINITE_TIMEOUT_MS
}

return engineConfig.requestTimeout
}
37 changes: 37 additions & 0 deletions ktor-client/ktor-client-cio/jvmAndNix/test/CIOEngineTest.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* Copyright 2014-2022 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license.
*/
import io.ktor.client.*
import io.ktor.client.engine.cio.*
import io.ktor.client.plugins.websocket.*
import io.ktor.client.tests.utils.*
import io.ktor.websocket.*
import kotlinx.coroutines.*
import kotlin.test.*

class CIOEngineTest {

@Test
fun testRequestTimeoutIgnoredWithWebSocket(): Unit = runBlocking {
val client = HttpClient(CIO) {
engine {
requestTimeout = 10
}

install(WebSockets)
}

var received = false
client.ws("$TEST_WEBSOCKET_SERVER/websockets/echo") {
delay(20)

send(Frame.Text("Hello"))

val response = incoming.receive() as Frame.Text
received = true
assertEquals("Hello", response.readText())
}

assertTrue(received)
}
}
20 changes: 0 additions & 20 deletions ktor-client/ktor-client-cio/jvmAndNix/test/EndpointTest.kt

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import io.ktor.client.engine.*
import io.ktor.client.network.sockets.*
import io.ktor.client.request.*
import io.ktor.client.utils.*
import io.ktor.http.*
import io.ktor.util.*
import io.ktor.utils.io.errors.*
import kotlinx.coroutines.*
Expand Down Expand Up @@ -135,8 +136,12 @@ public class HttpTimeout private constructor(
override fun prepare(block: HttpTimeoutCapabilityConfiguration.() -> Unit): HttpTimeout =
HttpTimeoutCapabilityConfiguration().apply(block).build()

@OptIn(InternalAPI::class)
override fun install(plugin: HttpTimeout, scope: HttpClient) {
scope.requestPipeline.intercept(HttpRequestPipeline.Before) {
val isWebSocket = context.url.protocol.isWebsocket()
if (isWebSocket || context.body is ClientUpgradeContent) return@intercept

var configuration = context.getCapabilityOrNull(HttpTimeout)
if (configuration == null && plugin.hasNotNullTimeouts()) {
configuration = HttpTimeoutCapabilityConfiguration()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import io.ktor.test.dispatcher.*
import io.ktor.util.reflect.*
import io.ktor.utils.io.charsets.*
import io.ktor.websocket.*
import kotlinx.coroutines.*
import kotlin.test.*

private const val TEST_SIZE: Int = 100
Expand Down Expand Up @@ -182,4 +183,25 @@ class WebSocketTest : ClientLoader() {
}
}
}

@Test
fun testRequestTimeoutIsNotApplied() = clientTests(listOf("Android", "Apache", "Curl")) {
config {
install(WebSockets)

install(HttpTimeout) {
requestTimeoutMillis = 10
}
}

test { client ->
client.webSocket("$TEST_WEBSOCKET_SERVER/websockets/echo") {
delay(20)

send("test")
val result = incoming.receive() as Frame.Text
assertEquals("test", result.readText())
}
}
}
}

0 comments on commit 32dd431

Please sign in to comment.