Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor page navigation #108

Merged
merged 7 commits into from
Apr 6, 2022
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

### Unreleased

### Features

- Add option extension: RemoveNavigationPageIntegration ([#108](https://github.com/getsentry/sentry-xamarin/pull/108))

### Fixes

- Ignore null data on Internal breadcrumbs ([#90](https://github.com/getsentry/sentry-xamarin/pull/90))
Expand Down
2 changes: 1 addition & 1 deletion Samples/Sample.Xamarin.Droid/MainActivity.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ protected override void OnCreate(Bundle savedInstanceState)
{
SentryXamarin.Init(options =>
{
options.Dsn = "https://5a193123a9b841bc8d8e42531e7242a1@o447951.ingest.sentry.io/5560112";
lucas-zimerman marked this conversation as resolved.
Show resolved Hide resolved
options.Dsn = "https://5a193123a9b841bc8d8e42531e7242a1@sentry.io/5560112";
options.AddXamarinFormsIntegration();
options.Debug = true;
options.AttachScreenshots = true;
Expand Down
15 changes: 15 additions & 0 deletions Src/Sentry.Xamarin.Forms/Extensibility/DisabledNavigationPage.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
using Sentry.Xamarin.Internals;

namespace Sentry.Xamarin.Forms.Extensibility
{
internal class DisabledNavigationPage : IPageNavigationTracker
{
public static DisabledNavigationPage Instance = new();

public string CurrentPage => null;

public void Register(IHub hub, SentryOptions options) { }

public void Unregister() { }
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Sentry.Xamarin.Forms.Internals;
using Sentry.Xamarin.Forms.Extensibility;
using Sentry.Xamarin.Forms.Internals;

namespace Sentry
{
Expand All @@ -15,8 +16,32 @@ public static partial class SentryXamarinOptionsExtensions
/// <param name="options">The Sentry Xamarion Options.</param>
public static void AddXamarinFormsIntegration(this SentryXamarinOptions options)
{
options.AddPageNavigationTrackerIntegration(new SentryXamarinFormsIntegration());
var applicationListener = new FormsApplicationListener(options);

var formsIntegration = new SentryXamarinFormsIntegration(options);
options.AddIntegration(formsIntegration);
applicationListener.AddListener(formsIntegration.RegisterRequestThemeChange);

if (options.PageTracker is null)
{
var navigationIntegration = new FormsNavigationIntegration();
options.AddPageNavigationTrackerIntegration(navigationIntegration);
applicationListener.AddListener(navigationIntegration.ApplySentryNavigationEvents);
}

options.ProtocolPackageName = ProtocolPackageName;

applicationListener.Invoke();
}

/// <summary>
/// Disables the default integration that registers the page and popup navigation.
/// </summary>
/// <param name="options">The Sentry Xamarion Options.</param>
public static void RemoveNavigationPageIntegration(this SentryXamarinOptions options)
{
options.PageTracker?.Unregister();
options.PageTracker = DisabledNavigationPage.Instance;
}
}
}
63 changes: 63 additions & 0 deletions Src/Sentry.Xamarin.Forms/Internals/FormsApplicationListener.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
using System;
using System.Collections.Generic;
using System.Threading.Tasks;
using Xamarin.Forms;

namespace Sentry.Xamarin.Forms.Internals
{
internal class FormsApplicationListener
{
private List<Action<Application>> _listeners { get; } = new List<Action<Application>>();

private SentryXamarinOptions _options { get; }

/// <summary>
/// Class that returns the current application to any listener once available.
/// </summary>
/// <param name="options"> The sentry Xamarin options.</param>
public FormsApplicationListener(SentryXamarinOptions options) => _options = options;

/// <summary>
/// Adds a callback to a function that requires the Application.
/// </summary>
/// <param name="listener"> A function that requires the application in order to work properly.</param>
public void AddListener(Action<Application> listener) => _listeners.Add(listener);

/// <summary>
/// Inokes a task that will wait for the initialization of Application.Current.
/// On Success, it'll return the current application to all registered listeners.
/// </summary>
public void Invoke() =>
//Don't lock the main Thread while waiting for the current application to be created.
Task.Run(async () =>
Copy link
Contributor

Choose a reason for hiding this comment

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

Yikes. I see this was in the old code and you are just refactoring, but we really shouldn't completely lose this task. It should be saved somewhere. We should also register it for cancellation so it can be shut down gracefully. I'm OK if this is done in a future PR, but wanted to point it out.

{
var application = await GetCurrentApplication().ConfigureAwait(false);
if (application is null)
{
_options.DiagnosticLogger?.Log(SentryLevel.Warning, "Sentry.Xamarin.Forms timeout for tracking Application.Current. Navigation tracking is going to be disabled");
return;
}

foreach(var hook in _listeners)
{
hook.Invoke(application);
}
_listeners.Clear();
});

/// <summary>
/// Get the current Application.
/// If SentrySDK was initialized from the Native project (Android/IOS) the Application might not have been created in time.
/// So we wait for max 7 seconds to see check if it was created or not
/// </summary>
/// <returns>Current application.</returns>
private async Task<Application> GetCurrentApplication()
{
for (int i = 0; i < _options.GetCurrentApplicationMaxRetries && Application.Current is null; i++)
{
await Task.Delay(_options.GetCurrentApplicationDelay).ConfigureAwait(false);
}
return Application.Current;
}
}
}
94 changes: 94 additions & 0 deletions Src/Sentry.Xamarin.Forms/Internals/FormsNavigationIntegration.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
using Sentry.Xamarin.Forms.Extensions;
using Sentry.Xamarin.Internals;
using System.Collections.Generic;
using Xamarin.Forms;

namespace Sentry.Xamarin.Forms.Internals
{
internal class FormsNavigationIntegration : IPageNavigationTracker
{
public string CurrentPage { get; private set; }
private IHub _hub { get; set; }

private bool _disabled { get; set; }
lucas-zimerman marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Registers the Page appearing and disappearing event to the given application..
/// </summary>
/// <param name="application">The Xamarin Application.</param>
internal void ApplySentryNavigationEvents(Application application)
{
if (!_disabled)
{
application.PageAppearing += Current_PageAppearing;
application.PageDisappearing += Current_PageDisappearing;
}
}

public void Register(IHub hub, SentryOptions _) => _hub = hub;

/// <summary>
/// Removes the PageDissapearing and PageAppearing events from the main Application, if found.
/// </summary>
public void Unregister()
{
if (!_disabled)
{
_disabled = true;
lucas-zimerman marked this conversation as resolved.
Show resolved Hide resolved
if (Application.Current is not null)
{
Application.Current.PageAppearing -= Current_PageAppearing;
Application.Current.PageDisappearing -= Current_PageDisappearing;
}
}
}

private void Current_PageDisappearing(object sender, Page e)
{
var type = e.GetPageType();
if (type.BaseType.StartsWith("PopupPage"))
{
_hub?.AddBreadcrumb(null,
"ui.lifecycle",
"navigation",
new Dictionary<string, string>
{
["popup"] = type.Name,
["state"] = "disappearing"
}, level: BreadcrumbLevel.Info);
}
}

private void Current_PageAppearing(object sender, Page e)
{
var pageType = e.GetPageType();
if (CurrentPage != null && CurrentPage != pageType.Name)
{
if (pageType.Name is "NavigationPage")
{
return;
}
if (pageType.BaseType is "PopupPage")
{
_hub?.AddBreadcrumb(null,
"ui.lifecycle",
"navigation",
new Dictionary<string, string>
{
["popup"] = pageType.Name,
["state"] = "appearing"
}, level: BreadcrumbLevel.Info);
return;
}
else
{
_hub?.AddBreadcrumb(null,
"navigation",
"navigation",
new Dictionary<string, string> { { "from", $"/{CurrentPage}" }, { "to", $"/{pageType.Name}" } });
}
}
CurrentPage = pageType.Name;
}
}
}
120 changes: 20 additions & 100 deletions Src/Sentry.Xamarin.Forms/Internals/SentryXamarinFormsIntegration.cs
Original file line number Diff line number Diff line change
@@ -1,25 +1,19 @@
using Sentry.Extensions;
using Sentry.Xamarin.Forms.Extensions;
using Sentry.Xamarin.Internals;
using Sentry.Integrations;
using System.Collections.Generic;
using System.Threading.Tasks;
using Xamarin.Forms;
using Xamarin.Forms.Internals;

namespace Sentry.Xamarin.Forms.Internals
{
internal class SentryXamarinFormsIntegration : IPageNavigationTracker
internal class SentryXamarinFormsIntegration : ISdkIntegration
{
internal static SentryXamarinFormsIntegration Instance;
private SentryXamarinOptions _options;
private DelegateLogListener _xamarinLogger;
private IHub _hub;
internal static SentryXamarinFormsIntegration Instance { get; set; }
private SentryXamarinOptions _options { get; }
private DelegateLogListener _xamarinLogger { get; set; }
private IHub _hub { get; set; }
lucas-zimerman marked this conversation as resolved.
Show resolved Hide resolved

public string CurrentPage { get; private set; }
public void RegisterXamarinOptions(SentryXamarinOptions options)
{
_options = options;
}
public SentryXamarinFormsIntegration(SentryXamarinOptions options) => _options = options;

/// <summary>
/// Register the Sentry Xamarin Forms SDK on Sentry.NET SDK
Expand All @@ -37,38 +31,29 @@ public void Register(IHub hub, SentryOptions options)
_hub = hub;

RegisterXamarinFormsLogListener(hub);

//Don't lock the main Thread while waiting for the current application to be created.
Task.Run(async () =>
lucas-zimerman marked this conversation as resolved.
Show resolved Hide resolved
{
var application = await GetCurrentApplication().ConfigureAwait(false);
if (application is null)
{
options.DiagnosticLogger?.Log(SentryLevel.Warning, "Sentry.Xamarin.Forms timeout for tracking Application.Current. Navigation tracking is going to be disabled");
}
else
{
application.PageAppearing += Current_PageAppearing;
application.PageDisappearing += Current_PageDisappearing;
application.RequestedThemeChanged += Current_RequestedThemeChanged;
}
});
}

/// <summary>
/// creates breadcrumbs from events received from RequestedThemeChanged on the given application.
/// </summary>
/// <param name="application">The Xamarin Application.</param>
internal void RegisterRequestThemeChange(Application application)
=> application.RequestedThemeChanged += Current_RequestedThemeChanged;

internal void RegisterXamarinFormsLogListener(IHub hub)
{
_xamarinLogger = new DelegateLogListener((arg1, arg2) =>
_xamarinLogger = new DelegateLogListener((logger, issue) =>
{
if (_options.XamarinLoggerEnabled)
{
hub.AddInternalBreadcrumb(_options,
hub?.AddInternalBreadcrumb(_options,
null,
"xamarin",
"info",
new Dictionary<string, string>
{
["logger"] = arg1,
["issue"] = arg2
["logger"] = logger,
["issue"] = issue
}, level: BreadcrumbLevel.Warning);
}
});
Expand All @@ -79,72 +64,7 @@ internal void RegisterXamarinFormsLogListener(IHub hub)
}
}

/// <summary>
/// Gets the current Application.
/// If SentrySDK was initialized from the Native project (Android/IOS) the Application might not have been created in time.
/// So we wait for max 7 seconds to see check if it was created or not
/// </summary>
/// <returns>Current application.</returns>
private async Task<Application> GetCurrentApplication()
{
for (int i = 0; i < _options.GetCurrentApplicationMaxRetries && Application.Current is null; i++)
{
await Task.Delay(_options.GetCurrentApplicationDelay).ConfigureAwait(false);
}
return Application.Current;
}

private void Current_RequestedThemeChanged(object sender, AppThemeChangedEventArgs e)
{
_hub.AddBreadcrumb(e.RequestedTheme.ToString(), "AppTheme.Change", level: BreadcrumbLevel.Info);
}

private void Current_PageDisappearing(object sender, Page e)
lucas-zimerman marked this conversation as resolved.
Show resolved Hide resolved
{
var type = e.GetPageType();
if (type.BaseType.StartsWith("PopupPage"))
{
_hub.AddBreadcrumb(null,
"ui.lifecycle",
"navigation",
new Dictionary<string, string>
{
["popup"] = type.Name,
["state"] = "disappearing"
}, level: BreadcrumbLevel.Info);
}
}

private void Current_PageAppearing(object sender, Page e)
{
var pageType = e.GetPageType();
if (CurrentPage != null && CurrentPage != pageType.Name)
{
if (pageType.Name is "NavigationPage")
{
return;
}
if (pageType.BaseType is "PopupPage")
{
_hub.AddBreadcrumb(null,
"ui.lifecycle",
"navigation",
new Dictionary<string, string>
{
["popup"] = pageType.Name,
["state"] = "appearing"
}, level: BreadcrumbLevel.Info);
return;
}
else
{
_hub.AddBreadcrumb(null,
"navigation",
"navigation",
new Dictionary<string, string> { { "from", $"/{CurrentPage}" }, { "to", $"/{pageType.Name}" } });
}
}
CurrentPage = pageType.Name;
}
private void Current_RequestedThemeChanged(object sender, AppThemeChangedEventArgs themeEvent)
=> _hub?.AddBreadcrumb(themeEvent.RequestedTheme.ToString(), "AppTheme.Change", level: BreadcrumbLevel.Info);
}
}
Loading