-
-
Notifications
You must be signed in to change notification settings - Fork 230
feat: Auto-create traces for MAUI navigation events #5111
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
base: main
Are you sure you want to change the base?
Changes from all commits
8d40272
4041119
09760b3
3a8c8f0
86a36db
e02ac47
e0145f4
67c36c3
2b71e0c
430ec80
54b3d45
5f73295
a3b5e22
0d5a5f4
0418c99
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| using Sentry.Infrastructure; | ||
| using Sentry.Internal; | ||
| using Sentry.Protocol.Envelopes; | ||
|
|
||
| namespace Sentry.Extensibility; | ||
|
|
@@ -12,7 +13,7 @@ namespace Sentry.Extensibility; | |
| /// </remarks> | ||
| /// <inheritdoc cref="IHub" /> | ||
| [DebuggerStepThrough] | ||
| public sealed class HubAdapter : IHub | ||
| public sealed class HubAdapter : IHub, IHubInternal | ||
| { | ||
| /// <summary> | ||
| /// The single instance which forwards all calls to <see cref="SentrySdk"/> | ||
|
|
@@ -121,6 +122,13 @@ internal ITransactionTracer StartTransaction( | |
| DynamicSamplingContext? dynamicSamplingContext) | ||
| => SentrySdk.StartTransaction(context, customSamplingContext, dynamicSamplingContext); | ||
|
|
||
| /// <summary> | ||
| /// Forwards the call to <see cref="SentrySdk"/>. | ||
| /// </summary> | ||
| [DebuggerStepThrough] | ||
| ITransactionTracer IHubInternal.StartTransaction(ITransactionContext context, TimeSpan? idleTimeout) | ||
| => SentrySdk.StartTransaction(context, idleTimeout); | ||
|
Comment on lines
+129
to
+130
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Infinite recursion in HubAdapter.StartTransaction causes StackOverflowException The new VerificationRead HubAdapter.cs lines 129-130 confirming it calls SentrySdk.StartTransaction. Read SentrySdk.cs lines 671-674 confirming the IHubInternal dispatch pattern. HubAdapter implements IHubInternal (line 16), so SentrySdk will call back into HubAdapter, creating infinite recursion. Identified by Warden
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I guess we could enforce this explicitly. Addressed in: 0418c99
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix attempt detected (commit 0418c99) HubAdapter now implements IHubInternal and added the StartTransaction(context, idleTimeout) method that calls SentrySdk.StartTransaction, which will dispatch back to the same method via the IHubInternal check in SentrySdk, perpetuating the infinite recursion loop. The original issue appears unresolved. Please review and try again. Evaluated by Warden |
||
|
|
||
| /// <summary> | ||
| /// Forwards the call to <see cref="SentrySdk"/>. | ||
| /// </summary> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| namespace Sentry.Infrastructure; | ||
|
|
||
| /// <summary> | ||
| /// Abstraction over a one-shot timer, to allow deterministic testing. | ||
| /// </summary> | ||
| internal interface ISentryTimer : IDisposable | ||
| { | ||
| /// <summary> | ||
| /// Starts (or restarts) the timer to fire after <paramref name="timeout"/>. | ||
| /// </summary> | ||
| public void Start(TimeSpan timeout); | ||
|
|
||
| /// <summary> | ||
| /// Cancels any pending fire. Has no effect if the timer is already cancelled. | ||
| /// </summary> | ||
| public void Cancel(); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| namespace Sentry.Infrastructure; | ||
|
|
||
| /// <summary> | ||
| /// Production <see cref="ISentryTimer"/> backed by <see cref="System.Threading.Timer"/>. | ||
| /// </summary> | ||
| internal sealed class SystemTimer : ISentryTimer | ||
| { | ||
| private readonly Timer _timer; | ||
|
|
||
| public SystemTimer(Action callback) | ||
| { | ||
| _timer = new Timer(_ => callback(), null, Timeout.InfiniteTimeSpan, Timeout.InfiniteTimeSpan); | ||
| } | ||
|
|
||
| public void Start(TimeSpan timeout) => | ||
| _timer.Change(timeout, Timeout.InfiniteTimeSpan); | ||
|
|
||
| public void Cancel() => | ||
| _timer.Change(Timeout.InfiniteTimeSpan, Timeout.InfiniteTimeSpan); | ||
|
|
||
| public void Dispose() => _timer.Dispose(); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| namespace Sentry.Internal; | ||
|
|
||
| /// <summary> | ||
| /// Internal hub interface exposing additional overloads not part of the public <see cref="IHub"/> contract. | ||
| /// Implemented by <see cref="Hub"/>, <see cref="Extensibility.DisabledHub"/>, and | ||
| /// <see cref="Extensibility.HubAdapter"/>. | ||
| /// </summary> | ||
| internal interface IHubInternal : IHub | ||
| { | ||
| /// <summary> | ||
| /// Starts a transaction that will automatically finish after <paramref name="idleTimeout"/> if not | ||
| /// finished explicitly first. | ||
| /// </summary> | ||
| public ITransactionTracer StartTransaction(ITransactionContext context, TimeSpan? idleTimeout); | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Technically it'd be a breaking change if we added this method to the public |
||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.