getsentry / getsentry/sentry-java

SentryOkHttpEventListener breaks EventListener.Factory contract

Đang mở
#5,967 2 bình luận 1 reaction 1 người được giao Được @markushi nhận Xem trên GitHub
Bug Java Platform: Java
Ngôn ngữ chính
Kotlin
Star
1.4k
Fork
478
Merge trung bình
2 ngày 23 giờ
Pull request đã merge (30 ngày)
67

Mô tả

### Integration

sentry-okhttp

### Java Version

8 but it's unrelated

### Other Error Monitoring Solution

No

### Other Error Monitoring Solution Name

_No response_

### Version

8.21.1 but it's unrelated

### Steps to Reproduce

This might be more related to the Sentry Android Gradle plugin than the Java project per se, but if we want to use SentryOkHttpEventListener (which I presume sends some useful Breadcrumbs? at least) I think the fix will need to be here (or for the Gradle plugin to use EventListener.plus in OkHttp 5.13, I think).

It looks like SentryOkHttpEventListener isn't designed for concurrency and that it breaks the EventListener.Factory contract.

If you use a custom EventListener.Factory with the Sentry Android Gradle plugin, the custom Factory gets wrapped by the SentryOkHttpEventListener.

However, the SentryOkHttpEventListener doesn't respect the EventListener.Factory contract, which expects that the EventListener instance is used for the lifetime of the Call. Instead, `SentryOkHttpEventListener` will pass the _latest_ EventListener instance to every concurrent Call, because it's updating the 1 instance it uses after every `SentryOkHttpEventListener.callStart`.

The steps to repro were simply:

1. use SentryOkHttpEventListener around a custom EventListener.Factory (only 1 eventListener or 1 eventListenerFactory can be set on an OkHttpClient at the same time)
2. in the custom EventListener.Factory, capture the original Call
3. launch multiple overlapping requests

```
private class MismatchDetectingListener(
private val ownCall: Call,
private val mismatches: MutableList,
) : EventListener() {
private fun verify(method: String, cbCall: Call) {
if (cbCall !== ownCall) {
val msg = "MISMATCH $method: built for ${ownCall.request().url} invoked with ${cbCall.request().url} " +
"thread=${Thread.currentThread().name}"
mismatches += msg
Log.e(TAG, msg)
}
}

override fun callStart(call: Call) = verify("callStart", call)
override fun dnsStart(call: Call, domainName: String) = verify("dnsStart", call)
}
....
val perCallFactory = EventListener.Factory { call -> MismatchDetectingListener(call, mismatches) }
val client = OkHttpClient.Builder()
.eventListener(SentryOkHttpEventListener(originalEventListenerFactory = perCallFactory))
.build()
....

val response1 = async { client.newCall(request1).execute() }
val response2 = async { client.newCall(request2).execute() }
val response3 = async { client.newCall(request3).execute() }
val response4= async { client.newCall(request4).execute() }

```

### Expected Result

Each EventListener should only ever receive calls from the Call it was created alongside in EventListener.Factory.create

### Actual Result

The latest EventListener that was created receives every callback

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.