-
Notifications
You must be signed in to change notification settings - Fork 19
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
feature: Save logging to cache directory #196
Conversation
Reviewer's Guide by SourceryThis pull request implements saving logs to the cache directory and loading them on application start. Sequence diagram for error log persistence flowsequenceDiagram
participant App
participant CrashLogNotifier
participant FileSystem
participant Logger
App->>CrashLogNotifier: init()
activate CrashLogNotifier
CrashLogNotifier->>FileSystem: getApplicationCacheDirectory()
FileSystem-->>CrashLogNotifier: directory
CrashLogNotifier->>FileSystem: read crash_logs.json
FileSystem-->>CrashLogNotifier: stored logs
CrashLogNotifier->>CrashLogNotifier: initialize logger
deactivate CrashLogNotifier
Note over CrashLogNotifier,FileSystem: When new error occurs
Logger->>CrashLogNotifier: logPrint(record)
activate CrashLogNotifier
CrashLogNotifier->>CrashLogNotifier: update state
CrashLogNotifier->>FileSystem: save to crash_logs.json
deactivate CrashLogNotifier
Class diagram showing the refactoring of error logging modelclassDiagram
class ErrorType {
<<enumeration>>
severe
warning
shout
}
class ErrorLogModel {
+ErrorType type
+String message
+DateTime time
+StackTrace? stackTrace
+String label
+String content
+String clipBoard
+Color color
+fromLogRecord(LogRecord) ErrorLogModel
+toJson() Map~String,dynamic~
+fromJson(Map~String,dynamic~) ErrorLogModel
}
class CrashLogNotifier {
-List~ErrorLogModel~ state
-Logger logger
-int maxLength
-String? logFilePath
+init() Future~void~
-initializeLogFile() Future~void~
-loadLogsFromFile() Future~void~
-saveLogsToFile() Future~void~
+clearLogs() void
+logPrint(LogRecord) void
+logFile(FlutterErrorDetails) void
}
ErrorLogModel -- ErrorType
CrashLogNotifier --> ErrorLogModel : manages
note for ErrorLogModel "Extracted from ErrorViewModel
Added JSON serialization"
note for CrashLogNotifier "Added file persistence
Reduced max length to 50"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @PartyDonut - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding error handling around file operations in _loadLogsFromFile() and _saveLogsToFile() to gracefully handle I/O failures
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Review instructions: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
||
void init() { | ||
Future<void> init() async { |
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.
issue (bug_risk): The async init() method is called from the constructor without being awaited, which could lead to race conditions
Consider making the constructor private and providing a static factory method that properly awaits initialization
} | ||
} | ||
|
||
Future<void> _saveLogsToFile() async { |
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.
issue (bug_risk): File operations should include error handling for I/O exceptions
Add try-catch blocks to handle potential file system errors and implement appropriate error recovery
Pull Request Description
Save logs to a .json file in the cache directory and load on application start as to preserve any crashes.