getsentry / sentry-java

A Sentry SDK for Java, Android and other JVM languages.
https://docs.sentry.io/
MIT License
1.12k stars 428 forks source link

Crashes on Motorola devices for Sentry.traceHeaders() IllegalArgumentException:String representation of SentryId has either 32 (UUID no dashes) or 36 characters long (completed UUID). Received: 00000000 #2325

Closed kibotu closed 1 year ago

kibotu commented 1 year ago

Gradle Version

7.5.1

AGP Version

7.3.0

Code Minifier/Optimizer

R8

Version

3.2.0

Sentry SDK Version

6.5.0

Steps to Reproduce

app crashes on Motorola devices with

String representation of SentryId has either 32 (UUID no dashes) or 36 characters long (completed UUID). Received: 00000000

looks like Sentry.traceHeaders() will not have valid uuids on Motorola devices.

   TransactionContext.fromSentryTrace(
                "HTTP ${request.method} ${request.url.host}${request.url.encodedPath}",
                "request",
                Sentry.traceHeaders() ?: SentryTraceHeader(SentryId.EMPTY_ID, SpanId.EMPTY_ID, true)
            ),
            false
        )

Expected Result

well ... not to crash (voting for github template for crash reporting)

Actual Result

crashes

affected devices

Screenshot 2022-11-02 at 12 02 19
romtsn commented 1 year ago

This looks like a Motorola issue indeed (e.g. a similar issue reported on Gson), but could you share a bit more details, why are you using TransactionContext.fromSentryTrace on mobile?

Also, I believe Sentry.traceHeaders() cannot cause the issue, because it does not invoke the constructor which throws the aforementioned exception, so it must be either the profile id or traceContext... Could you share the full stacktrace of the exception?

I'm all in for adding a guard flag against invalid UUID, but wanna better understand exactly what is the pattern. It's hard to reproduce since we do not have those Motorola devices in hand.

Thank you!

kibotu commented 1 year ago

Thanks for looking into it.

I'm afraid the stack trace is obfuscated until the block I've pasted which gets called during app start within an app initializer from Google's startupx library.

To be fair it's possible that we generally used this method wrong.

I'll try to add more infos on Monday.

(Unfortunately we also don't have any Motorola devices)

romtsn commented 1 year ago

@kibotu thanks, yeah, we'd need to find out where exactly this is coming from. You aren't uploading the proguard mappings, are you? Otherwise the stacktrace should be unminifed

kibotu commented 1 year ago

here the stacktrace, as mentioned sentry library is obfuscated:

java.lang.IllegalArgumentException: String representation of SentryId has either 32 (UUID no dashes) or 36 characters long (completed UUID). Received: 00000000
at iz5.<init>(SentryId.java:18)
at zz5.i(SentryTracer.java:38)
at zz5.v(SentryTracer.java:446)
at <reducted>.network.interceptor.SentryApiMonitoringInterceptor.intercept(SentryApiMonitoringInterceptor.kt:165)
at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:159)
at <reducted>.network.interceptor.BlacklistedInterceptor.intercept(BlacklistedInterceptor.kt:10)
at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:159)
at <reducted>.network.interceptor.ApiSentryInterceptor.intercept(ApiSentryInterceptor.kt:10)
at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:159)
at <reducted>.network.interceptor.HeaderInterceptor.intercept(HeaderInterceptor.kt:76)
at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:159)
at <reducted>.network.interceptor.MainThreadWatcherInterceptor.intercept(MainThreadWatcherInterceptor.kt:28)
at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:159)
at okhttp3.internal.connection.ConnectInterceptor.intercept(ConnectInterceptor.kt:33)
at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:159)
at okhttp3.internal.cache.CacheInterceptor.intercept(CacheInterceptor.kt:192)
at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:159)
at okhttp3.internal.http.BridgeInterceptor.intercept(BridgeInterceptor.kt:168)
at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:159)
at okhttp3.internal.http.RetryAndFollowUpInterceptor.intercept(RetryAndFollowUpInterceptor.kt:35)
at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:159)
at lz5.intercept(SentryOkHttpInterceptor.kt:205)
at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:159)
at <reducted>.network.interceptor.CacheSanitizer.intercept(CacheSanitizer.kt:15)
at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:159)
at okhttp3.internal.connection.RealCall.getResponseWithInterceptorChain$okhttp(RealCall.kt:147)
at okhttp3.internal.connection.RealCall$AsyncCall.run(RealCall.kt:48)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
at java.lang.Thread.run(Thread.java:923)

we were trying to add network performance monitoring and added an interceptor to the okhttp client


import io.sentry.Sentry
import io.sentry.SentryTraceHeader
import io.sentry.SpanId
import io.sentry.SpanStatus
import io.sentry.TransactionContext
import io.sentry.protocol.SentryId
import okhttp3.Interceptor
import okhttp3.Response

/**
 * Interceptor used to start transactions and/or spans for monitoring API Requests in Sentry Performance Monitoring
 */
internal class SentryApiMonitoringInterceptor : Interceptor {

    override fun intercept(chain: Interceptor.Chain): Response {

        val request = chain.request()

        val ongoingTransactionOrSpan = Sentry.getSpan() ?: Sentry.startTransaction("Request", "network")

        // add api call child spans to ongoing transaction/span
        val requestChildSpan = ongoingTransactionOrSpan.startChild("HTTP ${request.method}", "${request.url.host}${request.url.encodedPath}")

        // also create a separated transaction for api calls to be viewed independently
        val generalApiTransaction = Sentry.startTransaction(
            TransactionContext.fromSentryTrace(
                "HTTP ${request.method} ${request.url.host}${request.url.encodedPath}",
                "request",
                Sentry.traceHeaders() ?: SentryTraceHeader(SentryId.EMPTY_ID, SpanId.EMPTY_ID, true)
            ),
            false
        )

        val response = chain.proceed(request)

        requestChildSpan.finish(SpanStatus.fromHttpStatusCode(response.code))
        generalApiTransaction.finish(SpanStatus.fromHttpStatusCode(response.code))

        return response
    }
}
romtsn commented 1 year ago

So, after looking into this a bit more, I believe the culprit is here https://github.com/getsentry/sentry-java/blob/bb20890756cf52cb48409ac4162d86999e2cea02/sentry/src/main/java/io/sentry/protocol/SentryId.java#L16

probably on Motorola devices new UUID(0, 0) returns an incorrect UUID.

The immediate fix for you @kibotu will be to get rid of this block:

val generalApiTransaction = Sentry.startTransaction(
    TransactionContext.fromSentryTrace(
        "HTTP ${request.method} ${request.url.host}${request.url.encodedPath}",
        "request",
        Sentry.traceHeaders() ?: SentryTraceHeader(SentryId.EMPTY_ID, SpanId.EMPTY_ID, true)
    ),
    false
)

because fromSentryTrace is supposed to be used on backend anyway. If you want to have trace propagation on your custom transaction, you can check how we do it in our SentryOkHttpInterceptor.

However we should still add some checks to make SentryId fail-safe.

kibotu commented 1 year ago

thanks :)