fix(okhttp): keep the wrapped EventListener per Call by markushi · Pull Request #6003 · getsentry/sentry-java

markushi · 2026-08-26T09:35:39Z

📜 Description

SentryOkHttpEventListener held the wrapped EventListener in a single mutable field that
callStart overwrote for each call. It is now kept in a per-Call map, the same pattern the class
already uses for eventMap. No public API change.

💡 Motivation and Context

OkHttp uses one listener instance for all calls, thus concurrent calls were all delegated to the
listener made for the call that started last. This breaks the EventListener.Factory contract and
loses the terminal callEnd/callFailed of every overlapping call.

resolves: SentryOkHttpEventListener breaks EventListener.Factory contract #5967
resolves: JAVA-695

💚 How did you test it?

Added unit tests.

📝 Checklist

I added GH Issue ID & Linear ID
I added tests to verify the changes.
No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
I updated the docs if needed.
I updated the wizard if needed.
Review from the native team if needed.
No breaking change or entry added to the changelog.
No breaking change for hybrid SDKs or communicated to hybrid SDKs.
Public API changes reviewed by another Mobile SDK team member or implemented according to the develop docs spec.

🔮 Next steps

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

linear-code · 2026-08-26T09:35:43Z

JAVA-695

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

sentry · 2026-08-26T09:37:50Z

📲 Install Builds

Android

🔗 App Name	App ID	Version	Configuration
SDK Size	io.sentry.tests.size	8.56.0 (1)	release

⚙️ sentry-android Build Distribution Settings

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Move the okhttp changelog entry into a new Unreleased section, as 8.54.0 was released on main. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

0xadam-brown

Thanks for this 💯 !

One comment worth addressing; otherwise looking good.

Keep both Unreleased changelog entries. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

0xadam-brown

Excellent! One tweak more to satisfy the EventListener.Factory contract, and I think we'll be there 🥇

0xadam-brown · 2026-08-28T07:48:09Z

+    // callEnd()/callFailed(), so there is not always a listener bound to the call. Create one on
+    // the fly in that case, but do not put it in the map: nothing would remove it again, because
+    // a call that is canceled before it starts never gets a callEnd() or callFailed().
+    val originalEventListener =


We're close! (and thanks for the great updates)

We still need to preserve the contract of EventListener.Factory that ensures only one EventListener instance is produced per Call lifecycle. Ie, the listener returned by the factory for a given call needs to be the listener that captures i) all of that call's lifecycle and ii) no other call's lifecycle.

We've fixed (ii), but we're still violating (i) in the case of cancelation because we're creating an extra listener for early and late cancel() invocations.

Possible solution

Thoughts about using a weak per-call map for the wrapped listener instead? Something like a WeakHashMap<Call, EventListener> guarded by synchronized, with a getOrCreateOriginalEventListener(call) helper used by both callStart and canceled().

That'd^^ let us avoid removing entries on callEnd / callFailed, and completed calls would be gc'd as soon as the Call instance is unreachable.

0xadam-brown · 2026-08-28T07:51:20Z

+  }
+
+  @Test
+  fun `cancel before callStart is delegated`() {


Thanks for the new tests 💯

Bonus points if our cancellation tests can assert the stronger factory-contract invariant 👍

(Right now cancel before callStart is delegated and cancel after callEnd is delegated prove that some listener receives canceled(), rather than that a single listener receives all lifecycle callbacks.)

cursor

Cursor Bugbot has reviewed your changes and found 2 potential issues.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.

^{Reviewed by Cursor Bugbot for commit 728129d. Configure here.}

…r for them

victorlai-stripe · 2026-09-03T21:03:28Z

Lurking - hi @0xadam-brown! Crazy seeing you around, hope you're doing well!

0xadam-brown · 2026-09-06T05:05:08Z

Lurking - hi @0xadam-brown! Crazy seeing you around, hope you're doing well!

@victorlai-stripe – so good to see your name show up here! Definitely doing well (and enjoying Sentry). Hope Stripe's been an excellent change-up on your end too 💯 !

0xadam-brown

A few quick comments, but no blockers.

0xadam-brown · 2026-09-16T14:07:12Z

@@ -65,7 +70,9 @@ public open class SentryOkHttpEventListener(

  public constructor(


m: Given that we're deviating from the EventListener contract by ignoring cancel() calls outside callStart() and callEnd(), could be sensible to add KDocs to the constructors to put folks on notice.

Eg, maybe something like:

/** * Wraps a single [okhttp3.EventListener] instance and shares it across [Call]s. * * All callbacks are forwarded to the listener, including those delivered outside * the normal `callStart()`..`callEnd()` / `callFailed()` window. */ public constructor(originalEventListener: EventListener)

/** * Wraps an [okhttp3.EventListener.Factory] and creates a separate * [okhttp3.EventListener] per [Call]. * * Callbacks that OkHttp may deliver before `callStart()` or after `callEnd()` / * `callFailed()`, such as [okhttp3.EventListener.canceled], are not guaranteed to * be forwarded to the listener. */ public constructor(originalEventListenerFactory: Factory)

0xadam-brown · 2026-09-16T14:07:24Z

+  }
+
+  @Test
+  fun `cancel before callStart is not delegated and creates no listener`() {


l: Might be worth a quick KDoc noting that we're deliberately deviating from the EventListener contract here + why we're doing so. (Same for relevant tests below.)

fix(okhttp): keep the wrapped EventListener per Call

105211a

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

docs: add changelog entry

14f6b50

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

refactor(okhttp): look up the wrapped EventListener once per callback

11786ec

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

markushi added the sanity-check PR needs a lightweight review for obvious issues label Aug 26, 2026

markushi marked this pull request as ready for review August 26, 2026 10:00

markushi requested review from 0xadam-brown, adinauer, romtsn and runningcode as code owners August 26, 2026 10:00

chore: merge origin/main into fix/okhttp-event-listener-per-call

3506dcc

Move the okhttp changelog entry into a new Unreleased section, as 8.54.0 was released on main. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

0xadam-brown reviewed Aug 27, 2026

View reviewed changes

Comment thread sentry-okhttp/src/main/java/io/sentry/okhttp/SentryOkHttpEventListener.kt Outdated

chore: merge origin/main into fix/okhttp-event-listener-per-call

247ee38

Keep both Unreleased changelog entries. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

sentry Bot reviewed Aug 28, 2026

View reviewed changes

Comment thread sentry-okhttp/src/main/java/io/sentry/okhttp/SentryOkHttpEventListener.kt

fix(okhttp): delegate canceled() outside of the call window

b508070

markushi force-pushed the fix/okhttp-event-listener-per-call branch from bb23afd to b508070 Compare August 28, 2026 06:58

markushi requested a review from 0xadam-brown August 28, 2026 07:10

0xadam-brown reviewed Aug 28, 2026

View reviewed changes

markushi added 2 commits August 28, 2026 10:07

chore: merge origin/main into fix/okhttp-event-listener-per-call

552a731

fix(okhttp): bind out-of-window cancels to the listener of their call

728129d

sentry Bot reviewed Aug 31, 2026

View reviewed changes

Comment thread sentry-okhttp/src/main/java/io/sentry/okhttp/SentryOkHttpEventListener.kt Outdated

cursor Bot reviewed Aug 31, 2026

View reviewed changes

Comment thread sentry-okhttp/src/main/java/io/sentry/okhttp/SentryOkHttpEventListener.kt Outdated

Comment thread sentry-okhttp/src/main/java/io/sentry/okhttp/SentryOkHttpEventListener.kt

fix(okhttp): drop out-of-window cancels instead of creating a listene…

7b7f12e

…r for them

markushi added 3 commits September 15, 2026 16:48

fix(okhttp): avoid computeIfAbsent to keep Android API 21 compatibility

0600e6c

Merge branch 'main' into fix/okhttp-event-listener-per-call

841d1f5

Fix Changelog

af59e38

0xadam-brown reviewed Sep 16, 2026

View reviewed changes

		@@ -65,7 +70,9 @@ public open class SentryOkHttpEventListener(

		public constructor(

Uh oh!

Uh oh!

Conversation

markushi commented Aug 26, 2026

📜 Description

💡 Motivation and Context

💚 How did you test it?

📝 Checklist

🔮 Next steps

Uh oh!

linear-code Bot commented Aug 26, 2026

Uh oh!

sentry Bot commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📲 Install Builds

Android

Uh oh!

0xadam-brown left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

0xadam-brown left a comment

Choose a reason for hiding this comment

Uh oh!

0xadam-brown Aug 28, 2026

Choose a reason for hiding this comment

Uh oh!

0xadam-brown Aug 28, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

victorlai-stripe commented Sep 3, 2026

Uh oh!

0xadam-brown commented Sep 6, 2026

Uh oh!

0xadam-brown left a comment

Choose a reason for hiding this comment

Uh oh!

0xadam-brown Sep 16, 2026

Choose a reason for hiding this comment

Uh oh!

0xadam-brown Sep 16, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sentry Bot commented Aug 26, 2026 •

edited

Loading