Skip to content

Conversation

philipphofmann
Copy link
Member

📜 Description

Fix an edge case leading to flush continuing to time out when it timed out once before the HTTP transport finished sending.

💡 Motivation and Context

Came up while investigating #5421

💚 How did you test it?

Unit tests.

📝 Checklist

You have to check all boxes before merging:

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

Fix an edge case leading to flush continuing to time out when it timed
out once before the HTTP transport finished sending.
Copy link

codecov bot commented Jul 3, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.129%. Comparing base (9a17767) to head (b6ec152).
Report is 5 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #5565       +/-   ##
=============================================
- Coverage   86.552%   86.129%   -0.423%     
=============================================
  Files          417       417               
  Lines        35478     35442       -36     
  Branches     15342     15014      -328     
=============================================
- Hits         30707     30526      -181     
- Misses        4728      4875      +147     
+ Partials        43        41        -2     
Files with missing lines Coverage Δ
Sources/Sentry/SentryHttpTransport.m 98.207% <100.000%> (+0.039%) ⬆️

... and 29 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9a17767...b6ec152. Read the comment docs.

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

@philipphofmann philipphofmann marked this pull request as draft July 3, 2025 08:38
@philipphofmann
Copy link
Member Author

Converting back to draft because the unit tests are failing.

Copy link
Contributor

github-actions bot commented Jul 3, 2025

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1211.79 ms 1244.33 ms 32.55 ms
Size 23.75 KiB 874.29 KiB 850.54 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
b714cb9 1224.71 ms 1238.04 ms 13.33 ms
55f739c 1226.06 ms 1248.78 ms 22.71 ms
65f8d2e 1221.15 ms 1243.96 ms 22.81 ms
32e7197 1226.91 ms 1245.48 ms 18.56 ms
d7461dc 1233.69 ms 1255.29 ms 21.60 ms
b0e13a7 1227.71 ms 1245.88 ms 18.16 ms
f92cfa9 1228.45 ms 1251.33 ms 22.88 ms
2609f7a 1218.17 ms 1241.34 ms 23.17 ms
e18d392 1228.69 ms 1244.43 ms 15.73 ms
e64d3d4 1241.90 ms 1260.10 ms 18.20 ms

App size

Revision Plain With Sentry Diff
b714cb9 23.75 KiB 858.69 KiB 834.93 KiB
55f739c 23.75 KiB 858.73 KiB 834.98 KiB
65f8d2e 23.74 KiB 872.67 KiB 848.93 KiB
32e7197 23.75 KiB 866.69 KiB 842.94 KiB
d7461dc 23.75 KiB 874.45 KiB 850.70 KiB
b0e13a7 23.75 KiB 860.98 KiB 837.23 KiB
f92cfa9 23.75 KiB 855.38 KiB 831.62 KiB
2609f7a 23.75 KiB 867.04 KiB 843.29 KiB
e18d392 23.75 KiB 866.68 KiB 842.93 KiB
e64d3d4 23.75 KiB 855.37 KiB 831.62 KiB

Previous results on branch: fix/flushing-request-times-out

Startup times

Revision Plain With Sentry Diff
54cc390 1229.28 ms 1247.92 ms 18.64 ms
3965892 1229.72 ms 1249.86 ms 20.13 ms

App size

Revision Plain With Sentry Diff
54cc390 23.75 KiB 874.30 KiB 850.55 KiB
3965892 23.75 KiB 874.30 KiB 850.55 KiB

@philipphofmann philipphofmann marked this pull request as ready for review July 7, 2025 07:50
Copy link
Member

@armcknight armcknight left a comment

Choose a reason for hiding this comment

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

LGTM with small feedback items.

@philipphofmann philipphofmann merged commit f1b3d71 into main Jul 17, 2025
147 of 156 checks passed
@philipphofmann philipphofmann deleted the fix/flushing-request-times-out branch July 17, 2025 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants