[client] Throw error on non-successful HTTP responses (#889)

Currently GraphQLWebClient throws NoSuchElementException for non-200 with
empty body response (i.e 401 Unauthorized) instead of propagating the error.

This change makes use of WebClient.retrieve to throw exception on
non-sucessful HTTP responses to keep inline with KtorClient.

GraphQL always uses HTTP status 200 so other status like 4xx or 5xx should
be treated as failure and it does not need to unmarshal response body.

Author: nqv

clients/graphql-kotlin-ktor-client/src/test/kotlin/com/expediagroup/graphql/client/GraphQLKtorClientTest.kt

@@ -3,18 +3,21 @@ package com.expediagroup.graphql.client
 import com.expediagroup.graphql.types.GraphQLError
 import com.expediagroup.graphql.types.GraphQLResponse
 import com.expediagroup.graphql.types.SourceLocation
+import com.fasterxml.jackson.databind.exc.MismatchedInputException
 import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper
 import com.github.tomakehurst.wiremock.WireMockServer
 import com.github.tomakehurst.wiremock.client.MappingBuilder
 import com.github.tomakehurst.wiremock.client.WireMock
 import com.github.tomakehurst.wiremock.core.WireMockConfiguration
 import com.github.tomakehurst.wiremock.matching.EqualToPattern
 import io.ktor.client.engine.okhttp.OkHttp
+import io.ktor.client.features.ServerResponseException
 import io.ktor.client.features.logging.DEFAULT
 import io.ktor.client.features.logging.LogLevel
 import io.ktor.client.features.logging.Logger
 import io.ktor.client.features.logging.Logging
 import io.ktor.client.request.header
+import io.ktor.client.statement.readText
 import io.ktor.network.sockets.SocketTimeoutException
 import kotlinx.coroutines.runBlocking
 import org.junit.jupiter.api.AfterAll
@@ -136,6 +139,38 @@ class GraphQLKtorClientTest {
         }
     }
 
+    @Test
+    fun `verifies Non-OK HTTP responses will throw error`() {
+        WireMock.stubFor(
+            WireMock.post("/graphql")
+                .willReturn(WireMock.aResponse().withStatus(500).withBody("Internal server error"))
+        )
+
+        val client = GraphQLKtorClient(URL("${wireMockServer.baseUrl()}/graphql"))
+        val error = assertFailsWith<ServerResponseException> {
+            runBlocking {
+                client.execute<HelloWorldResult>("query HelloWorldQuery { helloWorld }")
+            }
+        }
+        assertEquals(500, error.response.status.value)
+        assertEquals("Internal server error", runBlocking { error.response.readText() })
+    }
+
+    @Test
+    fun `verifies response with empty body will throw error`() {
+        WireMock.stubFor(
+            WireMock.post("/graphql")
+                .willReturn(WireMock.aResponse().withStatus(204))
+        )
+
+        val client = GraphQLKtorClient(URL("${wireMockServer.baseUrl()}/graphql"))
+        assertFailsWith<MismatchedInputException> {
+            runBlocking {
+                client.execute<HelloWorldResult>("query HelloWorldQuery { helloWorld }")
+            }
+        }
+    }
+
     private fun stubResponse(response: GraphQLResponse<*>, delayMillis: Int = 0): MappingBuilder =
         WireMock.post("/graphql")
             .willReturn(

clients/graphql-kotlin-spring-client/src/main/kotlin/com/expediagroup/graphql/client/GraphQLWebClient.kt

@@ -21,13 +21,12 @@ import com.fasterxml.jackson.databind.DeserializationFeature
 import com.fasterxml.jackson.databind.JavaType
 import com.fasterxml.jackson.databind.ObjectMapper
 import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper
+import kotlinx.coroutines.reactive.awaitSingle
 import org.springframework.http.MediaType
 import org.springframework.http.codec.json.Jackson2JsonDecoder
 import org.springframework.http.codec.json.Jackson2JsonEncoder
 import org.springframework.web.reactive.function.client.ExchangeStrategies
 import org.springframework.web.reactive.function.client.WebClient
-import org.springframework.web.reactive.function.client.awaitBody
-import org.springframework.web.reactive.function.client.awaitExchange
 
 /**
  * A lightweight typesafe GraphQL HTTP client using Spring WebClient engine.
@@ -81,8 +80,9 @@ open class GraphQLWebClient(
             .accept(MediaType.APPLICATION_JSON)
             .contentType(MediaType.APPLICATION_JSON)
             .bodyValue(graphQLRequest)
-            .awaitExchange()
-            .awaitBody<String>()
+            .retrieve()
+            .bodyToMono(String::class.java)
+            .awaitSingle()
 
         @Suppress("BlockingMethodInNonBlockingContext")
         return mapper.readValue(rawResult, parameterizedType(resultType))

clients/graphql-kotlin-spring-client/src/test/kotlin/com/expediagroup/graphql/client/GraphQLWebClientTest.kt

@@ -21,6 +21,7 @@ import org.junit.jupiter.api.Test
 import org.springframework.http.client.reactive.ClientHttpConnector
 import org.springframework.http.client.reactive.ReactorClientHttpConnector
 import org.springframework.web.reactive.function.client.WebClient
+import org.springframework.web.reactive.function.client.WebClientResponseException
 import reactor.netty.http.client.HttpClient
 import java.util.concurrent.TimeUnit
 import kotlin.test.assertEquals
@@ -135,6 +136,38 @@ class GraphQLWebClientTest {
         }
     }
 
+    @Test
+    fun `verifies Non-OK HTTP responses will throw error`() {
+        WireMock.stubFor(
+            WireMock.post("/graphql")
+                .willReturn(WireMock.aResponse().withStatus(500).withBody("Internal server error"))
+        )
+
+        val client = GraphQLWebClient(url = "${wireMockServer.baseUrl()}/graphql")
+        val error = assertFailsWith<WebClientResponseException> {
+            runBlocking {
+                client.execute<HelloWorldResult>("query HelloWorldQuery { helloWorld }")
+            }
+        }
+        assertEquals(500, error.rawStatusCode)
+        assertEquals("Internal server error", error.responseBodyAsString)
+    }
+
+    @Test
+    fun `verifies response with empty body will throw error`() {
+        WireMock.stubFor(
+            WireMock.post("/graphql")
+                .willReturn(WireMock.aResponse().withStatus(204))
+        )
+
+        val client = GraphQLWebClient(url = "${wireMockServer.baseUrl()}/graphql")
+        assertFailsWith<NoSuchElementException> {
+            runBlocking {
+                client.execute<HelloWorldResult>("query HelloWorldQuery { helloWorld }")
+            }
+        }
+    }
+
     private fun stubResponse(response: GraphQLResponse<*>, delayMillis: Int = 0): MappingBuilder =
         WireMock.post("/graphql")
             .willReturn(