jillesvangurp / kt-search

Multi platform kotlin client for Elasticsearch & Opensearch with easily extendable Kotlin DSLs for queries, mappings, bulk, and more.
MIT License
95 stars 23 forks source link

Fix reading REST client response #62

Closed yassenb closed 1 year ago

yassenb commented 1 year ago

When reading raw bytes the response pipeline is circumvented and leads to problems procesing the response. For example when the ContentEncoding plugin is used the response is compressed.

yassenb commented 1 year ago

Opening following https://github.com/jillesvangurp/kt-search/pull/60. This doesn't assume a text response but just fixes reading the bytes correctly, through the ktor client pipeline

yassenb commented 1 year ago

Not sure why the test is failing. It does even after I revert my changes so it should be a problem with the test

jillesvangurp commented 1 year ago

This breaks for responses that don't have a body (head on datastream exists). It throws a null pointer exception on bodyNullable()!!.

Kind of ugly how they did that. Working around it with an even uglier try .. catch.

yassenb commented 1 year ago

@jillesvangurp, should be just .body<ByteArray?>() then, no need for a try-catch. I don't see any !! here: https://github.com/ktorio/ktor/blob/main/ktor-client/ktor-client-core/common/src/io/ktor/client/call/HttpClientCall.kt#L155

jillesvangurp commented 1 year ago

it's bodyAsNullable that does a !!. This is called by body internally. So, a null body triggers an npe. Using ByteArray? does not fix this.

jillesvangurp commented 1 year ago

This is the problem:

    @OptIn(InternalAPI::class)
    public suspend fun body(info: TypeInfo): Any = bodyNullable(info)!!
yassenb commented 1 year ago

The function you point to is not the one being called when you call .body<ByteArray?>(), it's the one in the link I posted:

public suspend inline fun <reified T> HttpResponse.body(): T = call.bodyNullable(typeInfo<T>()) as T

I have verified this by debugging

jillesvangurp commented 1 year ago

You are right. It still fails unfortunately. If you want to try this some more:

val responseBody = response.body<ByteArray?>() ?: ByteArray(0)

Causes the IndexTemplateTest to fail.

Expected 353, actual 0
java.lang.IllegalStateException: Expected 353, actual 0
    at io.ktor.client.plugins.DefaultTransformKt$defaultTransformers$2.invokeSuspend(DefaultTransform.kt:82)
    at io.ktor.client.plugins.DefaultTransformKt$defaultTransformers$2.invoke(DefaultTransform.kt)
    at io.ktor.client.plugins.DefaultTransformKt$defaultTransformers$2.invoke(DefaultTransform.kt)
    at io.ktor.util.pipeline.SuspendFunctionGun.loop(SuspendFunctionGun.kt:120)
    at io.ktor.util.pipeline.SuspendFunctionGun.proceed(SuspendFunctionGun.kt:78)
    at io.ktor.client.HttpClient$4.invokeSuspend(HttpClient.kt:177)
    at io.ktor.client.HttpClient$4.invoke(HttpClient.kt)
    at io.ktor.client.HttpClient$4.invoke(HttpClient.kt)
    at io.ktor.util.pipeline.SuspendFunctionGun.loop(SuspendFunctionGun.kt:120)
    at io.ktor.util.pipeline.SuspendFunctionGun.proceed(SuspendFunctionGun.kt:78)
    at io.ktor.client.plugins.logging.Logging$setupResponseLogging$2.invokeSuspend(Logging.kt:181)
    at io.ktor.client.plugins.logging.Logging$setupResponseLogging$2.invoke(Logging.kt)
    at io.ktor.client.plugins.logging.Logging$setupResponseLogging$2.invoke(Logging.kt)
    at io.ktor.util.pipeline.SuspendFunctionGun.loop(SuspendFunctionGun.kt:120)
    at io.ktor.util.pipeline.SuspendFunctionGun.proceed(SuspendFunctionGun.kt:78)
    at io.ktor.util.pipeline.SuspendFunctionGun.proceedWith(SuspendFunctionGun.kt:88)
    at io.ktor.client.plugins.HttpCallValidator$Companion$install$2.invokeSuspend(HttpCallValidator.kt:138)
    at io.ktor.client.plugins.HttpCallValidator$Companion$install$2.invoke(HttpCallValidator.kt)
    at io.ktor.client.plugins.HttpCallValidator$Companion$install$2.invoke(HttpCallValidator.kt)
    at io.ktor.util.pipeline.SuspendFunctionGun.loop(SuspendFunctionGun.kt:120)
    at io.ktor.util.pipeline.SuspendFunctionGun.proceed(SuspendFunctionGun.kt:78)
    at io.ktor.util.pipeline.SuspendFunctionGun.execute$ktor_utils(SuspendFunctionGun.kt:98)
    at io.ktor.util.pipeline.Pipeline.execute(Pipeline.kt:77)
    at io.ktor.client.call.HttpClientCall.bodyNullable(HttpClientCall.kt:88)
    at com.jillesvangurp.ktsearch.KtorRestClient.doRequest(KtorRestClient.kt:151)
    at com.jillesvangurp.ktsearch.KtorRestClient$doRequest$1.invokeSuspend(KtorRestClient.kt)
    (Coroutine boundary)
    at io.ktor.client.HttpClient$4.invokeSuspend(HttpClient.kt:177)
    at io.ktor.client.plugins.logging.Logging$setupResponseLogging$2.invokeSuspend(Logging.kt:181)
    at io.ktor.client.plugins.HttpCallValidator$Companion$install$2.invokeSuspend(HttpCallValidator.kt:138)
    at io.ktor.client.call.HttpClientCall.bodyNullable(HttpClientCall.kt:88)
    at com.jillesvangurp.ktsearch.KtorRestClient.doRequest(KtorRestClient.kt:151)
    at com.jillesvangurp.ktsearch.Index_templatesKt.exists(index-templates.kt:58)
    at com.jillesvangurp.ktsearch.IndexTemplateTest$shouldCreateDataStream$1.invokeSuspend(IndexTemplateTest.kt:40)
    at com.jillesvangurp.ktsearch.CoTestKt$coRun$1$1.invokeSuspend(coTest.kt:10)
    at com.jillesvangurp.ktsearch.CoTestKt$coRun$1.invokeSuspend(coTest.kt:9)
Caused by: java.lang.IllegalStateException: Expected 353, actual 0
    at io.ktor.client.plugins.DefaultTransformKt$defaultTransformers$2.invokeSuspend(DefaultTransform.kt:82)
    at io.ktor.client.plugins.DefaultTransformKt$defaultTransformers$2.invoke(DefaultTransform.kt)
    at io.ktor.client.plugins.DefaultTransformKt$defaultTransformers$2.invoke(DefaultTransform.kt)
    at io.ktor.util.pipeline.SuspendFunctionGun.loop(SuspendFunctionGun.kt:120)
    at io.ktor.util.pipeline.SuspendFunctionGun.proceed(SuspendFunctionGun.kt:78)
    at io.ktor.client.HttpClient$4.invokeSuspend(HttpClient.kt:177)
    at io.ktor.client.HttpClient$4.invoke(HttpClient.kt)
    at io.ktor.client.HttpClient$4.invoke(HttpClient.kt)
    at io.ktor.util.pipeline.SuspendFunctionGun.loop(SuspendFunctionGun.kt:120)
    at io.ktor.util.pipeline.SuspendFunctionGun.proceed(SuspendFunctionGun.kt:78)
    at io.ktor.client.plugins.logging.Logging$setupResponseLogging$2.invokeSuspend(Logging.kt:181)
    at io.ktor.client.plugins.logging.Logging$setupResponseLogging$2.invoke(Logging.kt)
    at io.ktor.client.plugins.logging.Logging$setupResponseLogging$2.invoke(Logging.kt)
    at io.ktor.util.pipeline.SuspendFunctionGun.loop(SuspendFunctionGun.kt:120)
    at io.ktor.util.pipeline.SuspendFunctionGun.proceed(SuspendFunctionGun.kt:78)
    at io.ktor.util.pipeline.SuspendFunctionGun.proceedWith(SuspendFunctionGun.kt:88)
    at io.ktor.client.plugins.HttpCallValidator$Companion$install$2.invokeSuspend(HttpCallValidator.kt:138)
    at io.ktor.client.plugins.HttpCallValidator$Companion$install$2.invoke(HttpCallValidator.kt)
    at io.ktor.client.plugins.HttpCallValidator$Companion$install$2.invoke(HttpCallValidator.kt)
    at io.ktor.util.pipeline.SuspendFunctionGun.loop(SuspendFunctionGun.kt:120)
    at io.ktor.util.pipeline.SuspendFunctionGun.proceed(SuspendFunctionGun.kt:78)
    at io.ktor.util.pipeline.SuspendFunctionGun.execute$ktor_utils(SuspendFunctionGun.kt:98)
    at io.ktor.util.pipeline.Pipeline.execute(Pipeline.kt:77)
    at io.ktor.client.call.HttpClientCall.bodyNullable(HttpClientCall.kt:88)
    at com.jillesvangurp.ktsearch.KtorRestClient.doRequest(KtorRestClient.kt:151)
    at com.jillesvangurp.ktsearch.KtorRestClient$doRequest$1.invokeSuspend(KtorRestClient.kt)
    at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
    at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:106)
    at kotlinx.coroutines.EventLoopImplBase.processNextEvent(EventLoop.common.kt:284)
    at kotlinx.coroutines.BlockingCoroutine.joinBlocking(Builders.kt:85)
    at kotlinx.coroutines.BuildersKt__BuildersKt.runBlocking(Builders.kt:59)
    at kotlinx.coroutines.BuildersKt.runBlocking(Unknown Source)
    at kotlinx.coroutines.BuildersKt__BuildersKt.runBlocking$default(Builders.kt:38)
    at kotlinx.coroutines.BuildersKt.runBlocking$default(Unknown Source)
    at com.jillesvangurp.ktsearch.CoTestKt.coRun-VtjQ1oo(coTest.kt:8)
    at com.jillesvangurp.ktsearch.CoTestKt.coRun-VtjQ1oo$default(coTest.kt:7)
    at com.jillesvangurp.ktsearch.IndexTemplateTest.shouldCreateDataStream(IndexTemplateTest.kt:10)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:568)
    at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:727)
    at org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60)
    at org.junit.jupiter.engine.execution.InvocationInterceptorChain$ValidatingInvocation.proceed(InvocationInterceptorChain.java:131)
    at org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:156)
    at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestableMethod(TimeoutExtension.java:147)
    at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestMethod(TimeoutExtension.java:86)
    at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(InterceptingExecutableInvoker.java:103)
    at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker.lambda$invoke$0(InterceptingExecutableInvoker.java:93)
    at org.junit.jupiter.engine.execution.InvocationInterceptorChain$InterceptedInvocation.proceed(InvocationInterceptorChain.java:106)
    at org.junit.jupiter.engine.execution.InvocationInterceptorChain.proceed(InvocationInterceptorChain.java:64)
    at org.junit.jupiter.engine.execution.InvocationInterceptorChain.chainAndInvoke(InvocationInterceptorChain.java:45)
    at org.junit.jupiter.engine.execution.InvocationInterceptorChain.invoke(InvocationInterceptorChain.java:37)
    at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker.invoke(InterceptingExecutableInvoker.java:92)
    at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker.invoke(InterceptingExecutableInvoker.java:86)
    at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeTestMethod$7(TestMethodTestDescriptor.java:217)
    at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeTestMethod(TestMethodTestDescriptor.java:213)
    at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:138)
    at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:68)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:151)
    at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
    at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
    at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
    at org.junit.platform.engine.support.hierarchical.ForkJoinPoolHierarchicalTestExecutorService$ExclusiveTask.compute(ForkJoinPoolHierarchicalTestExecutorService.java:202)
    at org.junit.platform.engine.support.hierarchical.ForkJoinPoolHierarchicalTestExecutorService.invokeAll(ForkJoinPoolHierarchicalTestExecutorService.java:146)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155)
    at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
    at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
    at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
    at org.junit.platform.engine.support.hierarchical.ForkJoinPoolHierarchicalTestExecutorService$ExclusiveTask.compute(ForkJoinPoolHierarchicalTestExecutorService.java:202)
    at org.junit.platform.engine.support.hierarchical.ForkJoinPoolHierarchicalTestExecutorService.invokeAll(ForkJoinPoolHierarchicalTestExecutorService.java:146)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155)
    at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
    at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
    at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
    at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
    at org.junit.platform.engine.support.hierarchical.ForkJoinPoolHierarchicalTestExecutorService$ExclusiveTask.compute(ForkJoinPoolHierarchicalTestExecutorService.java:202)
    at java.base/java.util.concurrent.RecursiveAction.exec(RecursiveAction.java:194)
    at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:373)
    at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1182)
    at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1655)
    at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1622)
    at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:165)

It's basically trying to read a response that isn't there. If you find a workaround, I'll integrate it of course. But thanks for this.

yassenb commented 1 year ago

How are you running the tests locally? Even without the ? at the end of ByteArray I only got a documentation test fail when running gradle check and I saw the same results in the CI run: https://github.com/jillesvangurp/kt-search/actions/runs/4394101456/jobs/7709782393. Even more strange, when I revert to .readBytes() I still get the same failure which leads me to believe the tests leave some state behind, probably some docker volume but not sure about the whole test infrastructure. In any case I can't reproduce a related failure when using .body(), I can open another PR to confirm this and we can both look at the CI run

jillesvangurp commented 1 year ago

OK, works fine on my mac. You should be able to do a docker-compose -f docker-compose-es-7.yml up -d and then run the tests. Gradle should be doing that for you.