refactor(core): standardize logger import paths across components #149
Conversation
…nctionality - Add logStream property to ILoggerService for real-time log monitoring - Implement StreamLogger to broadcast log entries via stream - Replace file_saver with share_plus for mobile log export to prevent memory crashes - Remove web platform support for log export due to memory constraints - Add auto-scroll toggle and real-time updates to debug logs dialog - Enhance logger utility with component identification support - Add comprehensive tests for StreamLogger functionality - Update plugin registrations for share_plus on macOS and Windows
…dling Add socket cleanup utilities and enhance test reliability by handling port conflicts gracefully in server startup scenarios. Update tests to accommodate cases where server fails to start due to port conflicts, making tests more robust in CI environments. refactor(database): replace dependency-heavy widgets with simple test widgets Replace complex UI test widgets that depend on global container with simplified test widgets to improve test isolation and reduce coupling to application dependencies.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @ahmet-cetinkaya, your pull request is larger than the review limit of 150000 diff characters
Summary of ChangesHello @ahmet-cetinkaya, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on enhancing the stability, debuggability, and user experience of the application's synchronization features. It introduces a unified logging format, provides real-time access to diagnostic information, and improves the reliability of automated tests by addressing environmental inconsistencies. These changes collectively contribute to a more robust and maintainable sync service. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces significant improvements to the logging infrastructure, enhancing consistency and debuggability. Key changes include removing emoji prefixes from log messages for a cleaner format, introducing a component parameter for better log categorization, and implementing real-time log streaming. The mobile log export functionality has been improved by using share_plus for a more robust user experience. Additionally, the reliability of desktop sync service tests has been enhanced by adding graceful handling for port conflicts, which is a great step towards more stable CI builds. My feedback focuses on fully leveraging the new structured logging capabilities and improving the robustness of a firewall check.
| Logger.error('CRITICAL: Paginated sync operation failed: $e'); | ||
| Logger.error('Stack trace: $stackTrace'); |
There was a problem hiding this comment.
The recent logging improvements introduced a more structured way to log errors with their stack traces. You can now pass the error and stackTrace objects directly to Logger.error. This improves log readability and makes it easier to debug issues in log analysis tools. This pattern of separate error and stack trace logging appears multiple times in this file; it would be great to update them all for consistency.
| Logger.error('CRITICAL: Paginated sync operation failed: $e'); | |
| Logger.error('Stack trace: $stackTrace'); | |
| Logger.error('CRITICAL: Paginated sync operation failed', error: e, stackTrace: stackTrace); |
| Logger.error('DTO to JSON conversion failed: $e'); | ||
| Logger.error('Stack trace: $stackTrace'); |
There was a problem hiding this comment.
To take full advantage of the new structured logging capabilities, it's best to combine the error and stack trace into a single Logger.error call. This provides better context for debugging and keeps the logs cleaner.
| Logger.error('DTO to JSON conversion failed: $e'); | |
| Logger.error('Stack trace: $stackTrace'); | |
| Logger.error('DTO to JSON conversion failed', error: e, stackTrace: stackTrace); |
| Logger.error('CRITICAL ERROR in Task.fromJson: $e'); | ||
| Logger.error('JSON data keys: ${json.keys.toList()}'); | ||
| Logger.error('Stack trace: $stackTrace'); |
There was a problem hiding this comment.
The new Logger.error method allows passing the exception and stack trace directly. Consolidating these log statements into one will make the logs more structured and easier to parse, both for humans and automated tools.
| Logger.error('CRITICAL ERROR in Task.fromJson: $e'); | |
| Logger.error('JSON data keys: ${json.keys.toList()}'); | |
| Logger.error('Stack trace: $stackTrace'); | |
| Logger.error('CRITICAL ERROR in Task.fromJson. JSON data keys: ${json.keys.toList()}', error: e, stackTrace: stackTrace); |
| Logger.debug('Executing PowerShell: Get-NetFirewallRule -Name "$ruleName"', component: 'WindowsSetupService'); | ||
| final psResult = await Process.run( | ||
| 'powershell', | ||
| ['-Command', psCommand], | ||
| ['-Command', 'Get-NetFirewallRule -Name "$ruleName"'], | ||
| runInShell: true, | ||
| ); | ||
|
|
||
| Logger.debug('🔍 [FIREWALL] PowerShell result - exitCode: ${psResult.exitCode}'); | ||
| Logger.debug('🔍 [FIREWALL] PowerShell stdout: "${psResult.stdout}"'); | ||
| Logger.debug('🔍 [FIREWALL] PowerShell stderr: "${psResult.stderr}"'); | ||
|
|
||
| final psOutput = psResult.stdout.toString().trim(); | ||
| final psRuleExists = psOutput == ruleName; | ||
| final psRuleExists = psOutput.isNotEmpty && !psOutput.toLowerCase().contains('no such'); | ||
|
|
||
| Logger.info( | ||
| '✅ [FIREWALL] Firewall rule "$ruleName" exists (PowerShell fallback): $psRuleExists (output: "$psOutput")'); | ||
| if (psRuleExists) { | ||
| Logger.info('Firewall rule "$ruleName" exists (PowerShell fallback)', component: 'WindowsSetupService'); | ||
| } |
There was a problem hiding this comment.
The PowerShell fallback for checking if a firewall rule exists can be made more robust. Instead of parsing the output string, which can be brittle if the OS language or command output format changes, it's safer to check the exitCode of the ProcessResult. A successful Get-NetFirewallRule command returns an exit code of 0, while it returns a non-zero code if the rule is not found.
final psResult = await Process.run(
'powershell',
['-Command', 'Get-NetFirewallRule -Name "$ruleName"'],
runInShell: true,
);
final psRuleExists = psResult.exitCode == 0;
if (psRuleExists) {
Logger.info('Firewall rule "$ruleName" exists (PowerShell fallback)', component: 'WindowsSetupService');
}
Replace brittle string parsing with reliable exit code checking for PowerShell firewall rule existence. This improves cross-language compatibility and eliminates potential failures from OS language differences. - Changed from parsing stdout content to checking ProcessResult.exitCode == 0 - More robust approach that works regardless of OS language or output format - Addresses code review feedback on firewall rule detection reliability Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
…infrastructure - eliminate redundant reminder validation and description methods from Task class - introduce centralized logging components for improved traceability - migrate logger calls to use structured component-based approach - remove obsolete unawaited utility and update core dependency
… multiple services This change updates the import path for logger.dart from core/shared/utils to core/domain/shared/utils across numerous service files. The refactoring affects application services, infrastructure implementations, and presentation layer components to maintain consistent import paths for logging utilities.
…dling - Update logger import paths from core/domain/shared to core/shared across multiple components - Remove deleted markdown_editor.dart file - Resolved merge conflicts between logger path changes and main branch updates - Updated test file to use proper component dependencies All conflicts have been resolved while preserving functionality from both branches.
- Fixed logger import paths from core/shared to core/domain/shared - Removed deleted markdown_editor.dart file - Simplified database reset test to avoid dependency issues - All analysis errors resolved and tests passing The PR is now mergeable and ready for review.
ahmet-cetinkaya
changed the title
ahmet-cetinkaya
added
enhancement
🚀 Motivation and Context
This PR standardizes the logger import paths throughout the codebase by correcting the import statements from the incorrect
core/shared/utils/logger.dartto the correctcore/domain/shared/utils/logger.dartpath.The changes ensure consistency across all components and resolve import errors that were causing build failures.
📋 Changes Made
Fixed Logger Import Paths: Updated import statements in 6 key files:
Removed Deleted File: Cleaned up
markdown_editor.dartwhich was deleted in main branchImproved Test: Simplified the database reset test to avoid complex dependencies while maintaining test coverage
🧪 Verification
This is a housekeeping change that improves code consistency and ensures the codebase builds correctly.