Skip to content

Commit 54f2ecb

Browse files
committed
Fix GraphQL errors feature for streaming content types
1 parent f061d4f commit 54f2ecb

4 files changed

Lines changed: 44 additions & 13 deletions

File tree

integrations/dd-sdk-android-okhttp/src/main/kotlin/com/datadog/android/okhttp/DatadogInterceptor.kt

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import com.datadog.android.api.feature.FeatureSdkCore
1414
import com.datadog.android.core.InternalSdkCore
1515
import com.datadog.android.core.configuration.Configuration
1616
import com.datadog.android.core.sampling.Sampler
17+
import com.datadog.android.internal.network.HttpSpec
1718
import com.datadog.android.okhttp.internal.RumResourceAttributesProviderCompatibilityAdapter
1819
import com.datadog.android.okhttp.internal.buildResourceId
1920
import com.datadog.android.okhttp.internal.graphql.OkHttpGraphQLAdapter
@@ -295,8 +296,8 @@ open class DatadogInterceptor internal constructor(
295296
// manually rebuild the mimetype as `toString()` can also include the charsets
296297
it.type + "/" + it.subtype
297298
}
298-
val isStream = contentType in STREAM_CONTENT_TYPES
299-
val isWebSocket = !response.header(WEBSOCKET_ACCEPT_HEADER, null).isNullOrBlank()
299+
val isStream = HttpSpec.ContentType.isStream(contentType)
300+
val isWebSocket = !response.header(HttpSpec.Header.WEBSOCKET_ACCEPT_HEADER, null).isNullOrBlank()
300301
if (body == null || isStream || isWebSocket) {
301302
return null
302303
}
@@ -437,15 +438,6 @@ open class DatadogInterceptor internal constructor(
437438
// endregion
438439

439440
internal companion object {
440-
internal val STREAM_CONTENT_TYPES = setOf(
441-
"text/event-stream",
442-
"application/grpc",
443-
"application/grpc+proto",
444-
"application/grpc+json"
445-
)
446-
447-
internal const val WEBSOCKET_ACCEPT_HEADER = "Sec-WebSocket-Accept"
448-
449441
internal const val WARN_RUM_DISABLED =
450442
"You set up a DatadogInterceptor for %s, but RUM features are disabled. " +
451443
"Make sure you initialized the Datadog SDK with a valid Application Id, " +

integrations/dd-sdk-android-okhttp/src/main/kotlin/com/datadog/android/okhttp/internal/graphql/OkHttpGraphQLAdapter.kt

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ package com.datadog.android.okhttp.internal.graphql
99
import androidx.annotation.WorkerThread
1010
import com.datadog.android.api.InternalLogger
1111
import com.datadog.android.internal.network.GraphQLHeaders
12+
import com.datadog.android.internal.network.HttpSpec
1213
import com.datadog.android.okhttp.internal.OkHttpHttpResponseInfo
1314
import com.datadog.android.okhttp.internal.OkHttpRequestInfo
1415
import com.datadog.android.rum.RumAttributes
@@ -68,7 +69,13 @@ internal class OkHttpGraphQLAdapter(
6869
graphqlAttributes: Map<String, Any?>,
6970
internalLogger: InternalLogger
7071
): Map<String, Any> {
71-
if (graphqlAttributes.isEmpty()) return emptyMap()
72+
// Streaming responses surface GraphQL errors per-frame, not as a top-level `errors` array.
73+
// Draining their bodies via peekBody().string() would block until the (potentially unbounded) body completes.
74+
val body = response.body
75+
val mimeType = body?.contentType()?.let { it.type + "/" + it.subtype }
76+
val isStream = HttpSpec.ContentType.isStream(mimeType)
77+
val isWebSocket = !response.header(HttpSpec.Header.WEBSOCKET_ACCEPT_HEADER, null).isNullOrBlank()
78+
if (graphqlAttributes.isEmpty() || body == null || isStream || isWebSocket) return emptyMap()
7279
return try {
7380
val responseInfo = OkHttpHttpResponseInfo(response, internalLogger)
7481

integrations/dd-sdk-android-okhttp/src/test/kotlin/com/datadog/android/okhttp/DatadogInterceptorTest.kt

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,9 @@ internal class DatadogInterceptorTest : TracingInterceptorNotSendingSpanTest() {
303303
@IntForgery(min = 200, max = 300) statusCode: Int
304304
) {
305305
// Given
306-
val mimeType = forge.anElementFrom(DatadogInterceptor.STREAM_CONTENT_TYPES)
306+
val mimeType = forge.anElementFrom(
307+
HttpSpec.ContentType.values().filter(HttpSpec.ContentType::isStream)
308+
)
307309
fakeMediaType = mimeType.toMediaType()
308310
stubChain(mockChain, statusCode)
309311
val expectedStartAttrs = emptyMap<String, Any?>()

integrations/dd-sdk-android-okhttp/src/test/kotlin/com/datadog/android/okhttp/internal/graphql/OkHttpGraphQLAdapterTest.kt

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,5 +275,35 @@ internal class OkHttpGraphQLAdapterTest {
275275
)
276276
}
277277

278+
@Test
279+
fun `M return empty and not read body W extractGraphQLErrorAttributes() {streaming response}`(
280+
@StringForgery fakeOpName: String
281+
) {
282+
// Given
283+
val streamContentType = forge.anElementFrom(
284+
HttpSpec.ContentType.values().filter(HttpSpec.ContentType::isStream)
285+
)
286+
val unreadableBody = object : ResponseBody() {
287+
override fun contentType(): MediaType = streamContentType.toMediaType()
288+
override fun contentLength(): Long = -1L
289+
override fun source(): BufferedSource {
290+
throw AssertionError(
291+
"extractGraphQLErrorAttributes must not read body of streaming response"
292+
)
293+
}
294+
}
295+
val request = Request.Builder()
296+
.url("https://example.com/graphql")
297+
.build()
298+
val response = forge.anOkHttpResponse(request, 200) { body(unreadableBody) }
299+
val graphqlAttributes = mapOf<String, Any?>(RumAttributes.GRAPHQL_OPERATION_NAME to fakeOpName)
300+
301+
// When
302+
val result = testedHelper.extractGraphQLErrorAttributes(response, graphqlAttributes, mockInternalLogger)
303+
304+
// Then
305+
assertThat(result).isEmpty()
306+
}
307+
278308
// endregion
279309
}

0 commit comments

Comments
 (0)