Conversation
src/Sentry/GlobalSessionManager.cs
Outdated
|
|
||
| var persistedSessionUpdate = new PersistedSessionUpdate(update, pauseTimestamp); | ||
| persistedSessionUpdate.WriteToFile(filePath); | ||
| persistedSessionUpdate.WriteToFile(filePath, _options.DiagnosticLogger!); |
There was a problem hiding this comment.
We'll need to remove this !. No other way but to make it nullable I believe
| // The only location in the protocol we allow dynamic objects are Extra and Contexts | ||
| // In the event of an instance that can't be serialized, we don't want to throw away a whole event | ||
| // so we'll suppress issues here. | ||
| logger?.LogError("Failed to serialize object for property {0}", e, propertyName); |
There was a problem hiding this comment.
We could use the same approach @SimonCropp took and if logger is null) { throw;
There was a problem hiding this comment.
I went with catch Exception ex when (logger != null)
Codecov Report
@@ Coverage Diff @@
## main #1388 +/- ##
==========================================
- Coverage 82.73% 81.40% -1.34%
==========================================
Files 215 215
Lines 7165 7173 +8
Branches 1410 1411 +1
==========================================
- Hits 5928 5839 -89
- Misses 802 908 +106
+ Partials 435 426 -9
Continue to review full report at Codecov.
|
|
Ping @bruno-garcia for review since he's the PR owner :D |
|
Thank you @lucas-zimerman for fixing this PR |
| writer.WritePropertyName(propertyName); | ||
| writer.WriteDynamicValue(value, logger); | ||
| } | ||
| catch (Exception e) when (logger != null) |
There was a problem hiding this comment.
| catch (Exception e) when (logger != null) | |
| catch (Exception e) when (logger is not null) |
5 participants
Resolves #946