Manually handle sending of crashes #43
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Added `HandleCrashes` and `WriteCrashes` properties to `ApplicationInsightsProvider` for better control over crash handling and reporting. Updated crash handling logic to use `HandleCrashes`. Made `SendCrashes` method public and added `HasCrashed` method. Updated `IInsights` and `IInsightsProvider` interfaces with new methods and properties. Modified `Insights` class to implement new interface methods and added logic to check and send crashes.
A new method `ResetCrashes` has been added to the `ApplicationInsightsProvider` class, changing its access modifier from private to public. The `IInsights` and `IInsightsProvider` interfaces have been updated to include the new `ResetCrashes` method. The `Insights` class has been modified to implement the `ResetCrashes` method, which iterates through all `insightsProviders` and calls their `ResetCrashes` method.
IeuanWalker
commented
Dec 17, 2024
| return; | ||
| } | ||
|
|
||
| if(!IsTrackPageViewsEnabled) |
There was a problem hiding this comment.
This check is missing, so regardless what happens to this PR, this should get added
|
I really like this! I added two comments on your comment. Can you rebase against the latest code in main and make sure it works with that one too? It should, because GitHub says it can merge, but it would be great if you can to it to verify that it works as it should. |
Added `HandleCrashes` and `WriteCrashes` properties to `ApplicationInsightsProvider` for better control over crash handling and reporting. Updated crash handling logic to use `HandleCrashes`. Made `SendCrashes` method public and added `HasCrashed` method. Updated `IInsights` and `IInsightsProvider` interfaces with new methods and properties. Modified `Insights` class to implement new interface methods and added logic to check and send crashes.
A new method `ResetCrashes` has been added to the `ApplicationInsightsProvider` class, changing its access modifier from private to public. The `IInsights` and `IInsightsProvider` interfaces have been updated to include the new `ResetCrashes` method. The `Insights` class has been modified to implement the `ResetCrashes` method, which iterates through all `insightsProviders` and calls their `ResetCrashes` method.
This commit focuses on enhancing the readability of the codebase by adding spaces after keywords and before parentheses in conditional statements. Key changes include: - Standardizing formatting in the `Initialize` method and various error handling methods. - Improving the clarity of global properties handling and telemetry client creation. - Ensuring consistent spacing in loops and conditionals throughout the code. These adjustments aim to maintain the existing functionality while making the code more maintainable and easier to read.
Updated `foreach` statements for consistent spacing and enhanced readability. Adjusted null check handling for the `properties` parameter in `TrackErrorAsync` to improve clarity.
Removed the `HandleCrashes` property and replaced it with `IsTrackCrashesEnabled` in both the `ApplicationInsightsProvider` class and the `IInsightsProvider` interface. Updated exception handling logic to use `IsTrackCrashesEnabled`, ensuring that crash tracking behavior remains consistent with a default value of `true`.
The property `IsTrackCrashesEnabled` was defined twice in the `ApplicationInsightsProvider` class. The duplicate definition has been removed, streamlining the code and ensuring that the property is only declared once. The remaining definition retains its default value of `true`.
Modified the code to execute `Task.Run(SendCrashes)` only when the `WriteCrashes` flag is true. This change improves performance by avoiding unnecessary crash reporting when not needed.
|
@dhindrik PR updated, and tested with our app, all looks good :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Not sure if this is wanted or not, but we had a requirement that if the app crashed and the user has the analytics turned off, on the next launch, it would pop up saying the app crashed last time and then ask them if they want to enable analytics and send the report -
To achieve this I added 3 new methods, so I can manually handle the sending of the crashes -
And added 1 new property to handle the writing/ sending of the crashes -
- This can be defaulted to true to prevent a breaking change
- For some reason, I renamed
IsTrackCrashesEnabledtoHandleCrashes. This can be reverted to prevent a breaking changeWhat this allows me to do is have
HandleCrashes(IsTrackCrashesEnabled) always set to true regardless of the users settings. So that all crashes get written to a file. And haveWriteCrashesset too false, and have my own logic to send the crashesAs long as the stuff mentioned above is done, there aren't any breaking changes.