-
Notifications
You must be signed in to change notification settings - Fork 91
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
Add support for offline storage of errors to Raygun4Net Core #530
Conversation
@phillip-haydon this is mostly ready for review. Can review the approach, and the linkage to the client for now There is a singleton background worker, and it is wired up to a cache - which should also be a singleton. The clients (not a singleton yet) can then use the offline store to put failed crash reports, to be picked up by the background worker at a later date. If it fails to send, then it does not remove the cached error from the disk |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lot's of good work! Some questions on my end
Mindscape.Raygun4Net.NetCore.Common/Storage/CachedCrashReportBackgroundWorker.cs
Outdated
Show resolved
Hide resolved
Mindscape.Raygun4Net.NetCore/Storage/FileSystemCrashReportCache.cs
Outdated
Show resolved
Hide resolved
Mindscape.Raygun4Net.NetCore/Storage/FileSystemCrashReportCache.cs
Outdated
Show resolved
Hide resolved
Mindscape.Raygun4Net.NetCore/Storage/FileSystemCrashReportCache.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tagged phill to review the public api surface in specific areas
Mindscape.Raygun4Net.NetCore.Common/Storage/CachedCrashReportBackgroundWorker.cs
Outdated
Show resolved
Hide resolved
Mindscape.Raygun4Net.NetCore/Storage/FileSystemCrashReportCache.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only 1 comment..
Mindscape.Raygun4Net.NetCore.Tests/RaygunMessageBuilderTests.cs
Outdated
Show resolved
Hide resolved
1e3fa0b
to
ca579cf
Compare
This is an early implementation to allow for early feedback and suggestions
This needs a little more thinking
Mark failed files
This should encapsulate the background timer logic, but not mess with weird statics. It ties a store to a send strategy, and that store can be a singleton managed by the user
8be724a
to
1ce8541
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All good! A couple of non-blocking comments
Mindscape.Raygun4Net.NetCore.Common/Offline/OfflineStoreBase.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😠
Mindscape.Raygun4Net.NetCore.Common/Offline/OfflineStoreBase.cs
Outdated
Show resolved
Hide resolved
Mindscape.Raygun4Net.NetCore/Storage/LocalApplicationDataCrashReportStore.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
Add offline storage of crash reports to Raygun4Net
OfflineStoreBase
IBackgroundSendStrategy
which is used to determine how often the store should attempt to send it's errorsAll these classes are designed to be able to be updated/changed by users if they need specific functionality
MAUI will have it's own store implementation
This is an early implementation to allow for early feedback and suggestionsIt is intended that the storage is treated as a singleton. There are default implementations in the net core project, but for other providers such as MAUI there should be another implementation.There is also a singleton background sender, which has an internal timer.Due to the way RaygunClientBase isn't exactly a singleton, we need to loosely couple the actual send method using a delegate (gross)