Skip to content

fix(okhttp): keep the wrapped EventListener per Call - #6003

Merged
markushi merged 15 commits into
mainfrom
fix/okhttp-event-listener-per-call
Sep 18, 2026
Merged

markushi merged 15 commits into
mainfrom
fix/okhttp-event-listener-per-call

Conversation

@markushi

Copy link
Copy Markdown
Member

📜 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.

💚 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

linear-code Bot commented Aug 26, 2026

Copy link
Copy Markdown

JAVA-695

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

sentry Bot commented Aug 26, 2026

Copy link
Copy Markdown

📲 Install Builds

Android

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

⚙️ sentry-android Build Distribution Settings

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@markushi markushi added the sanity-check PR needs a lightweight review for obvious issues label Aug 26, 2026
@markushi
markushi marked this pull request as ready for review August 26, 2026 10:00
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 0xadam-brown left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this 💯 !

One comment worth addressing; otherwise looking good.

Comment thread sentry-okhttp/src/main/java/io/sentry/okhttp/SentryOkHttpEventListener.kt Outdated
Keep both Unreleased changelog entries.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@markushi
markushi force-pushed the fix/okhttp-event-listener-per-call branch from bb23afd to b508070 Compare August 28, 2026 06:58
@markushi
markushi requested a review from 0xadam-brown August 28, 2026 07:10

@0xadam-brown 0xadam-brown left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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 Outdated

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Fix All in Cursor

❌ 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.

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

Copy link
Copy Markdown

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

@0xadam-brown

Copy link
Copy Markdown
Member

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 0xadam-brown left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few quick comments, but no blockers.

@markushi
markushi merged commit 8f0dc10 into main Sep 18, 2026
70 of 71 checks passed
@markushi
markushi deleted the fix/okhttp-event-listener-per-call branch September 18, 2026 11:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sanity-check PR needs a lightweight review for obvious issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SentryOkHttpEventListener breaks EventListener.Factory contract

3 participants