How to define connection timeout?

[slava110]

Hi. In my program I'm starting an external application and then I need to wait for webtools to initialize in this application. However, I cannot set DevTools websocket connection timeout for some reason. Well, I can, but it doesn't work. What I've tried:

val client = ChromeDPClient(
        remoteDebugUrl = "http://localhost:8999",
        webSocketClient = Jdk11WebSocketClient {
            connectTimeout(Duration.ofSeconds(20))
        },
        httpClient = HttpClient {
            install(HttpTimeout) {
                connectTimeoutMillis = 20000
            }
        }
    )

In theory it should set connection timeout to 20 seconds but it's still 5 seconds. Is this an issue or am I missing something?

[slava110]

Wrong approach. For some reason I thought that connectTimeout will set timeout between requests while client will retry to execute them again and again. But that's not how it works
Found ktor.io/docs/client-retry and it saved my day

val httpClient = HttpClient {
    // From default client
    install(ContentNegotiation) {
        json(Json { ignoreUnknownKeys = true })
    }
    install(HttpRequestRetry) {
        maxRetries = 3
        constantDelay(1000)
        retryOnExceptionIf { _, error ->
            println("Retrying...")
            error is ConnectException
        }
    }
}

The only problem is that I need to manually add ContentNegotiation from default DevTools HttpClient manually, but it's not a big deal. Sorry for disturbing you

[joffrey-bion]

Hi! Thanks for opening an issue, and don't worry about disturbing. I'm happy to have questions, it means I can improve the docs and/or fix stuff :)

What exactly was timing out then? The initial version() HTTP call within the client.webSocket() call?

I understand that this was not the best UX, especially since any additional config on the Ktor http client requires you to manually rewrite all the initial default config from this library.

I'll try to make things better in this respect. Actually I could add retries by default, because it's probably desirable for most use cases.

[joffrey-bion]

Reopening to track improvements/retry

[slava110]

What exactly was timing out then? The initial version() HTTP call within the client.webSocket() call?

Nope, timeout is something else. And it doesn't help if HttpClient is throwing Connection refused: No further information

I understand that this was not the best UX, especially since any additional config on the Ktor http client requires you to manually rewrite all the initial default config from this library.

That's fine I think. I'm not sure how this could be improved tbh. I can only think about something like

class ChromeDPClient(
    private val remoteDebugUrl: String = "http://localhost:9222",
    private val overrideHostHeader: Boolean = false,
    private val webSocketClient: WebSocketClient = DEFAULT_WEBSOCKET_CLIENT,
    private val httpClient: HttpClient = if (overrideHostHeader) DEFAULT_HTTP_CLIENT_WITH_HOST_OVERRIDE else DEFAULT_HTTP_CLIENT
) {

constructor(
    remoteDebugUrl: String = "http://localhost:9222",
    overrideHostHeader: Boolean = false,
    webSocketClient: WebSocketClient = DEFAULT_WEBSOCKET_CLIENT,
    httpClientConfig: HttpClient.() -> Unit
): this(remoteDebugUrl, overrideHostHeader, webSocketClient, if (overrideHostHeader) DEFAULT_HTTP_CLIENT_WITH_HOST_OVERRIDE else DEFAULT_HTTP_CLIENT) {
    httpClient.httpClientConfig()
}

//...
}

Or maybe DSL instead of constructor. But I don't think it's that important tbh 🤷
About retrying: it isn't something crucial for DevTools library to operate so I don't think that you should retry by default.

[joffrey-bion]

Yes this kind of lambda for extra config is what I had in mind.

Also, would a function like awaitDebugger() solve your problem in a cleaner way than retries on connect? I might add this because your use case seems rather common

[slava110]

Maybe. Retrying is configurable so you'll probably need something like function awaitDebugger(optionalConfig: HttpRequestRetry.Configuration.() = {}) in such case

[joffrey-bion]

I was thinking about hiding the retry completely as an implementation detail, and exposing only a simple function to wait (users can use withTimeout or regular cancellation to stop waiting)

[slava110]

Well, you can, but in some cases people might want to use constant delay instead of exponential (default one). Or have more/less retries

[joffrey-bion]

True, it might be desirable to leave the option to the user to do more or less active polling. As long as there is a default.

[joffrey-bion]

Closing this question, as further improvements will be tracked in #174