Skip to content

Commit

Permalink
[ktor] Put back the workaround for KTOR-6883 but for Ktor WASM/JS tes…
Browse files Browse the repository at this point in the history
…ts this time (artificial delay)"

This reverts commit bb60044
  • Loading branch information
joffrey-bion committed Jan 26, 2025
1 parent 5bdc2ed commit 080550f
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@ import io.ktor.client.engine.*
import io.ktor.client.plugins.websocket.*
import org.hildan.krossbow.websocket.*
import org.hildan.krossbow.websocket.test.*
import kotlin.time.Duration

abstract class KtorClientTestSuite(
supportsStatusCodes: Boolean,
shouldTestNegotiatedSubprotocol: Boolean = true,
) : WebSocketClientTestSuite(supportsStatusCodes, shouldTestNegotiatedSubprotocol) {
headersTestDelay: Duration? = null,
) : WebSocketClientTestSuite(supportsStatusCodes, shouldTestNegotiatedSubprotocol, headersTestDelay) {

override fun provideClient(): WebSocketClient = KtorWebSocketClient(
HttpClient(provideEngine()) { install(WebSockets) },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ import io.ktor.client.*
import io.ktor.client.plugins.websocket.*
import org.hildan.krossbow.websocket.*
import org.hildan.krossbow.websocket.test.*
import kotlin.test.Test
import kotlin.time.Duration.Companion.milliseconds

// WinHttp: error is too generic and doesn't differ per status code
// JS browser: cannot support status codes for security reasons
// JS / WASM browser: cannot support status codes for security reasons
// JS node: supports status codes since Kotlin 2.0
// Other: currently the other platforms use the CIO engine because of classpath order, and CIO supports status codes
private val Platform.supportsStatusCodes: Boolean
Expand All @@ -21,6 +21,8 @@ class KtorMppWebSocketClientTest : WebSocketClientTestSuite(
// Just to be sure we don't attempt to test this with the Java or JS engines
// See https://youtrack.jetbrains.com/issue/KTOR-6970
shouldTestNegotiatedSubprotocol = false,
// workaround for https://youtrack.jetbrains.com/issue/KTOR-6883 (NOT fixed for WASM)
headersTestDelay = 400.milliseconds.takeIf { currentPlatform() == Platform.WasmJs.NodeJs },
) {
override fun provideClient(): WebSocketClient = KtorWebSocketClient(
HttpClient {
Expand All @@ -32,7 +34,4 @@ class KtorMppWebSocketClientTest : WebSocketClientTestSuite(
install(WebSockets)
},
)

@Test
fun ets() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,15 @@ package org.hildan.krossbow.websocket.ktor
import io.ktor.client.engine.*
import io.ktor.client.engine.js.*
import org.hildan.krossbow.websocket.test.*
import kotlin.time.Duration.Companion.milliseconds

class KtorJsWebSocketClientTest : KtorClientTestSuite(
// JS browser: cannot support status codes for security reasons
supportsStatusCodes = currentJsPlatform() !is Platform.Js.Browser,
supportsStatusCodes = currentPlatform() !in setOf(Platform.Js.Browser, Platform.WasmJs.Browser),
// See https://youtrack.jetbrains.com/issue/KTOR-6970
shouldTestNegotiatedSubprotocol = false,
// workaround for https://youtrack.jetbrains.com/issue/KTOR-6883 (NOT fixed for WASM)
headersTestDelay = 400.milliseconds.takeIf { currentPlatform() == Platform.WasmJs.NodeJs },
) {
override fun provideEngine(): HttpClientEngineFactory<*> = Js
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import kotlin.time.Duration.Companion.seconds
abstract class WebSocketClientTestSuite(
private val supportsStatusCodes: Boolean = true,
private val shouldTestNegotiatedSubprotocol: Boolean = true,
private val headersTestDelay: Duration? = null,
) {
abstract fun provideClient(): WebSocketClient

Expand Down Expand Up @@ -191,8 +192,14 @@ abstract class WebSocketClientTestSuite(

@Test
fun testHandshakeSubprotocolHeader_noProtocol() = runTestRealTime {
// workaround for https://youtrack.jetbrains.com/issue/KTOR-6883
val extraParams = if (headersTestDelay != null) mapOf("scheduleDelay" to headersTestDelay.toString()) else emptyMap()
val connection = wsClient.connect(
url = testUrl(path = "/sendHandshakeHeaders", testCaseName = "testHandshakeSubprotocolHeader_noProtocol"),
url = testUrl(
path = "/sendHandshakeHeaders",
testCaseName = "testHandshakeSubprotocolHeader_noProtocol",
otherParams = extraParams,
),
protocols = emptyList(),
)
try {
Expand All @@ -207,8 +214,14 @@ abstract class WebSocketClientTestSuite(

@Test
fun testHandshakeSubprotocolHeader_singleProtocol() = runTestRealTime {
// workaround for https://youtrack.jetbrains.com/issue/KTOR-6883
val extraParams = if (headersTestDelay != null) mapOf("scheduleDelay" to headersTestDelay.toString()) else emptyMap()
val connection = wsClient.connect(
url = testUrl(path = "/sendHandshakeHeaders", testCaseName = "testHandshakeSubprotocolHeader_singleProtocol"),
url = testUrl(
path = "/sendHandshakeHeaders",
testCaseName = "testHandshakeSubprotocolHeader_singleProtocol",
otherParams = extraParams,
),
protocols = listOf("v12.stomp"),
)
try {
Expand All @@ -225,8 +238,14 @@ abstract class WebSocketClientTestSuite(

@Test
fun testHandshakeSubprotocolHeader_multipleProtocols() = runTestRealTime {
// workaround for https://youtrack.jetbrains.com/issue/KTOR-6883
val extraParams = if (headersTestDelay != null) mapOf("scheduleDelay" to headersTestDelay.toString()) else emptyMap()
val connection = wsClient.connect(
url = testUrl(path = "/sendHandshakeHeaders", testCaseName = "testHandshakeSubprotocolHeader_singleProtocol"),
url = testUrl(
path = "/sendHandshakeHeaders",
testCaseName = "testHandshakeSubprotocolHeader_multipleProtocols",
otherParams = extraParams,
),
protocols = listOf("unknown-protocol", "v12.stomp", "v11.stomp", "v10.stomp"),
)
try {
Expand All @@ -249,14 +268,20 @@ abstract class WebSocketClientTestSuite(
fun testHandshakeCustomHeaders() = runTestRealTime {
if (wsClient.supportsCustomHeaders) {
println("Connecting with agent $agent to ${testServerConfig.wsUrl}/sendHandshakeHeaders")
// workaround for https://youtrack.jetbrains.com/issue/KTOR-6883
val extraParams = if (headersTestDelay != null) mapOf("scheduleDelay" to headersTestDelay.toString()) else emptyMap()
val connection = wsClient.connect(
url = testUrl(path = "/sendHandshakeHeaders", testCaseName = "testHandshakeCustomHeaders"),
url = testUrl(
path = "/sendHandshakeHeaders",
testCaseName = "testHandshakeCustomHeaders",
otherParams = extraParams,
),
headers = mapOf("My-Header-1" to "my-value-1", "My-Header-2" to "my-value-2"),
)
println("Connected with agent $agent to ${testServerConfig.wsUrl}/sendHandshakeHeaders")
try {
// for some reason, this can be pretty long with the Ktor/JS client in nodeJS tests on macOS
val echoedHeadersFrame = connection.expectTextFrame("header info frame")
val echoedHeadersFrame = connection.expectTextFrame("header info frame", 50.seconds)
val headers = echoedHeadersFrame.text.lines()
assertContains(headers, "My-Header-1=my-value-1")
assertContains(headers, "My-Header-2=my-value-2")
Expand Down Expand Up @@ -318,7 +343,7 @@ private fun runTestRealTime(
testBody: suspend CoroutineScope.() -> Unit,
) = runTest(timeout = timeout) {
// Switches to a regular dispatcher to avoid the virtual time from runTest.
// We also use limitedParallelism to keep things deterministic
// We also use limitedParallelism to keep things deterministic
withContext(Dispatchers.Default.limitedParallelism(1) + context) {
testBody()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import org.java_websocket.protocols.*
import org.java_websocket.server.*
import java.net.*
import java.nio.*
import kotlin.time.Duration

private val Draft6455Default = Draft_6455()
private val Draft6455WithStomp12 = Draft_6455(emptyList(), listOf(Protocol("v12.stomp")))
Expand All @@ -16,20 +17,34 @@ internal class EchoWebSocketServer(port: Int = 0) : WebSocketServer(
InetSocketAddress(port),
listOf(Draft6455WithStomp12, Draft6455Default),
) {
private val delayedHeadersScope = CoroutineScope(SupervisorJob() + Dispatchers.Default)

override fun onStart() {
}

override fun onOpen(conn: WebSocket, handshake: ClientHandshake) {
val uri = URI.create(handshake.resourceDescriptor)
if (uri.path == "/sendHandshakeHeaders") {
conn.sendMessageWithHeaders(handshake)
val queryParams = uri.queryAsMap()
val scheduleDelay = queryParams["scheduleDelay"]?.let(Duration::parse)
conn.sendMessageWithHeaders(handshake, scheduleDelay)
}
}

private fun WebSocket.sendMessageWithHeaders(handshake: ClientHandshake) {
private fun WebSocket.sendMessageWithHeaders(handshake: ClientHandshake, scheduleDelay: Duration? = null) {
val headerNames = handshake.iterateHttpFields().asSequence().toList()
val headersData = headerNames.joinToString("\n") { "$it=${handshake.getFieldValue(it)}" }
send(headersData)
if (scheduleDelay != null) {
// necessary due to https://youtrack.jetbrains.com/issue/KTOR-6883
println("Scheduling message with headers in $scheduleDelay")
delayedHeadersScope.launch {
delay(scheduleDelay)
send(headersData)
println("Headers frame sent!")
}
} else {
send(headersData)
}
}

override fun onMessage(conn: WebSocket, message: String?) {
Expand Down Expand Up @@ -60,3 +75,7 @@ internal class EchoWebSocketServer(port: Int = 0) : WebSocketServer(
port
}
}

private fun URI.queryAsMap() = query.split("&")
.map { it.split("=") }
.associate { it[0] to it[1] }

0 comments on commit 080550f

Please sign in to comment.