Skip to content

fix: prevent false replay config restarts from ScreenshotWidgetStatus equality issues#3114

Merged
buenaflor merged 15 commits into
mainfrom
fix/screenshot-status-equality
Jul 31, 2025
Merged

fix: prevent false replay config restarts from ScreenshotWidgetStatus equality issues#3114
buenaflor merged 15 commits into
mainfrom
fix/screenshot-status-equality

Conversation

@buenaflor

@buenaflor buenaflor commented Jul 29, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

📜 Description

This basically checks if the current device configuration e.g orientation, size changed and if so update the config, however this check fails currently meaning it treats every onBuild trigger as a new config.

-> Fixed ScreenshotWidgetStatus equality implementation to prevent false replay configuration restarts and worker spawning on rebuilds.

💡 Motivation and Context

The ScreenshotWidgetStatus class lacked equality comparison, causing the replay configuration to be incorrectly identified as changed on every rebuild.

Improves #3110

💚 How did you test it?

Manual tests, unit tests

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPii is enabled
  • I updated the docs if needed
  • All tests passing
  • No breaking changes

🔮 Next steps

@codecov

codecov Bot commented Jul 29, 2025 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.88889% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.31%. Comparing base (f761369) to head (a440b19).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...r/lib/src/screenshot/sentry_screenshot_widget.dart 80.00% 2 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #3114      +/-   ##
==========================================
+ Coverage   87.66%   88.31%   +0.64%     
==========================================
  Files         288       98     -190     
  Lines        9813     3569    -6244     
==========================================
- Hits         8603     3152    -5451     
+ Misses       1210      417     -793

☔ View full report in Codecov by Sentry.
📢 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.

@buenaflor
buenaflor requested a review from vaind July 29, 2025 13:28
cursor[bot]

This comment was marked as outdated.

Sign in to view

@github-actions

github-actions Bot commented Jul 29, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

iOS Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1264.23 ms 1281.57 ms 17.34 ms
Size 7.86 MiB 9.44 MiB 1.58 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
cc4e375 1253.06 ms 1263.81 ms 10.75 ms
6f47800 1247.52 ms 1259.37 ms 11.85 ms
793f4dc 1262.50 ms 1282.35 ms 19.85 ms
73dca78 1246.65 ms 1265.42 ms 18.76 ms
640ad0c 1241.04 ms 1253.96 ms 12.92 ms
2d34233 1258.19 ms 1268.92 ms 10.73 ms
b6c8720 1252.65 ms 1266.61 ms 13.96 ms
93b7728 1247.23 ms 1264.87 ms 17.64 ms
ec78888 1251.37 ms 1269.40 ms 18.04 ms
dbd526b 1244.78 ms 1259.02 ms 14.24 ms

App size

Revision Plain With Sentry Diff
cc4e375 7.86 MiB 9.44 MiB 1.58 MiB
6f47800 7.86 MiB 9.44 MiB 1.58 MiB
793f4dc 7.86 MiB 9.44 MiB 1.58 MiB
73dca78 7.86 MiB 9.44 MiB 1.58 MiB
640ad0c 7.86 MiB 9.44 MiB 1.58 MiB
2d34233 7.86 MiB 9.44 MiB 1.58 MiB
b6c8720 7.86 MiB 9.44 MiB 1.58 MiB
93b7728 7.86 MiB 9.44 MiB 1.58 MiB
ec78888 7.86 MiB 9.44 MiB 1.58 MiB
dbd526b 7.86 MiB 9.44 MiB 1.58 MiB

Previous results on branch: fix/screenshot-status-equality

Startup times

Revision Plain With Sentry Diff
3ab07e1 1240.89 ms 1252.31 ms 11.41 ms
73ae3ba 1254.98 ms 1264.59 ms 9.61 ms
b254a7e 1259.84 ms 1266.15 ms 6.31 ms

App size

Revision Plain With Sentry Diff
3ab07e1 7.86 MiB 9.44 MiB 1.58 MiB
73ae3ba 7.86 MiB 9.44 MiB 1.58 MiB
b254a7e 7.86 MiB 9.44 MiB 1.58 MiB

@github-actions

github-actions Bot commented Jul 29, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Android Performance metrics 🚀

  Plain With Sentry Diff
Startup time 498.57 ms 560.86 ms 62.29 ms
Size 6.54 MiB 7.70 MiB 1.16 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
827bf09 475.40 ms 547.14 ms 71.74 ms
9b99523 456.91 ms 490.55 ms 33.64 ms
dbd526b 504.88 ms 569.02 ms 64.15 ms
793f4dc 462.68 ms 544.21 ms 81.53 ms
73a3c38 478.18 ms 526.62 ms 48.44 ms
73dca78 476.53 ms 522.21 ms 45.68 ms
0fb3800 465.64 ms 536.77 ms 71.13 ms
81f83eb 471.40 ms 522.14 ms 50.74 ms
6ba4675 499.80 ms 632.43 ms 132.63 ms
575ebaa 478.00 ms 585.76 ms 107.76 ms

App size

Revision Plain With Sentry Diff
827bf09 6.54 MiB 7.53 MiB 1015.27 KiB
9b99523 6.54 MiB 7.69 MiB 1.15 MiB
dbd526b 6.54 MiB 7.69 MiB 1.15 MiB
793f4dc 6.54 MiB 7.69 MiB 1.15 MiB
73a3c38 6.54 MiB 7.69 MiB 1.15 MiB
73dca78 6.54 MiB 7.69 MiB 1.15 MiB
0fb3800 6.54 MiB 7.69 MiB 1.15 MiB
81f83eb 6.54 MiB 7.69 MiB 1.15 MiB
6ba4675 6.54 MiB 7.53 MiB 1015.26 KiB
575ebaa 6.54 MiB 7.69 MiB 1.15 MiB

Previous results on branch: fix/screenshot-status-equality

Startup times

Revision Plain With Sentry Diff
3ab07e1 472.26 ms 551.67 ms 79.41 ms
73ae3ba 483.87 ms 621.00 ms 137.13 ms
b254a7e 472.41 ms 544.43 ms 72.01 ms

App size

Revision Plain With Sentry Diff
3ab07e1 6.54 MiB 7.70 MiB 1.16 MiB
73ae3ba 6.54 MiB 7.70 MiB 1.16 MiB
b254a7e 6.54 MiB 7.70 MiB 1.16 MiB

vaind
vaind approved these changes Jul 31, 2025
View reviewed changes

@vaind vaind left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The changes to the equality operator are OK. However, in order to address #3110 (comment) we should change the condition - instead of checking equality, I think we should check for the absolute diff on the dimensions instead. The issue is that these are float/double so even with the new operator, they would not necessarily be equal. I think adding another "close-to" check on dimensions, after checking for equality of state would further decrease the chance of hitting reconfiguration when nothing really changes.

@buenaflor

buenaflor commented Jul 31, 2025

Copy link
Copy Markdown
Contributor Author

thx will have a look at updating it

cursor[bot]

This comment was marked as outdated.

Sign in to view

cursor[bot]

This comment was marked as outdated.

Sign in to view

vaind
vaind requested changes Jul 31, 2025
View reviewed changes
Comment thread flutter/lib/src/screenshot/sentry_screenshot_widget.dart Outdated
@buenaflor

buenaflor commented Jul 31, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

I agree with Cursor on this - I think we should add additional function, something like bool matches(SentryScreenshotWidgetStatus other) and either keep the original equality operator you've done before these recent changes and use both, or just remove the equality operator to avoid potential issues down the road.

yep, did exactly that right now, will remove the original equality as well

@buenaflor
buenaflor merged commit 765aa8b into main Jul 31, 2025
70 checks passed
@buenaflor
buenaflor deleted the fix/screenshot-status-equality branch July 31, 2025 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cursor cursor[bot] cursor[bot] left review comments
@vaind vaind vaind approved these changes
@stefanosiano stefanosiano Awaiting requested review from stefanosiano
@denrase denrase Awaiting requested review from denrase denrase is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@buenaflor @vaind