feat: Close session for unhandled error

[brustolin]

📜 Description

Closes session as crashed for hybrids SDK when a new envelope with unhandled crashed event is captured, and start a new session.

💡 Motivation and Context

closes #2593

💚 How did you test it?

📝 Checklist

You have to check all boxes before merging:

• I reviewed the submitted code.
• 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.
• 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.

#skip-changelog

[github-actions]

	Messages
📖	Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against 1c55ca4

[codecov]

Codecov Report

Merging #3091 (1c55ca4) into main (60dd0f5) will increase coverage by 0.001%.
The diff coverage is 93.846%.

Additional details and impacted files

@@              Coverage Diff              @@
##              main     #3091       +/-   ##
=============================================
+ Coverage   88.958%   88.960%   +0.001%     
=============================================
  Files          495       495               
  Lines        53464     53545       +81     
  Branches     19139     19164       +25     
=============================================
+ Hits         47561     47634       +73     
- Misses        4944      4950        +6     
- Partials       959       961        +2     

Impacted Files	Coverage Δ
Sources/Sentry/SentrySerialization.m	88.590% <57.142%> (-1.551%)	⬇️
Sources/Sentry/SentryHub.m	98.454% <94.285%> (-0.375%)	⬇️
Tests/SentryTests/SentryHubTests.swift	98.283% <100.000%> (+0.069%)	⬆️

... and 7 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 60dd0f5...1c55ca4. Read the comment docs.

[github-actions]

Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	1239.59 ms	1244.27 ms	4.68 ms
Size	20.76 KiB	393.77 KiB	373.01 KiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
e84bc3f	1201.49 ms	1232.82 ms	31.33 ms
407ff99	1225.49 ms	1232.88 ms	7.39 ms
9faf217	1268.86 ms	1274.82 ms	5.96 ms
15b8c61	1223.16 ms	1244.83 ms	21.67 ms
43aa39d	1239.16 ms	1270.42 ms	31.26 ms
60dd0f5	1212.24 ms	1240.82 ms	28.58 ms
f0737f6	1220.43 ms	1236.44 ms	16.01 ms
f938d24	1223.26 ms	1242.12 ms	18.86 ms
b2f82fa	1237.78 ms	1256.02 ms	18.24 ms
25a5e8b	1249.18 ms	1268.42 ms	19.24 ms

App size

Revision	Plain	With Sentry	Diff
e84bc3f	20.76 KiB	434.72 KiB	413.96 KiB
407ff99	20.76 KiB	427.87 KiB	407.10 KiB
9faf217	20.76 KiB	419.70 KiB	398.94 KiB
15b8c61	20.76 KiB	419.67 KiB	398.91 KiB
43aa39d	20.76 KiB	432.82 KiB	412.06 KiB
60dd0f5	20.76 KiB	393.37 KiB	372.61 KiB
f0737f6	20.76 KiB	437.12 KiB	416.36 KiB
f938d24	20.76 KiB	434.88 KiB	414.12 KiB
b2f82fa	20.76 KiB	419.62 KiB	398.86 KiB
25a5e8b	20.76 KiB	436.33 KiB	415.57 KiB

Previous results on branch: feat/closesession-forerror

Startup times

Revision	Plain	With Sentry	Diff
7eae871	1223.65 ms	1226.14 ms	2.49 ms
8541ffe	1228.92 ms	1237.72 ms	8.80 ms
3bd1370	1226.51 ms	1236.50 ms	9.99 ms
b267779	1196.93 ms	1239.13 ms	42.19 ms
80127d9	1219.98 ms	1236.58 ms	16.60 ms
ef9f754	1252.96 ms	1259.38 ms	6.42 ms

App size

Revision	Plain	With Sentry	Diff
7eae871	20.76 KiB	393.68 KiB	372.92 KiB
8541ffe	20.76 KiB	393.74 KiB	372.98 KiB
3bd1370	20.76 KiB	393.74 KiB	372.98 KiB
b267779	20.76 KiB	393.70 KiB	372.94 KiB
80127d9	20.76 KiB	393.68 KiB	372.92 KiB
ef9f754	20.76 KiB	393.70 KiB	372.94 KiB

[philipphofmann]

Thanks for doing this @brustolin.

[philipphofmann]

h: That's not how we safely end and start a new session. I think we should call the following instead.

[self endSessionWithTimestamp:[_currentDateProvider date]];
[hub startSession];

As we also do in the session tracker

sentry-cocoa/Sources/Sentry/SentrySessionTracker.m

Lines 173 to 174 in 60dd0f5

	[hub endSessionWithTimestamp:self.lastInForeground];
	[hub startSession];

Or am I missing something?

[brustolin]

Maybe I'm missing something, but this is what I get:

• currentSession is a clone of the current session, If a call self endSessionWithTimestamp this will not end currentSession.
• If I end it before cloning it I cannot clone it.
• By calling [hub endSession] the session is immediately captured, but here, the session was being sent in the same envelope.
• By calling startSession the last session is also capture, which would duplicate the capture

[philipphofmann]

Okay, then what I proposed doesn't seem correct. Your solution definitely needs a lock with @synchronized(_sessionLock) when creating the new session, you need to handle _errorsBeforeSession, you need to handle the edge case when there is no releaseName, and you need to store the new session and also capture the new session. Similar to what startSession does. I think it would be better to maybe refacrot startSession so you can also use it here.

Furthermore, I think it would make sense to not increment the session error when the event is unhandled. We should just mark it as crashed.

[brustolin]

Please, take a look at the new approach.

[philipphofmann]

That should work now 👍

[philipphofmann]

That comment is important, thanks 😃