Skip to content

Commit f0d8837

Browse files
committed
Fix GraphQL errors feature for streaming content types
1 parent 7a4cd87 commit f0d8837

4 files changed

Lines changed: 45 additions & 14 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.internal.telemetry.InternalTelemetryEvent
1819
import com.datadog.android.okhttp.internal.RumResourceAttributesProviderCompatibilityAdapter
1920
import com.datadog.android.okhttp.internal.buildResourceId
@@ -289,8 +290,8 @@ open class DatadogInterceptor internal constructor(
289290
// manually rebuild the mimetype as `toString()` can also include the charsets
290291
it.type + "/" + it.subtype
291292
}
292-
val isStream = contentType in STREAM_CONTENT_TYPES
293-
val isWebSocket = !response.header(WEBSOCKET_ACCEPT_HEADER, null).isNullOrBlank()
293+
val isStream = HttpSpec.ContentType.isStream(contentType)
294+
val isWebSocket = !response.header(HttpSpec.Header.WEBSOCKET_ACCEPT_HEADER, null).isNullOrBlank()
294295
if (body == null || isStream || isWebSocket) {
295296
return null
296297
}
@@ -433,15 +434,6 @@ open class DatadogInterceptor internal constructor(
433434
// endregion
434435

435436
internal companion object {
436-
internal val STREAM_CONTENT_TYPES = setOf(
437-
"text/event-stream",
438-
"application/grpc",
439-
"application/grpc+proto",
440-
"application/grpc+json"
441-
)
442-
443-
internal const val WEBSOCKET_ACCEPT_HEADER = "Sec-WebSocket-Accept"
444-
445437
internal const val WARN_RUM_DISABLED =
446438
"You set up a DatadogInterceptor for %s, but RUM features are disabled. " +
447439
"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: 12 additions & 2 deletions
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.OkHttpRequestInfo
1314
import com.datadog.android.okhttp.internal.OkHttpResponseInfo
1415
import com.datadog.android.rum.RumAttributes
@@ -63,22 +64,31 @@ internal class OkHttpGraphQLAdapter(
6364
graphQLExtractor.extractGraphQLAttributes(OkHttpRequestInfo(request))
6465

6566
@WorkerThread
67+
@Suppress("ReturnCount")
6668
fun extractGraphQLErrorAttributes(
6769
response: Response,
6870
graphqlAttributes: Map<String, Any?>,
6971
internalLogger: InternalLogger
7072
): Map<String, Any> {
7173
if (graphqlAttributes.isEmpty()) return emptyMap()
74+
// Streaming responses surface GraphQL errors per-frame, not as a top-level `errors` array.
75+
// Draining their bodies via peekBody().string() would block until the (potentially unbounded) body completes.
76+
val body = response.body
77+
val mimeType = body?.contentType()?.let { it.type + "/" + it.subtype }
78+
val isStream = HttpSpec.ContentType.isStream(mimeType)
79+
val isWebSocket = !response.header(HttpSpec.Header.WEBSOCKET_ACCEPT_HEADER, null).isNullOrBlank()
80+
if (body == null || isStream || isWebSocket) return emptyMap()
81+
7282
return try {
7383
val responseInfo = OkHttpResponseInfo(response, internalLogger)
7484

7585
@Suppress("UnsafeThirdPartyFunctionCall") // exceptions are caught
76-
val body = response
86+
val bodyString = response
7787
.peekBody(GraphQLExtractor.MAX_GRAPHQL_BODY_PEEK)
7888
.string()
7989
graphQLExtractor.extractGraphQLErrors(
8090
responseInfo.contentType,
81-
body,
91+
bodyString,
8292
internalLogger
8393
)?.let {
8494
mapOf(RumAttributes.GRAPHQL_ERRORS to it)

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
@@ -312,7 +312,9 @@ internal class DatadogInterceptorTest : TracingInterceptorNotSendingSpanTest() {
312312
@IntForgery(min = 200, max = 300) statusCode: Int
313313
) {
314314
// Given
315-
val mimeType = forge.anElementFrom(DatadogInterceptor.STREAM_CONTENT_TYPES)
315+
val mimeType = forge.anElementFrom(
316+
HttpSpec.ContentType.values().filter(HttpSpec.ContentType::isStream)
317+
)
316318
fakeMediaType = mimeType.toMediaType()
317319
stubChain(mockChain, statusCode)
318320
val expectedStartAttrs = emptyMap<String, Any?>()

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import org.mockito.kotlin.anyOrNull
3535
import org.mockito.kotlin.argumentCaptor
3636
import org.mockito.kotlin.doReturn
3737
import org.mockito.kotlin.eq
38+
import org.mockito.kotlin.never
3839
import org.mockito.kotlin.verify
3940
import org.mockito.kotlin.whenever
4041
import java.io.IOException
@@ -275,5 +276,31 @@ internal class OkHttpGraphQLAdapterTest {
275276
)
276277
}
277278

279+
@Test
280+
fun `M return empty and skip extraction W extractGraphQLErrorAttributes() {streaming response}`(
281+
@StringForgery fakeOpName: String,
282+
@StringForgery fakeBody: String
283+
) {
284+
// Given
285+
val streamContentType = forge.anElementFrom(
286+
HttpSpec.ContentType.values().filter(HttpSpec.ContentType::isStream)
287+
)
288+
val request = Request.Builder()
289+
.url("https://example.com/graphql")
290+
.build()
291+
292+
val response = forge.anOkHttpResponse(request, 200) {
293+
body(fakeBody.toResponseBody(streamContentType.toMediaType()))
294+
}
295+
val graphqlAttributes = mapOf<String, Any?>(RumAttributes.GRAPHQL_OPERATION_NAME to fakeOpName)
296+
297+
// When
298+
val result = testedHelper.extractGraphQLErrorAttributes(response, graphqlAttributes, mockInternalLogger)
299+
300+
// Then
301+
assertThat(result).isEmpty()
302+
verify(mockGraphQLExtractor, never()).extractGraphQLErrors(any(), any(), any())
303+
}
304+
278305
// endregion
279306
}

0 commit comments

Comments
 (0)