Skip to content

fix: capturing with a configureScope callback no longer re-runs BeforeBreadcrumb or re-syncs the scope to native SDKs - #5653

Merged
jamescrosswell merged 5 commits into
mainfrom
fix/scope-clone-side-effects
Oct 6, 2026
Merged

jamescrosswell merged 5 commits into
mainfrom
fix/scope-clone-side-effects

Conversation

@jamescrosswell

@jamescrosswell jamescrosswell commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Scope.Clone() copied data into the new scope through the public mutators (AddBreadcrumb, SetTag, User, Environment, ...). Every clone therefore:

  • re-ran BeforeBreadcrumb on breadcrumbs it had already processed. This affects every platform, not only scope sync: with a transforming callback, CaptureException(ex, s => ...) sent crumb [scrubbed] [scrubbed] where CaptureException(ex) sent crumb [scrubbed]
  • re-applied TagFilters
  • re-sent the whole scope (breadcrumbs, tags, extras, user one property at a time, environment, trace) to the scope observer. On MAUI this duplicated breadcrumbs on the native scope on every CaptureException(ex, configureScope), and in non-global mode on every PushScope

Clone() now fills in the new scope with scope sync turned off, and copies breadcrumbs and tags directly so BeforeBreadcrumb and TagFilters don't run again.

Separately, the temporary scope handed to configureScope in CaptureEvent/CaptureFeedback no longer syncs to the scope observer. That scope only exists for a single event, but tags, users and breadcrumbs set on it were synced to the native scope and stayed there, ending up on later native crashes.

Notes for review

  • The second change answers the open question in the issue. Managed events are sent by the managed transport on every platform, so the native scope doesn't need per-event data. It's easy to drop if we'd rather not: two lines in Hub and the two *_ConfigureScope_ChangesNotSynced tests.
  • Both changes use a new internal Scope.ScopeSyncEnabled flag. The ten observer calls in Scope now go through a single private ScopeObserver accessor that checks it, instead of each checking Options.EnableScopeSync.
  • In non-global mode, the first PushScope() in the Hub constructor used to send the root trace to the scope observer as a side effect of cloning. The Hub constructor now does that explicitly, so native crashes stay on the same trace as managed errors. Global mode (iOS/Android) never had this sync and is unchanged.
  • SdkVersion.CopyTo is shared by Apply and Clone.
  • Apply(Scope) is public and unchanged: applying one scope onto another still goes through the mutators.

Closes #5647

🤖 Generated with Claude Code

jamescrosswell and others added 2 commits October 1, 2026 11:25
Scope.Clone copied data through the public mutators, so every clone re-ran BeforeBreadcrumb and TagFilters and re-sent the whole scope to the scope observer. Clone now copies state directly.

The temporary scope handed to configureScope in CaptureEvent and CaptureFeedback no longer syncs to the scope observer either, so per-event data stays off the native scope.

Fixes #5647

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

codecov Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.05%. Comparing base (70909e9) to head (6ad6152).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5653      +/-   ##
==========================================
+ Coverage   74.94%   75.05%   +0.11%     
==========================================
  Files         515      515              
  Lines       18975    18990      +15     
  Branches     3695     3692       -3     
==========================================
+ Hits        14220    14253      +33     
+ Misses       3875     3869       -6     
+ Partials      880      868      -12     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread src/Sentry/Scope.cs Outdated
Clone now populates the new scope with scope sync disabled, so Environment, User and Transaction can go through their setters. Only breadcrumbs and tags are still copied directly, to skip BeforeBreadcrumb and TagFilters.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jamescrosswell
jamescrosswell marked this pull request as ready for review October 4, 2026 23:53
@github-actions github-actions Bot added the risk: medium PR risk score: medium label Oct 4, 2026

@ric-oliv ric-oliv 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.

Nice! I've added a couple of edge cases as inline comments, but the rest looks great!

Comment thread src/Sentry/Scope.cs
Comment thread test/Sentry.Tests/ScopeTests.cs
jamescrosswell and others added 2 commits October 6, 2026 06:06
Clone_CopiesAllData now also covers OnEvaluating, SessionUpdate and the scope processors, and Clone_EveryFieldHandled fails when a Scope field is added without deciding whether Clone copies it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The first PushScope in the Hub constructor used to send the root propagation context to the scope observer as a side effect of cloning. Cloning no longer syncs, so the Hub now does it explicitly, keeping native crashes on the same trace as managed errors.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ric-oliv
ric-oliv self-requested a review October 6, 2026 11:35

@ric-oliv ric-oliv 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.

Nice!

@jamescrosswell
jamescrosswell merged commit 510785b into main Oct 6, 2026
85 of 87 checks passed
@jamescrosswell
jamescrosswell deleted the fix/scope-clone-side-effects branch October 6, 2026 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk: medium PR risk score: medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Scope.Clone re-syncs breadcrumbs, tags and user to the native SDK

2 participants