Skip to content

Commit

Permalink
fix: check crash callback for sessions (#1222)
Browse files Browse the repository at this point in the history
  • Loading branch information
@bruno-garcia
bruno-garcia authored Sep 29, 2021
1 parent 2cf2d02 commit 678944f
Show file tree
Hide file tree
Showing 5 changed files with 126 additions and 57 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## Unreleased

### Fixes

- Unity Android support: check for native crashes before closing session as Abnormal ([#1222](https://github.com/getsentry/sentry-dotnet/pull/1222))

## 3.9.3

### Fixes
Expand Down
65 changes: 27 additions & 38 deletions src/Sentry/GlobalSessionManager.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,7 @@ public GlobalSessionManager(

Directory.CreateDirectory(directoryPath);

_options.DiagnosticLogger?.LogDebug(
"Created directory for installation ID file ({0}).",
_options.DiagnosticLogger?.LogDebug("Created directory for installation ID file ({0}).",
directoryPath);

var filePath = Path.Combine(directoryPath, ".installation");
Expand All @@ -74,8 +73,7 @@ public GlobalSessionManager(
}
catch (FileNotFoundException)
{
_options.DiagnosticLogger?.LogDebug(
"File containing installation ID does not exist ({0}).",
_options.DiagnosticLogger?.LogDebug("File containing installation ID does not exist ({0}).",
filePath);
}
catch (DirectoryNotFoundException)
Expand All @@ -90,8 +88,7 @@ public GlobalSessionManager(
var id = Guid.NewGuid().ToString();
File.WriteAllText(filePath, id);

_options.DiagnosticLogger?.LogDebug(
"Saved installation ID '{0}' to file '{1}'.",
_options.DiagnosticLogger?.LogDebug("Saved installation ID '{0}' to file '{1}'.",
id, filePath);

return id;
Expand All @@ -100,9 +97,7 @@ public GlobalSessionManager(
// and let the next installation id strategy kick in
catch (Exception ex)
{
_options.DiagnosticLogger?.LogError(
"Failed to resolve persistent installation ID.",
ex);
_options.DiagnosticLogger?.LogError("Failed to resolve persistent installation ID.", ex);

return null;
}
Expand Down Expand Up @@ -133,9 +128,7 @@ public GlobalSessionManager(
}
catch (Exception ex)
{
_options.DiagnosticLogger?.LogError(
"Failed to resolve hardware installation ID.",
ex);
_options.DiagnosticLogger?.LogError("Failed to resolve hardware installation ID.", ex);

return null;
}
Expand Down Expand Up @@ -172,14 +165,11 @@ internal string GetMachineNameInstallationId() =>

if (!string.IsNullOrWhiteSpace(id))
{
_options.DiagnosticLogger?.LogDebug(
"Resolved installation ID '{0}'.",
id);
_options.DiagnosticLogger?.LogDebug("Resolved installation ID '{0}'.", id);
}
else
{
_options.DiagnosticLogger?.LogDebug(
"Failed to resolve installation ID.");
_options.DiagnosticLogger?.LogDebug("Failed to resolve installation ID.");
}

return _resolvedInstallationId = id;
Expand Down Expand Up @@ -221,11 +211,9 @@ private void PersistSession(SessionUpdate update, DateTimeOffset? pauseTimestamp

private void DeletePersistedSession()
{
_options.DiagnosticLogger?.LogDebug("Deleting persisted session file.");

if (string.IsNullOrWhiteSpace(_persistenceDirectoryPath))
{
_options.DiagnosticLogger?.LogDebug("Persistence directory is not set, returning.");
_options.DiagnosticLogger?.LogDebug("Persistence directory is not set, not deleting any persisted session file.");
return;
}

Expand All @@ -237,8 +225,7 @@ private void DeletePersistedSession()
{
try
{
_options.DiagnosticLogger?.LogDebug(
"Deleting persisted session file with contents: {0}",
_options.DiagnosticLogger?.LogDebug("Deleting persisted session file with contents: {0}",
File.ReadAllText(filePath));
}
catch (Exception ex)
Expand All @@ -252,9 +239,7 @@ private void DeletePersistedSession()

File.Delete(filePath);

_options.DiagnosticLogger?.LogInfo(
"Deleted persisted session file '{0}'.",
filePath);
_options.DiagnosticLogger?.LogInfo("Deleted persisted session file '{0}'.", filePath);
}
catch (Exception ex)
{
Expand All @@ -281,28 +266,35 @@ private void DeletePersistedSession()
var recoveredUpdate = _persistedSessionProvider(filePath);

// Create a session update to end the recovered session
return new SessionUpdate(
var sessionUpdate = new SessionUpdate(
recoveredUpdate.Update,
// We're recovering an ongoing session, so this can never be initial
false,
// If the session was paused, then use that as timestamp, otherwise use current timestamp
recoveredUpdate.PauseTimestamp ?? _clock.GetUtcNow(),
// Increment sequence number
recoveredUpdate.Update.SequenceNumber + 1,
// If the session was paused then end normally, otherwise abnormal or crashed
_options.CrashedLastRun switch
// If there's a callback for native crashes, check that first.
_options.CrashedLastRun?.Invoke() switch
{
// Native crash (if native SDK enabled):
true => SessionEndStatus.Crashed,
// Ended while on the background, healthy session:
_ when recoveredUpdate.PauseTimestamp is not null => SessionEndStatus.Exited,
{ } crashedLastRun => crashedLastRun() ? SessionEndStatus.Crashed : SessionEndStatus.Abnormal,
// Possibly out of battery, killed by OS or user, solar flare:
_ => SessionEndStatus.Abnormal
});

_options.DiagnosticLogger?.LogInfo("Recovered session: EndStatus: {0}. PauseTimestamp: {1}",
sessionUpdate.EndStatus,
recoveredUpdate.PauseTimestamp);

return sessionUpdate;
}
catch (IOException ioEx) when (ioEx is FileNotFoundException or DirectoryNotFoundException)
{
// Not a notable error
_options.DiagnosticLogger?.LogDebug(
"A persisted session does not exist at {0}.",
filePath);
_options.DiagnosticLogger?.LogDebug("A persisted session does not exist at {0}.", filePath);

return null;
}
Expand Down Expand Up @@ -348,8 +340,7 @@ private void DeletePersistedSession()
EndSession(previousSession, _clock.GetUtcNow(), SessionEndStatus.Exited);
}

_options.DiagnosticLogger?.LogInfo(
"Started new session (SID: {0}; DID: {1}).",
_options.DiagnosticLogger?.LogInfo("Started new session (SID: {0}; DID: {1}).",
session.Id, session.DistinctId);

var update = session.CreateUpdate(true, _clock.GetUtcNow());
Expand All @@ -361,8 +352,7 @@ private void DeletePersistedSession()

private SessionUpdate EndSession(Session session, DateTimeOffset timestamp, SessionEndStatus status)
{
_options.DiagnosticLogger?.LogInfo(
"Ended session (SID: {0}; DID: {1}) with status '{2}'.",
_options.DiagnosticLogger?.LogInfo("Ended session (SID: {0}; DID: {1}) with status '{2}'.",
session.Id, session.DistinctId, status);

var update = session.CreateUpdate(false, timestamp, status);
Expand All @@ -377,8 +367,7 @@ private SessionUpdate EndSession(Session session, DateTimeOffset timestamp, Sess
var session = Interlocked.Exchange(ref _currentSession, null);
if (session is null)
{
_options.DiagnosticLogger?.LogDebug(
"Failed to end session because there is none active.");
_options.DiagnosticLogger?.LogDebug("Failed to end session because there is none active.");

return null;
}
Expand Down
9 changes: 6 additions & 3 deletions src/Sentry/Integrations/AppDomainProcessExitIntegration.cs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using Sentry.Extensibility;
using Sentry.Internal;

namespace Sentry.Integrations
Expand All @@ -7,26 +8,28 @@ internal class AppDomainProcessExitIntegration : IInternalSdkIntegration
{
private readonly IAppDomain _appDomain;
private IHub? _hub;
private SentryOptions? _options;

public AppDomainProcessExitIntegration(IAppDomain? appDomain = null)
{
_appDomain = appDomain ?? AppDomainAdapter.Instance;
}
=> _appDomain = appDomain ?? AppDomainAdapter.Instance;

public void Register(IHub hub, SentryOptions options)
{
_hub = hub;
_options = options;
_appDomain.ProcessExit += HandleProcessExit;
}

public void Unregister(IHub hub)
{
_appDomain.ProcessExit -= HandleProcessExit;
_hub = null;
_options = null;
}

internal void HandleProcessExit(object? sender, EventArgs e)
{
_options?.DiagnosticLogger?.LogInfo("AppDomain process exited: Disposing SDK.");
(_hub as IDisposable)?.Dispose();
}
}
Expand Down
20 changes: 5 additions & 15 deletions src/Sentry/Internal/Hub.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,9 +183,7 @@ public void StartSession()
}
catch (Exception ex)
{
_options.DiagnosticLogger?.LogError(
"Failed to recover persisted session.",
ex);
_options.DiagnosticLogger?.LogError("Failed to recover persisted session.", ex);
}
}

Expand All @@ -200,9 +198,7 @@ public void StartSession()
}
catch (Exception ex)
{
_options.DiagnosticLogger?.LogError(
"Failed to start a session.",
ex);
_options.DiagnosticLogger?.LogError("Failed to start a session.", ex);
}
}

Expand All @@ -216,9 +212,7 @@ public void PauseSession()
}
catch (Exception ex)
{
_options.DiagnosticLogger?.LogError(
"Failed to pause a session.",
ex);
_options.DiagnosticLogger?.LogError("Failed to pause a session.", ex);
}
}
}
Expand All @@ -236,9 +230,7 @@ public void ResumeSession()
}
catch (Exception ex)
{
_options.DiagnosticLogger?.LogError(
"Failed to resume a session.",
ex);
_options.DiagnosticLogger?.LogError("Failed to resume a session.", ex);
}
}
}
Expand All @@ -255,9 +247,7 @@ private void EndSession(DateTimeOffset timestamp, SessionEndStatus status)
}
catch (Exception ex)
{
_options.DiagnosticLogger?.LogError(
"Failed to end a session.",
ex);
_options.DiagnosticLogger?.LogError("Failed to end a session.", ex);
}
}

Expand Down
83 changes: 82 additions & 1 deletion test/Sentry.Tests/GlobalSessionManagerTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ private class Fixture : IDisposable

public ISystemClock Clock { get; }

public Func<string, PersistedSessionUpdate> PersistedSessionProvider { get; }
public Func<string, PersistedSessionUpdate> PersistedSessionProvider { get; set; }

public Fixture(Action<SentryOptions> configureOptions = null)
{
Expand Down Expand Up @@ -355,6 +355,71 @@ public void TryGetPersistentInstallationId_SessionStarted_CrashDelegateReturnsFa
persistedSessionUpdate!.EndStatus.Should().Be(SessionEndStatus.Abnormal);
}

[Fact]
public void TryGetPersistentInstallationId_CrashDelegateReturnsTrueWithPauseTimestamp_EndsAsCrashed()
{
// Arrange
_fixture.Options.CrashedLastRun = () => true;
// Session was paused before persisted:
var pausedTimestamp = DateTimeOffset.Now;
_fixture.PersistedSessionProvider = _ => new PersistedSessionUpdate(
AnySessionUpdate(),
pausedTimestamp);

var sut = _fixture.GetSut();

// Act
var persistedSessionUpdate = sut.TryRecoverPersistedSession();

// Assert
persistedSessionUpdate.Should().NotBeNull();
persistedSessionUpdate!.EndStatus.Should().Be(SessionEndStatus.Crashed);
}

[Fact]
public void TryGetPersistentInstallationId_CrashDelegateIsNullWithPauseTimestamp_EndsAsExited()
{
// Arrange
_fixture.Options.CrashedLastRun = null;
// Session was paused before persisted:
var pausedTimestamp = DateTimeOffset.Now;
_fixture.PersistedSessionProvider = _ => new PersistedSessionUpdate(
AnySessionUpdate(),
pausedTimestamp);

var sut = _fixture.GetSut();

// Act
var persistedSessionUpdate = sut.TryRecoverPersistedSession();

// Assert
persistedSessionUpdate.Should().NotBeNull();
persistedSessionUpdate!.EndStatus.Should().Be(SessionEndStatus.Exited);
}

[Fact]
public void TryGetPersistentInstallationId_CrashDelegateIsNullWithoutPauseTimestamp_EndsAsAbnormal()
{
// Arrange
_fixture.Options.CrashedLastRun = null;
var pausedTimestamp = DateTimeOffset.Now;
_fixture.PersistedSessionProvider = _ => new PersistedSessionUpdate(
AnySessionUpdate(),
// No pause timestamp:
null);

var sut = _fixture.GetSut();

sut.StartSession();

// Act
var persistedSessionUpdate = sut.TryRecoverPersistedSession();

// Assert
persistedSessionUpdate.Should().NotBeNull();
persistedSessionUpdate!.EndStatus.Should().Be(SessionEndStatus.Abnormal);
}

[Fact]
public void TryGetPersistentInstallationId_SessionStarted_CrashDelegateReturnsTrue_EndsAsCrashed()
{
Expand Down Expand Up @@ -407,5 +472,21 @@ public void TryGetPersistentInstallationId_SessionEnded_ReturnsNull()
// Assert
persistedSessionUpdate.Should().BeNull();
}

// A session update (of which the state doesn't matter for the test):
private static SessionUpdate AnySessionUpdate()
=> new(
SentryId.Create(),
"did",
DateTimeOffset.Now,
"release",
"env",
"ip",
"ua",
0,
true,
DateTimeOffset.Now,
1,
null);
}
}

0 comments on commit 678944f

Please sign in to comment.