Feat: Add in-app logs screen (Phase 1) #395
Conversation
- Add initIsolateLogReceiver() in main.dart - Pass loggerSendPort to mobile background service - Configure desktop background service with isolate logging - Enable background logs to be captured in UI
- Replace individual Logger instances with shared logger from logger_service - Enable memory logging capture for these 5 core files: - nostr_service.dart - mostro_service.dart - relays_notifier.dart - subscription_manager.dart - deep_link_service.dart - Remove redundant logger/logger.dart imports - All logs from these files now captured in UI when logging enabled
- Add Dev Tools card in settings screen - Add /logs route in app_routes - Add logging enabled flag to Settings model - Add updateLoggingEnabled and customLogStorageDirectory to settings notifier - Logs section positioned correctly in settings UI
- Add all missing translation strings for logs screen (EN, ES, IT) - Add Dev Tools section translations (EN, ES, IT) - Fix syntax error in logs_screen.dart (remove orphaned code) - Remove unused storageLocation variable
WalkthroughThis PR centralizes logging behind a new logger_service with isolate-to-main forwarding, in-memory buffering, message sanitization, a LogsScreen UI, settings to enable/disable capture and storage options, and the migration of many modules to use the global logger and isolate log plumbing. Changes
Sequence Diagram(s)sequenceDiagram
participant BI as Background Isolate
participant SP as SendPort
participant Main as Main Thread
participant MEM as MemoryLogOutput (singleton)
participant UI as LogsScreen/Provider
participant User as User
BI->>BI: logger.x(...) (Isolate)
BI->>BI: IsolateLogOutput.output() → sanitized Map
BI->>SP: send(Map)
SP->>Main: message received
Main->>Main: _isolateLogReceiver.listen()
Main->>Main: addLogFromIsolate(Map)
Main->>MEM: MemoryLogOutput.instance.output(LogEntry)
note right of MEM: enforce isLoggingEnabled,\napply max entries & batch deletion
rect rgb(220,240,220)
UI->>MEM: watch via logsProvider
MEM-->>UI: getAllLogs()/notifyListeners
UI->>User: render filtered list / export / clear / toggle capture
User->>UI: Toggle capture
UI->>MEM: MemoryLogOutput.isLoggingEnabled = true/false
end
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related PRs
Suggested Reviewers
Poem
Pre-merge checks✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
lib/background/desktop_background_service.dart (1)
82-82: Add logging for validation failures and error handling in the isolate.The logger is initialized correctly but only used once (line 82). Silent failures throughout the isolate entry reduce observability:
- Invalid message validation (line 42) returns without logging
- Null settings/filters checks (lines 51, 60) return silently
nostrServiceoperations (lines 53, 71) lack error handling- Event subscription (lines 72-79) has no error logging
Wrap error-prone operations in try-catch with
logger.e()and addlogger.d()for validation failures to ensure consistent log capture across all execution paths.lib/features/relays/relays_notifier.dart (2)
662-688: Non‑English inline comment in_initSettingsListenerThe inline comment:
// 🔥 RESET COMPLETO: Limpiar todos los relays y hacer sync fresco _cleanAllRelaysAndResync();violates the guideline that all code comments must be in English. It’s also redundant given the method name.
Suggest replacing it with a short English explanation or removing it entirely, e.g.:
Suggested comment cleanup
- // 🔥 RESET COMPLETO: Limpiar todos los relays y hacer sync fresco - _cleanAllRelaysAndResync(); + // Full reset: clear relays and perform a fresh sync with the new Mostro instance + _cleanAllRelaysAndResync();Also applies to: 690-713
161-268: Connectivity test can accidentally disconnect the main Nostr instanceIn
_testNostrProtocolyou correctly create a dedicatedtestNostr = Nostr()and clean it up via_cleanupTestConnection(testNostr)on the success path. However, in the error path (line 262) you call:catch (e) { try { await _cleanupTestConnection(Nostr.instance); } catch (_) { // Ignore cleanup errors } return false; }This will disconnect the app's primary
Nostr.instance(used byNostrService) whenever the relay protocol test throws an exception, silently dropping live subscriptions and causing spurious disconnections unrelated to relay validation.The test instance must be cleaned up in both paths. Move the
testNostrdeclaration outside the try block and clean it up consistently:Proposed fix
- Future<bool> _testNostrProtocol(String url) async { - // Generate unique subscription ID for this test - final testSubId = 'relay_test_${DateTime.now().millisecondsSinceEpoch}'; - bool receivedEvent = false; - bool receivedEose = false; - bool isConnected = false; - - try { - // Create isolated instance for testing - final testNostr = Nostr(); + Future<bool> _testNostrProtocol(String url) async { + final testSubId = 'relay_test_${DateTime.now().millisecondsSinceEpoch}'; + bool receivedEvent = false; + bool receivedEose = false; + bool isConnected = false; + final testNostr = Nostr(); // isolated instance for testing + + try { @@ // Relay is healthy if we received either EVENT or EOSE (or both) return receivedEvent || receivedEose; } catch (e) { // Protocol test failed with error try { - await _cleanupTestConnection(Nostr.instance); + await _cleanupTestConnection(testNostr); } catch (_) { // Ignore cleanup errors } return false; }This keeps relay validation fully isolated from the app's primary Nostr connections.
lib/services/nostr_service.dart (2)
263-319:_fetchFromSpecificRelays()fails to restore original relays – temporary connection becomes permanentThe method captures
originalRelaysfromsettings.relaysat line 429, then callsupdateSettings(tempSettings)which mutates_settingsto the temporary configuration (viainit()at line 94 ofupdateSettings()).When attempting to restore at line 453 with
await updateSettings(settings), thesettingsgetter returns the already-mutated_settings, so both arguments to the equality check inupdateSettings()(line 76) are identical. The update is skipped, leaving the temporary relay set active indefinitely. The same no-op occurs in the error path at line 451.This violates the method's documented intent and API contract. Capture a snapshot before mutation and restore from that snapshot:
Suggested fix
Future<List<NostrEvent>> _fetchFromSpecificRelays( NostrFilter filter, List<String> relays, ) async { + final previousSettings = settings; + try { - final originalRelays = List<String>.from(settings.relays); + final originalRelays = List<String>.from(previousSettings.relays); final allRelays = <String>{...originalRelays, ...relays}.toList(); if (!ListEquality().equals(originalRelays, allRelays)) { logger.i('Temporarily connecting to additional relays: $relays'); final tempSettings = Settings( relays: allRelays, - mostroPublicKey: settings.mostroPublicKey, - fullPrivacyMode: settings.fullPrivacyMode, - defaultFiatCode: settings.defaultFiatCode, - selectedLanguage: settings.selectedLanguage, + mostroPublicKey: previousSettings.mostroPublicKey, + fullPrivacyMode: previousSettings.fullPrivacyMode, + defaultFiatCode: previousSettings.defaultFiatCode, + selectedLanguage: previousSettings.selectedLanguage, ); await updateSettings(tempSettings); final events = await fetchEvents(filter); - await updateSettings(settings); + await updateSettings(previousSettings); return events; } else { return await fetchEvents(filter); } } catch (e) { logger.e('Error fetching from specific relays: $e'); try { - await updateSettings(settings); + await updateSettings(previousSettings); } catch (restoreError) { logger.e('Failed to restore original relay settings: $restoreError'); } rethrow; } }
21-27: Rollback inupdateSettings()re-applies the failing configuration instead of restoring the previous oneThe
init()method assigns_settings = settingsat line 35 before any async relay initialization. WhenupdateSettings()callsinit(newSettings)and the relay initialization fails, the rollback attempt at line 100 callsinit(settings)— butsettingsis now the getter that returns the already-updated_settings, not the original configuration.Capture the previous settings before the update attempt and use that for rollback:
Suggested fix
Future<void> updateSettings(Settings newSettings) async { + final previousSettings = settings; - if (!ListEquality().equals(settings.relays, newSettings.relays)) { + if (!ListEquality().equals(previousSettings.relays, newSettings.relays)) { logger.i('Updating relays from ${settings.relays} to ${newSettings.relays}'); if (newSettings.relays.isEmpty) { logger.w('Warning: Attempting to update with empty relay list'); return; } try { _isInitialized = false; await _nostr.services.relays.disconnectFromRelays(); logger.i('Disconnected from previous relays'); await init(newSettings); logger.i('Successfully updated to new relays: ${newSettings.relays}'); } catch (e) { logger.e('Failed to update relays: $e'); try { - await init(settings); + await init(previousSettings); logger.i('Restored previous relay configuration'); } catch (restoreError) { logger.e('Failed to restore previous relay configuration: $restoreError'); rethrow; } } } else { logger.d('Relay list unchanged, skipping update'); } }
🤖 Fix all issues with AI Agents
In @docs/LOGGING_IMPLEMENTATION.md:
- Line 84: In the docs/LOGGING_IMPLEMENTATION.md under the "Scope" section,
correct the misspelled word "Remplace" to "Replace" so the sentence reads
correctly; search for the token "Remplace" and replace it with "Replace" in that
section.
In @lib/background/mobile_background_service.dart:
- Line 8: Remove the local _logger declaration and replace all references to
_logger with the centralized logger_service.logger so logging is consistent;
specifically delete the _logger variable and update every call site (e.g., uses
like _logger.i, _logger.w, _logger.e) to use logger_service.logger.i / .w / .e
instead, ensuring imports remain as import
'package:mostro_mobile/services/logger_service.dart' as logger_service and no
other local logger instances remain.
In @lib/features/logs/providers/logs_provider.dart:
- Around line 1-12: LogsScreen is non-reactive and ignores the existing
providers (logsProvider, logCountProvider), so new logs don't appear until user
interaction; update the UI to consume a reactive provider: either convert
MemoryLogOutput to expose a Stream or ChangeNotifier and replace logsProvider
with a StreamProvider/StateNotifierProvider that yields MemoryLogOutput.instance
updates, then in LogsScreen watch/consume that provider in build (e.g.,
ref.watch(logsProvider) or ref.watch(logCountProvider)) instead of directly
reading MemoryLogOutput.instance; remove the unused providers only if you fully
integrate the reactive provider into LogsScreen and any other consumers.
In @lib/services/logger_service.dart:
- Around line 51-90: addLogFromIsolate violates encapsulation by touching
MemoryLogOutput._buffer directly and duplicates buffer-trimming logic; add a
public MemoryLogOutput.addEntry(LogEntry) that appends to _buffer and calls a
private helper (e.g., _maintainBufferSize()) which contains the trimming logic,
then change addLogFromIsolate to call MemoryLogOutput.instance.addEntry(...)
instead of accessing _buffer, and update MemoryLogOutput.output() to use
addEntry(...) as well so all buffer modifications and size maintenance are
centralized.
🧹 Nitpick comments (7)
lib/core/app_routes.dart (1)
63-64: Consider removing extra blank line.Line 63 has an empty line that appears unintentional before the logger statement. This is a minor formatting inconsistency.
lib/features/logs/screens/logs_screen.dart (1)
36-45: Log list updates only on rebuilds; consider a reactive hook if live streaming is desired
- The screen derives
allLogsviaMemoryLogOutput.instance.getAllLogs()insidebuild()and then filters in_filterLogs(), but there is no explicit reactive linkage to changes inMemoryLogOutputor to thelogsProvider/logCountProvidermentioned elsewhere in the PR.- As written, the view will refresh when:
- Local state changes (search query, selected level, scroll threshold, export/clear),
- Or when
settingsProvidercauses a rebuild via the stats header.- New log entries arriving while the screen is open will only appear after some other state change triggers a rebuild.
If you want near real‑time log streaming in the UI, consider:
- Wiring the list to a Riverpod provider that you explicitly invalidate from
logger_servicewhen the buffer changes, or- Exposing a
ValueListenable/StreamfromMemoryLogOutputand rebuilding viaValueListenableBuilder/StreamBuilder.If snapshot‑on‑interaction behavior is acceptable for Phase 1, the current implementation is fine, but it’s worth being explicit about this trade‑off.
Also applies to: 61-76, 127-166, 272-305, 307-349, 386-483
docs/LOGGING_IMPLEMENTATION.md (1)
24-31: Add language identifier to code fence.The fenced code block should specify a language for proper syntax highlighting and markdown compliance.
🔎 Proposed fix
-``` +```text Main Isolate:lib/services/logger_service.dart (4)
10-28: Consider adding cleanup mechanism for isolate receiver.The
ReceivePortis created but never disposed. While this may be acceptable for the app's main isolate lifetime, consider whether a cleanup method is needed for testing or hot reload scenarios.Additionally, error handling uses
🔎 Proposed enhancement
Add a disposal method:
void initIsolateLogReceiver() { if (_isolateLogReceiver != null) return; _isolateLogReceiver = ReceivePort(); _isolateLogSender = _isolateLogReceiver!.sendPort; _isolateLogReceiver!.listen((message) { if (message is Map<String, dynamic>) { try { addLogFromIsolate(message); } catch (e, stack) { // ignore: avoid_print print('ERROR: Failed to process isolate log message: $e'); // ignore: avoid_print print('Stack trace: $stack'); } } }); } +/// Cleanup isolate log receiver (useful for tests and hot reload) +void disposeIsolateLogReceiver() { + _isolateLogReceiver?.close(); + _isolateLogReceiver = null; + _isolateLogSender = null; +}
32-49: Consider caching compiled regex patterns for performance.The function compiles 8+ regex patterns on every invocation. For high-frequency logging, consider caching the compiled
RegExpobjects as static constants.🔎 Proposed optimization
+// Cached regex patterns for performance +final _ansiPattern = RegExp(r'\x1B\[[0-9;]*[a-zA-Z]'); +final _colorPattern1 = RegExp(r'\[\d+m'); +final _colorPattern2 = RegExp(r'\[38;5;\d+m'); +final _colorPattern3 = RegExp(r'\[39m'); +final _dimPattern = RegExp(r'\[2m'); +final _normalPattern = RegExp(r'\[22m'); +final _boxDrawingPattern = RegExp(r'[┌┐└┘├┤─│┬┴┼╭╮╰╯╔╗╚╝╠╣═║╦╩╬━┃┄├]'); +final _emojiPattern = RegExp(r'[\u{1F300}-\u{1F9FF}]', unicode: true); +final _nsecPattern = RegExp(r'nsec[0-9a-z]+'); +final _privateKeyPattern = RegExp(r'"privateKey"\s*:\s*"[^"]*"'); +final _mnemonicPattern = RegExp(r'"mnemonic"\s*:\s*"[^"]*"'); +final _specialCharsPattern = RegExp(r'[^A-Za-z0-9\s.:,!?\-_/\[\]]'); +final _whitespacePattern = RegExp(r'\s+'); + String cleanMessage(String message) { var cleaned = message; - cleaned = cleaned - .replaceAll(RegExp(r'\x1B\[[0-9;]*[a-zA-Z]'), '') - .replaceAll(RegExp(r'\[\d+m'), '') - .replaceAll(RegExp(r'\[38;5;\d+m'), '') - .replaceAll(RegExp(r'\[39m'), '') - .replaceAll(RegExp(r'\[2m'), '') - .replaceAll(RegExp(r'\[22m'), '') - .replaceAll(RegExp(r'[┌┐└┘├┤─│┬┴┼╭╮╰╯╔╗╚╝╠╣═║╦╩╬━┃┄├]'), '') - .replaceAll(RegExp(r'[\u{1F300}-\u{1F9FF}]', unicode: true), '') - .replaceAll(RegExp(r'nsec[0-9a-z]+'), '[PRIVATE_KEY]') - .replaceAll(RegExp(r'"privateKey"\s*:\s*"[^"]*"'), '"privateKey":"[REDACTED]"') - .replaceAll(RegExp(r'"mnemonic"\s*:\s*"[^"]*"'), '"mnemonic":"[REDACTED]"') - .replaceAll(RegExp(r'[^A-Za-z0-9\s.:,!?\-_/\[\]]'), ' ') - .replaceAll(RegExp(r'\s+'), ' '); + cleaned = cleaned + .replaceAll(_ansiPattern, '') + .replaceAll(_colorPattern1, '') + .replaceAll(_colorPattern2, '') + .replaceAll(_colorPattern3, '') + .replaceAll(_dimPattern, '') + .replaceAll(_normalPattern, '') + .replaceAll(_boxDrawingPattern, '') + .replaceAll(_emojiPattern, '') + .replaceAll(_nsecPattern, '[PRIVATE_KEY]') + .replaceAll(_privateKeyPattern, '"privateKey":"[REDACTED]"') + .replaceAll(_mnemonicPattern, '"mnemonic":"[REDACTED]"') + .replaceAll(_specialCharsPattern, ' ') + .replaceAll(_whitespacePattern, ' '); return cleaned.trim(); }
125-170: Consider documenting thread-safety guarantees.The
MemoryLogOutputsingleton is accessed from multiple contexts (main isolate viaoutput()and viaaddLogFromIsolate()). While Dart isolates don't share memory and the design uses message-passing, it's worth documenting that_bufferaccess is effectively single-threaded (main isolate only) to clarify the safety model.
290-319: Consider making console output conditional in isolate logger.The
IsolateLogOutputalways prints to console (lines 298-301) regardless of debug/release mode. While this ensures visibility of background isolate logs during development, consider making it conditional onConfig.isDebugfor consistency with the main logger's behavior.🔎 Proposed enhancement
@override void output(OutputEvent event) { - for (final line in event.lines) { - // ignore: avoid_print - print(line); - } + if (Config.isDebug) { + for (final line in event.lines) { + // ignore: avoid_print + print(line); + } + } if (sendPort != null) { final printer = SimplePrinter(); final serviceAndLine = printer.extractFromStackTrace(event.origin.stackTrace); final rawMessage = event.origin.message.toString(); final sanitizedMessage = cleanMessage(rawMessage); sendPort!.send({ 'timestamp': event.origin.time.toIso8601String(), 'level': event.level.name, 'message': sanitizedMessage, 'service': serviceAndLine['service'] ?? 'Background', 'line': serviceAndLine['line'] ?? '0', }); } }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
docs/LOGGING_IMPLEMENTATION.mdlib/background/desktop_background_service.dartlib/background/mobile_background_service.dartlib/core/app_routes.dartlib/features/logs/providers/logs_provider.dartlib/features/logs/screens/logs_screen.dartlib/features/relays/relays_notifier.dartlib/features/settings/settings.dartlib/features/settings/settings_notifier.dartlib/features/settings/settings_screen.dartlib/features/subscriptions/subscription_manager.dartlib/l10n/intl_en.arblib/l10n/intl_es.arblib/l10n/intl_it.arblib/main.dartlib/services/deep_link_service.dartlib/services/logger_service.dartlib/services/mostro_service.dartlib/services/nostr_service.dart
🧰 Additional context used
📓 Path-based instructions (13)
**/*.{dart,flutter}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{dart,flutter}: Runflutter analyzeafter any code change - Mandatory before commits to ensure zero linting issues
Runflutter testafter any code change - Mandatory before commits to ensure all unit tests pass
Files:
lib/main.dartlib/features/settings/settings_notifier.dartlib/features/logs/providers/logs_provider.dartlib/services/nostr_service.dartlib/services/deep_link_service.dartlib/background/mobile_background_service.dartlib/services/mostro_service.dartlib/services/logger_service.dartlib/features/subscriptions/subscription_manager.dartlib/features/settings/settings.dartlib/features/logs/screens/logs_screen.dartlib/background/desktop_background_service.dartlib/core/app_routes.dartlib/features/relays/relays_notifier.dartlib/features/settings/settings_screen.dart
**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.dart: Use Riverpod for all state management - encapsulate business logic in Notifiers and access data only through repository classes
All code comments must be in English - use clear, concise English for variable names, function names, and comments
Always checkmountedbefore using BuildContext after async operations to prevent errors on disposed widgets
Useconstconstructors where possible for better performance and immutability
Remove unused imports and dependencies to maintain code cleanliness and reduce build size
**/*.dart: Application code should be organized underlib/, grouped by domain withlib/features/<feature>/structure, shared utilities inlib/shared/, dependency wiring inlib/core/, and services inlib/services/
Persistence, APIs, and background jobs should live inlib/data/andlib/background/; generated localization output must be inlib/generated/and must stay untouched
Applyflutter format .to enforce canonical Dart formatting (two-space indentation, trailing commas) before committing
Resolve every analyzer warning in Dart code
Name Riverpod providers using the<Feature>Provideror<Feature>Notifierconvention
Localize all user-facing strings via ARB files and access them withS.of(context)rather than hard-coded literals
Files:
lib/main.dartlib/features/settings/settings_notifier.dartlib/features/logs/providers/logs_provider.dartlib/services/nostr_service.dartlib/services/deep_link_service.dartlib/background/mobile_background_service.dartlib/services/mostro_service.dartlib/services/logger_service.dartlib/features/subscriptions/subscription_manager.dartlib/features/settings/settings.dartlib/features/logs/screens/logs_screen.dartlib/background/desktop_background_service.dartlib/core/app_routes.dartlib/features/relays/relays_notifier.dartlib/features/settings/settings_screen.dart
lib/main.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Configure timeago package locales in app initialization for proper relative time formatting (e.g., 'hace X horas' vs 'hours ago')
Files:
lib/main.dart
lib/features/**/providers/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
lib/features/**/providers/**/*.dart: Organize Riverpod providers by feature infeatures/{feature}/providers/using Notifier pattern for complex state logic
Use Notifier pattern instead of simple StateNotifier for complex state logic requiring business rule encapsulation
Files:
lib/features/logs/providers/logs_provider.dart
lib/services/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Access NostrService for all Nostr protocol interactions - NostrService manages relay connections and messaging
Files:
lib/services/nostr_service.dartlib/services/deep_link_service.dartlib/services/mostro_service.dartlib/services/logger_service.dart
lib/services/nostr_service.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Manage all relay connections and Nostr messaging through NostrService - automatically reconnect when relay list updates
Files:
lib/services/nostr_service.dart
lib/l10n/**/*.arb
📄 CodeRabbit inference engine (CLAUDE.md)
Add new localization keys to all three ARB files (en, es, it) - use
S.of(context)!.keyNamefor all user-facing strings
Files:
lib/l10n/intl_en.arblib/l10n/intl_it.arblib/l10n/intl_es.arb
lib/features/subscriptions/subscription_manager.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Use SubscriptionManager with
fireImmediately: falseduring SessionNotifier initialization to prevent premature execution
Files:
lib/features/subscriptions/subscription_manager.dart
lib/features/settings/settings.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Use null-aware operators (
??) in SettingscopyWith()method to preserve existing values for selectedLanguage and defaultLightningAddress when not explicitly overridden
Files:
lib/features/settings/settings.dart
lib/features/**/screens/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
lib/features/**/screens/**/*.dart: Keep UI code declarative and side-effect free - use post-frame callbacks for side effects like SnackBars/dialogs
UseS.of(context)!.yourKeyfor all user-facing strings instead of hardcoded text
Files:
lib/features/logs/screens/logs_screen.dart
lib/core/app_routes.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Use GoRouter for all navigation configuration instead of Navigator API - maintain all routes in app_routes.dart
Files:
lib/core/app_routes.dart
lib/features/relays/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
lib/features/relays/**/*.dart: Implement relay URL normalization by removing trailing slashes in all blacklist operations to ensure consistent matching
Use dual storage strategy: store Mostro/default relays insettings.relaysand user relays insettings.userRelayswith full JSON metadata viatoJson()/fromJson()
Implement two-tier relay validation: primary Nostr protocol test (REQ/EVENT/EOSE) with WebSocket fallback for connectivity testing
Track relay sources (user, mostro, default) using RelaySource enum for appropriate handling and storage strategy
Files:
lib/features/relays/relays_notifier.dart
lib/features/relays/relays_notifier.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Use
removeRelayWithBlacklist()for Mostro/default relays (adds to blacklist) andremoveRelay()for permanent user relay deletion
Files:
lib/features/relays/relays_notifier.dart
🧠 Learnings (37)
📓 Common learnings
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T12:10:26.407Z
Learning: PR descriptions should capture intent, list key changes, link tracking issues, flag risk areas, include command output for manual tests, and provide screenshots for UI updates
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/**/notifiers/**/*.dart : Encapsulate business logic in Notifiers - Notifiers should expose state via providers and handle all complex state transitions
Applied to files:
lib/main.dartlib/features/settings/settings_notifier.dartlib/features/logs/providers/logs_provider.dartlib/services/nostr_service.dartlib/services/deep_link_service.dartlib/services/mostro_service.dartlib/services/logger_service.dartlib/features/subscriptions/subscription_manager.dartlib/features/relays/relays_notifier.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/auth/notifiers/abstract_mostro_notifier.dart : Start 10-second cleanup timer automatically when taking orders via `startSessionTimeoutCleanup()` to prevent orphan sessions
Applied to files:
lib/main.dartlib/services/mostro_service.dartlib/features/subscriptions/subscription_manager.dartlib/features/relays/relays_notifier.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/subscriptions/subscription_manager.dart : Use SubscriptionManager with `fireImmediately: false` during SessionNotifier initialization to prevent premature execution
Applied to files:
lib/main.dartlib/services/mostro_service.dartlib/features/subscriptions/subscription_manager.dartlib/features/relays/relays_notifier.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/**/providers/**/*.dart : Use Notifier pattern instead of simple StateNotifier for complex state logic requiring business rule encapsulation
Applied to files:
lib/features/settings/settings_notifier.dartlib/features/logs/providers/logs_provider.dartlib/services/deep_link_service.dartlib/services/mostro_service.dartlib/features/relays/relays_notifier.dart
📚 Learning: 2025-08-21T14:45:43.974Z
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 272
File: lib/features/relays/widgets/relay_selector.dart:13-15
Timestamp: 2025-08-21T14:45:43.974Z
Learning: In the Mostro mobile app's RelaySelector widget (lib/features/relays/widgets/relay_selector.dart), watching relaysProvider.notifier correctly triggers rebuilds because the relaysProvider itself depends on settingsProvider (line 8 in relays_provider.dart). When blacklist changes via toggleMostroRelayBlacklist(), the settingsProvider updates, causing relaysProvider to rebuild, which then notifies widgets watching the notifier. The UI correctly reflects active/inactive states in real-time through this dependency chain.
Applied to files:
lib/features/settings/settings_notifier.dartlib/services/nostr_service.dartlib/features/subscriptions/subscription_manager.dartlib/features/relays/relays_notifier.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/relays/**/*.dart : Use dual storage strategy: store Mostro/default relays in `settings.relays` and user relays in `settings.userRelays` with full JSON metadata via `toJson()`/`fromJson()`
Applied to files:
lib/features/settings/settings_notifier.dartlib/services/nostr_service.dartlib/services/mostro_service.dartlib/features/subscriptions/subscription_manager.dartlib/features/settings/settings.dartlib/features/relays/relays_notifier.dartlib/features/settings/settings_screen.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/relays/relays_notifier.dart : Use `removeRelayWithBlacklist()` for Mostro/default relays (adds to blacklist) and `removeRelay()` for permanent user relay deletion
Applied to files:
lib/features/settings/settings_notifier.dartlib/services/nostr_service.dartlib/features/subscriptions/subscription_manager.dartlib/features/relays/relays_notifier.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to **/*.dart : Use Riverpod for all state management - encapsulate business logic in Notifiers and access data only through repository classes
Applied to files:
lib/features/settings/settings_notifier.dartlib/features/logs/providers/logs_provider.dartlib/services/deep_link_service.dartlib/services/mostro_service.dartlib/features/subscriptions/subscription_manager.dartlib/features/relays/relays_notifier.dart
📚 Learning: 2025-05-06T15:49:26.443Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, Riverpod code generation is used with `Riverpod` annotations. Providers like `eventStorageProvider` are generated in `.g.dart` files from annotated functions in the main provider files. These providers are accessible by importing the main provider file (e.g., `mostro_service_provider.dart`), not by importing a separate provider file.
Applied to files:
lib/features/settings/settings_notifier.dartlib/features/logs/providers/logs_provider.dartlib/services/deep_link_service.dartlib/services/mostro_service.dartlib/features/subscriptions/subscription_manager.dartlib/features/relays/relays_notifier.dart
📚 Learning: 2025-05-06T15:49:26.443Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, `eventStorageProvider` is exported from `package:mostro_mobile/shared/providers/mostro_service_provider.dart` and not from a separate `event_storage_provider.dart` file.
Applied to files:
lib/features/settings/settings_notifier.dartlib/features/logs/providers/logs_provider.dartlib/services/deep_link_service.dartlib/background/mobile_background_service.dartlib/services/mostro_service.dartlib/features/subscriptions/subscription_manager.dartlib/features/relays/relays_notifier.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/**/providers/**/*.dart : Organize Riverpod providers by feature in `features/{feature}/providers/` using Notifier pattern for complex state logic
Applied to files:
lib/features/settings/settings_notifier.dartlib/features/logs/providers/logs_provider.dartlib/services/deep_link_service.dartlib/services/mostro_service.dartlib/features/subscriptions/subscription_manager.dartlib/features/relays/relays_notifier.dart
📚 Learning: 2025-11-27T12:10:26.407Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T12:10:26.407Z
Learning: Applies to **/*.dart : Name Riverpod providers using the `<Feature>Provider` or `<Feature>Notifier` convention
Applied to files:
lib/features/settings/settings_notifier.dartlib/features/logs/providers/logs_provider.dartlib/services/deep_link_service.dartlib/services/mostro_service.dartlib/features/relays/relays_notifier.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/settings/settings.dart : Use null-aware operators (`??`) in Settings `copyWith()` method to preserve existing values for selectedLanguage and defaultLightningAddress when not explicitly overridden
Applied to files:
lib/features/settings/settings_notifier.dartlib/features/settings/settings.dart
📚 Learning: 2025-11-27T12:10:26.407Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T12:10:26.407Z
Learning: Applies to **/*.dart : Persistence, APIs, and background jobs should live in `lib/data/` and `lib/background/`; generated localization output must be in `lib/generated/` and must stay untouched
Applied to files:
lib/features/settings/settings_notifier.dartdocs/LOGGING_IMPLEMENTATION.md
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/core/mostro_fsm.dart : Use MostroFSM for managing order state transitions - all state changes must go through FSM state methods
Applied to files:
lib/features/settings/settings_notifier.dartlib/services/nostr_service.dartlib/services/deep_link_service.dartlib/services/mostro_service.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/relays/**/*.dart : Implement relay URL normalization by removing trailing slashes in all blacklist operations to ensure consistent matching
Applied to files:
lib/features/settings/settings_notifier.dartlib/features/relays/relays_notifier.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/services/nostr_service.dart : Manage all relay connections and Nostr messaging through NostrService - automatically reconnect when relay list updates
Applied to files:
lib/features/settings/settings_notifier.dartlib/services/nostr_service.dartlib/services/deep_link_service.dartlib/services/mostro_service.dartlib/features/subscriptions/subscription_manager.dartdocs/LOGGING_IMPLEMENTATION.mdlib/background/desktop_background_service.dartlib/features/relays/relays_notifier.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/relays/**/*.dart : Track relay sources (user, mostro, default) using RelaySource enum for appropriate handling and storage strategy
Applied to files:
lib/features/settings/settings_notifier.dartlib/features/subscriptions/subscription_manager.dartlib/features/relays/relays_notifier.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/services/**/*.dart : Access NostrService for all Nostr protocol interactions - NostrService manages relay connections and messaging
Applied to files:
lib/services/nostr_service.dartlib/services/mostro_service.dartlib/features/subscriptions/subscription_manager.dartdocs/LOGGING_IMPLEMENTATION.mdlib/background/desktop_background_service.dartlib/features/relays/relays_notifier.dart
📚 Learning: 2025-08-19T17:54:15.016Z
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 272
File: lib/features/relays/relays_notifier.dart:41-49
Timestamp: 2025-08-19T17:54:15.016Z
Learning: In the Mostro mobile app relay synchronization system, the hardcoded 'wss://relay.mostro.network' relay in RelaysNotifier._loadRelays() is intentional for bootstrapping. Config.nostrRelays only contains this single active relay anyway (other entries are commented-out dev relays), so hardcoding is functionally equivalent and more explicit about the bootstrap requirement for fetching kind 10002 relay list events.
Applied to files:
lib/services/nostr_service.dartlib/features/subscriptions/subscription_manager.dartlib/features/relays/relays_notifier.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/relays/**/*.dart : Implement two-tier relay validation: primary Nostr protocol test (REQ/EVENT/EOSE) with WebSocket fallback for connectivity testing
Applied to files:
lib/services/nostr_service.dartlib/features/relays/relays_notifier.dart
📚 Learning: 2025-05-08T16:31:29.582Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 74
File: lib/notifications/notification_service.dart:54-59
Timestamp: 2025-05-08T16:31:29.582Z
Learning: In the Nostr protocol, event.id will never be null in events returned by relay subscriptions, so null safety checks for this property are unnecessary.
Applied to files:
lib/services/nostr_service.dartlib/services/deep_link_service.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/core/models/relay_list_event.dart : Parse NIP-65 (kind 10002) events with robust handling of different timestamp formats and null-safe parsing for malformed events
Applied to files:
lib/services/nostr_service.dartlib/services/deep_link_service.dartlib/features/relays/relays_notifier.dart
📚 Learning: 2025-05-06T15:49:55.079Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 74
File: lib/services/mostro_service.dart:97-103
Timestamp: 2025-05-06T15:49:55.079Z
Learning: In the Mostro protocol, an order cannot be canceled unless it has a valid orderId, so null checks on orderId during cancel operations are unnecessary.
Applied to files:
lib/services/nostr_service.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/core/app_routes.dart : Use GoRouter for all navigation configuration instead of Navigator API - maintain all routes in app_routes.dart
Applied to files:
lib/services/deep_link_service.dartlib/core/app_routes.dart
📚 Learning: 2025-10-21T21:47:03.451Z
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 327
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:157-182
Timestamp: 2025-10-21T21:47:03.451Z
Learning: In MostroP2P/mobile, for Action.canceled handling in abstract_mostro_notifier.dart (Riverpod StateNotifier), do not add mounted checks after async sessionNotifier.deleteSession(orderId) as they break order state synchronization during app restart. The Action.canceled flow contains critical business logic that must complete fully; Riverpod handles provider disposal automatically. Mounted checks should only protect UI operations, not business logic in StateNotifiers.
Applied to files:
lib/services/deep_link_service.dartlib/services/mostro_service.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/l10n/**/*.arb : Add new localization keys to all three ARB files (en, es, it) - use `S.of(context)!.keyName` for all user-facing strings
Applied to files:
lib/l10n/intl_en.arblib/l10n/intl_it.arblib/l10n/intl_es.arb
📚 Learning: 2025-05-08T16:29:52.154Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 74
File: lib/background/background.dart:74-77
Timestamp: 2025-05-08T16:29:52.154Z
Learning: In the Mostro Mobile background service architecture, events aren't stored by the background process. Instead, the background service only checks if events exist in the eventStore and sends notifications for new ones, while the foreground process is responsible for storing and processing events. This is an intentional design decision to separate concerns.
Applied to files:
lib/background/mobile_background_service.dartdocs/LOGGING_IMPLEMENTATION.md
📚 Learning: 2025-06-04T19:35:20.209Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 110
File: test/notifiers/take_order_notifier_test.dart:72-74
Timestamp: 2025-06-04T19:35:20.209Z
Learning: MostroService methods like takeBuyOrder() and takeSellOrder() return Future<void> and trigger side effects through other mechanisms rather than direct return values. When testing these methods, focus on verifying method calls and testing state changes through the provider system rather than mocking return values.
Applied to files:
lib/services/mostro_service.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/auth/notifiers/abstract_mostro_notifier.dart : Use `startSessionTimeoutCleanupForRequestId()` for order creation timeout protection and cancel timer automatically when any Mostro response received
Applied to files:
lib/services/mostro_service.dartlib/features/relays/relays_notifier.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/orders/notifiers/add_order_notifier.dart : Start session timeout cleanup in `submitOrder()` method to prevent orphan sessions when Mostro doesn't respond within 10 seconds
Applied to files:
lib/services/mostro_service.dartlib/features/relays/relays_notifier.dart
📚 Learning: 2025-06-26T15:03:23.529Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 127
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:45-54
Timestamp: 2025-06-26T15:03:23.529Z
Learning: In AbstractMostroNotifier, state updates occur for all messages regardless of timestamp to hydrate the OrderNotifier from MostroStorage during sync, while handleEvent is only called for recent messages (within 60 seconds) to prevent re-triggering side effects like notifications and navigation for previously handled messages. This design prevents displaying stale notifications when the app is reopened or brought to the foreground.
Applied to files:
lib/services/mostro_service.dart
📚 Learning: 2025-06-08T23:54:09.987Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 110
File: lib/features/trades/widgets/mostro_message_detail_widget.dart:134-141
Timestamp: 2025-06-08T23:54:09.987Z
Learning: In the OrderState refactor, public keys should be accessed via `tradeState.peer?.publicKey` from the OrderState instance rather than through `session?.peer?.publicKey`, as the OrderState encapsulates peer information directly.
Applied to files:
lib/services/mostro_service.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/**/screens/**/*.dart : Keep UI code declarative and side-effect free - use post-frame callbacks for side effects like SnackBars/dialogs
Applied to files:
lib/features/logs/screens/logs_screen.dartlib/core/app_routes.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/**/screens/**/*.dart : Use `S.of(context)!.yourKey` for all user-facing strings instead of hardcoded text
Applied to files:
lib/features/logs/screens/logs_screen.dartlib/core/app_routes.dart
📚 Learning: 2025-07-20T23:33:17.689Z
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 215
File: lib/features/order/notfiers/order_notifier.dart:0-0
Timestamp: 2025-07-20T23:33:17.689Z
Learning: In AbstractMostroNotifier, the handleEvent method is synchronous and returns void, not Future<void>. Do not suggest adding await to super.handleEvent() calls as this would cause compilation errors.
Applied to files:
lib/features/relays/relays_notifier.dart
🪛 LanguageTool
docs/LOGGING_IMPLEMENTATION.md
[grammar] ~84-~84: Ensure spelling is correct
Context: ... 4: Full migration (Pending) Scope: - Remplace Logger() local instances for logger_ser...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
docs/LOGGING_IMPLEMENTATION.md
24-24: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (20)
lib/core/app_routes.dart (1)
288-296: LGTM! Logs route properly integrated.The new
/logsroute follows the established pattern with appropriate page builder and transition configuration.lib/services/deep_link_service.dart (1)
8-8: LGTM! Logger service migration is clean.The migration from private
_loggerto the centralizedloggerservice is consistent throughout the file and aligns with the PR's logging architecture.lib/background/desktop_background_service.dart (1)
26-36: LGTM! Isolate logging setup is correct.The logger initialization properly handles the optional
loggerSendPortfrom args and configures the isolate logger with appropriate printer and output for cross-isolate communication.lib/main.dart (1)
22-23: No error handling needed. TheinitIsolateLogReceiver()function is synchronous, has no throwing operations, and already includes internal error handling for message processing. The call is correct as written.Likely an incorrect or invalid review comment.
lib/features/settings/settings_notifier.dart (1)
3-3: LGTM! Clean migration to centralized logging.The migration from private
_loggerto the global logger fromlogger_serviceis complete and consistent throughout the file. The newupdateLoggingEnabledmethod follows existing patterns and correctly persists the logging preference.Also applies to: 53-53, 62-62, 92-95, 105-105, 116-116, 141-141, 149-149
lib/features/settings/settings_screen.dart (1)
103-105: LGTM! Dev Tools card implementation follows existing patterns.The new Dev Tools section is properly integrated into the Settings screen. The
_buildDevToolsCardmethod follows the same structure as other card builders in the file, and the navigation to/logsroute is correctly implemented.Also applies to: 499-595
lib/features/subscriptions/subscription_manager.dart (1)
6-6: LGTM! Clean migration and critical bug fix.The migration to centralized logging is complete and consistent. The new
_initializeExistingSessionsmethod is properly documented with clear comments explaining its purpose (fixing stuck orders on app restart). This ensures subscriptions are created for existing sessions whenfireImmediately: falseis used, which aligns with the coding guideline for SubscriptionManager usage.Also applies to: 35-35, 46-46, 58-71, 62-62, 65-65, 68-68, 75-75, 93-93, 110-110, 113-113, 166-166, 184-184, 272-272, 274-274, 297-297, 322-322
lib/l10n/intl_en.arb (1)
1238-1291: All logging UI keys are correctly present in the Spanish and Italian ARB files. The coding guideline requirement has been satisfied—all new localization keys have been added to en, es, and it files.lib/services/mostro_service.dart (1)
8-8: Centralized logging + restore/dispute filtering look correct and robust
- Using the shared
loggerfromlogger_service.dartininit(),dispose(),_onData(),_maybeLinkChildOrder(),_prepareChildOrderIfNeeded(), andpublishOrder()is consistent with the new logging architecture._isRestorePayload()is defensively written (null/type checks, specific-field detection) and safely used in_onData()to skip restore-only payloads without risking type errors.- The extra guards in
_onData()(result is! List || result.isEmpty, dispute‑DM skip, restore‑payload skip) make the handler more robust against malformed or non‑DM content without changing the happy path.- Using
decryptedEvent.id ?? event.id ?? 'msg_...'formessageKeyis a reasonable fallback hierarchy and should not break existing storage semantics.- New info/warn/error logs around child‑order preparation and DM receipt are well‑scoped and leverage the central redaction/formatting pipeline.
No changes required here.
Also applies to: 43-95, 97-159, 181-183, 293-319, 355-356
lib/services/nostr_service.dart (1)
118-147: Publish/teardown guards and logging improvements are solid
- The explicit
_isInitializedcheck plus the new “no relays configured” guard inpublishEvent()provide clearer failure modes and avoid relying on internal assertions.- Logging of relay lists on publish, as well as success/failure logs, should be very helpful in diagnosing connectivity or configuration issues.
disconnectFromRelays()now logs a clear message on teardown, improving observability without changing behavior.No changes needed here.
Also applies to: 178-184
lib/features/relays/relays_notifier.dart (1)
120-157: Smart relay URL normalization + connectivity validation align with relays guidelines
normalizeRelayUrl()andisValidDomainFormat()enforce:
- trimmed, lower‑cased host names,
wss://only (rejectsws://andhttp(s)),- non‑IP domains with at least one dot and reasonable label lengths.
testRelayConnectivity()correctly implements the two‑tier validation strategy:
- First a full Nostr protocol test (
REQ/EVENT/EOSEvia_testNostrProtocol),- then a basic WebSocket fallback if the protocol test fails.
addRelayWithSmartValidation()wires this into UX cleanly:
- Normalizes input and maps specific failure modes to localized messages (
errorOnlySecure,errorNoHttp,errorInvalidDomain,errorAlreadyExists,errorNotValid).- Prevents duplicates against the in‑memory state.
- Only adds the relay if
testRelayConnectivity()reports it as healthy.- Automatically removes a relay from the blacklist if a user manually re‑adds it (using the normalized URL) and stores it as a
RelaySource.user.Assuming existing blacklist entries are already stored using normalized URLs (which seems consistent with how state URLs are generated), this behavior is correct and matches the documented relay validation strategy.
Also applies to: 161-170, 331-400
lib/features/settings/settings.dart (1)
10-24: Settings extensions for logging are consistent and backward compatible
- New fields
customLogStorageDirectoryandisLoggingEnabledare cleanly integrated into:
- The constructor (with
isLoggingEnableddefaulting tofalse),copyWith(using??to preserve existing values),toJson/fromJson(withisLoggingEnableddefaulting tofalsewhen absent).- This keeps deserialization safe against older stored data and ensures the logging flag is always well‑defined.
- If you later need to explicitly clear
customLogStorageDirectory(set it back tonull), you may want a dedicatedclearCustomLogStorageDirectoryflag similar toclearDefaultLightningAddress, but that can be added in Phase 2 when the directory selection UI is implemented.No immediate changes required.
Also applies to: 26-52, 55-81
lib/features/logs/screens/logs_screen.dart (1)
61-125: LogsScreen wiring (toggle, stats, export, clear) is cohesive and follows UI guidelines
- Logging toggle:
_buildStatsHeadercorrectly derivesisLoggingEnabledfromsettingsProvider._showPerformanceWarningenablesMemoryLogOutput.isLoggingEnabledonly after user confirmation and persists viasettingsProvider.notifier.updateLoggingEnabled(true)._disableLoggingAndSavedisables capture, clears the in‑memory buffer, and persists the flag asfalse.- UI behavior:
- All user‑visible text goes through
S.of(context)!and uses the new logging strings (performance warning, totals, max entries, clear/confirm, etc.).- The stats header properly shows
filteredCountvs total and respectsConfig.logMaxEntries.- Clear‑logs flow uses a confirmation dialog before calling
MemoryLogOutput.instance.clear()and triggers a localsetStateto refresh the list.- Export/share uses a temp file with a clearly formatted header and guards
_isExportingwith button disabling andmountedchecks.- Snackbars and dialogs are invoked from event handlers (not during build) and respect
mountedwhere async is involved.From this file’s perspective the Phase‑1 logging UI is well‑structured and consistent with the rest of the app’s patterns.
Also applies to: 127-187, 189-270, 498-519, 559-600, 602-666
lib/l10n/intl_es.arb (1)
1210-1268: Spanish logging UI strings are complete and consistent with usage
- The new
@_comment_logging_uiblock provides all the keys referenced by the logs UI and Dev Tools card (logCapture,capturingLogs,performanceWarning*, level labels, empty state, copy/share/export, clear‑logs confirmation,devTools,devToolsWarning,viewAndExportLogs).- Placeholders for:
totalLogs(count: int),maxEntries(count: int),logsSavedTo(path: String)
match how the Dart code invokes these methods.- JSON structure remains valid (trailing commas handled correctly), and terminology is consistent with the rest of the Spanish UI.
Looks good.
lib/l10n/intl_it.arb (1)
1265-1323: Italian logging UI localization block matches the new feature set
- The new
@_comment_logging_uisection defines all required strings for the logs screen and Dev Tools card (logCapture,capturingLogs,clearLogs,totalLogs,maxEntries,searchLogs, level labels, empty state, copy/share/export flows, clear confirmation,devTools,devToolsWarning,viewAndExportLogs).- Placeholder metadata for:
totalLogs(count: int),maxEntries(count: int),logsSavedTo(path: String)
correctly matches the Dart calls.- Wording is clear and consistent with other Italian strings (e.g., “Strumenti di Sviluppo”, “Esporta Log”).
No changes needed.
lib/services/logger_service.dart (5)
103-123: LGTM!The
LogEntryclass is well-designed as an immutable data structure with a usefulformat()method for string representation.
172-184: LGTM!The
_MultiOutputclass cleanly implements the composite pattern to forward logs to multiple outputs.
186-253: LGTM!The
SimplePrinterimplementation correctly extracts service and line information from stack traces with appropriate filtering of internal frames.
255-266: LGTM!The
_ProductionOptimizedFiltercorrectly implements the debug vs. release behavior as documented, ensuring console logging only occurs when appropriate.
268-288: LGTM!The lazy-initialized logger is well-configured with appropriate settings for debug and release modes. The use of
PrettyPrinterfor the main logger provides detailed output during development whileSimplePrinteris used for isolates.
- Migrate mobile_background_service to use shared logger_service - Convert MemoryLogOutput to ChangeNotifier for reactive updates - Add ChangeNotifierProvider for reactive log state - Update LogsScreen to watch logsProvider (auto-updates UI) - Centralize buffer logic in addEntry() and _maintainBufferSize() - Remove code duplication in addLogFromIsolate()
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
lib/services/logger_service.dart (3)
161-170: Refine the buffer trimming logic to avoid over-deletion.The current logic deletes
logBatchDeleteSizeentries whenever the buffer exceedslogMaxEntries, regardless of how many entries are over the limit. This can result in aggressive over-deletion.For example, with
logMaxEntries = 1000andlogBatchDeleteSize = 100:
- Buffer at 1001 entries → deletes 100 → leaves 901 (99 below limit)
- Buffer at 1005 entries → deletes 100 → leaves 905 (95 below limit)
🔎 Suggested improvement
void _maintainBufferSize() { if (_buffer.length > Config.logMaxEntries) { - final deleteCount = _buffer.length < Config.logBatchDeleteSize - ? _buffer.length - Config.logMaxEntries - : Config.logBatchDeleteSize; + final excess = _buffer.length - Config.logMaxEntries; + final deleteCount = excess < Config.logBatchDeleteSize + ? excess + : Config.logBatchDeleteSize; if (deleteCount > 0) { _buffer.removeRange(0, deleteCount); } } }This ensures we only delete the excess when it's small, and use batch deletion only when the excess itself exceeds the batch size.
135-137: Clarify the StackTrace.current fallback behavior.The comments state "Use StackTrace.current as fallback to get accurate caller info," but
StackTrace.currentcaptures the stack at this location (withinlogger_service.dart), not at the original log call site. While this fallback is reasonable when no stack trace is available, the comment is misleading.🔎 Suggested comment update
- // Use StackTrace.current as fallback to get accurate caller info + // Use StackTrace.current as fallback when no stack trace is provided final stackTrace = event.origin.stackTrace ?? StackTrace.current;Also applies to: 195-197
223-255: Stack trace parsing uses fallback handling that gracefully degrades on unsupported formats.The regex patterns (lines 237, 245) may not match all stack trace formats across Dart versions and platforms (iOS, Android, web, desktop), but the code handles mismatches safely by returning
{'service': 'Unknown', 'line': '0'}. The two-tier regex approach and filtering of known edge cases (async frames, stdlib, logger frames) provide reasonable robustness. Testing across target platforms would strengthen confidence in these patterns, but the current fallback mechanism prevents silent failures.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
lib/background/mobile_background_service.dartlib/features/logs/providers/logs_provider.dartlib/features/logs/screens/logs_screen.dartlib/services/logger_service.dart
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/features/logs/providers/logs_provider.dart
- lib/features/logs/screens/logs_screen.dart
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{dart,flutter}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{dart,flutter}: Runflutter analyzeafter any code change - Mandatory before commits to ensure zero linting issues
Runflutter testafter any code change - Mandatory before commits to ensure all unit tests pass
Files:
lib/services/logger_service.dartlib/background/mobile_background_service.dart
**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.dart: Use Riverpod for all state management - encapsulate business logic in Notifiers and access data only through repository classes
All code comments must be in English - use clear, concise English for variable names, function names, and comments
Always checkmountedbefore using BuildContext after async operations to prevent errors on disposed widgets
Useconstconstructors where possible for better performance and immutability
Remove unused imports and dependencies to maintain code cleanliness and reduce build size
**/*.dart: Application code should be organized underlib/, grouped by domain withlib/features/<feature>/structure, shared utilities inlib/shared/, dependency wiring inlib/core/, and services inlib/services/
Persistence, APIs, and background jobs should live inlib/data/andlib/background/; generated localization output must be inlib/generated/and must stay untouched
Applyflutter format .to enforce canonical Dart formatting (two-space indentation, trailing commas) before committing
Resolve every analyzer warning in Dart code
Name Riverpod providers using the<Feature>Provideror<Feature>Notifierconvention
Localize all user-facing strings via ARB files and access them withS.of(context)rather than hard-coded literals
Files:
lib/services/logger_service.dartlib/background/mobile_background_service.dart
lib/services/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Access NostrService for all Nostr protocol interactions - NostrService manages relay connections and messaging
Files:
lib/services/logger_service.dart
🧠 Learnings (4)
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/services/nostr_service.dart : Manage all relay connections and Nostr messaging through NostrService - automatically reconnect when relay list updates
Applied to files:
lib/background/mobile_background_service.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/services/**/*.dart : Access NostrService for all Nostr protocol interactions - NostrService manages relay connections and messaging
Applied to files:
lib/background/mobile_background_service.dart
📚 Learning: 2025-05-08T16:29:52.154Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 74
File: lib/background/background.dart:74-77
Timestamp: 2025-05-08T16:29:52.154Z
Learning: In the Mostro Mobile background service architecture, events aren't stored by the background process. Instead, the background service only checks if events exist in the eventStore and sends notifications for new ones, while the foreground process is responsible for storing and processing events. This is an intentional design decision to separate concerns.
Applied to files:
lib/background/mobile_background_service.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/subscriptions/subscription_manager.dart : Use SubscriptionManager with `fireImmediately: false` during SessionNotifier initialization to prevent premature execution
Applied to files:
lib/background/mobile_background_service.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (7)
lib/services/logger_service.dart (5)
52-82: Past refactoring suggestion has been successfully implemented.The encapsulation violation and code duplication flagged in the previous review have been resolved. The function now correctly calls
MemoryLogOutput.instance.addEntry()(line 75) instead of directly accessing the private_buffer, and buffer management is properly centralized in theaddEntry()and_maintainBufferSize()methods.
33-50: Verify the aggressive character filtering intent.Line 47 removes all characters except alphanumerics and a narrow whitelist (
.:,!?\-_/[]). This strips out parentheses, quotes, braces, angle brackets, and other punctuation commonly found in error messages and stack traces, which may reduce log readability.For example:
"Error in foo(bar)"becomes"Error in foo bar ""Object {id: 123}"becomes"Object id 123 "If this aggressive filtering is intentional for security/sanitization when exporting logs, consider adding a comment explaining the rationale.
293-322: Well-designed isolate log forwarding.The implementation correctly:
- Logs to the isolate's console for immediate visibility (lines 301-304)
- Forwards sanitized messages to the main thread via SendPort when available (lines 306-320)
- Sanitizes sensitive data before transmission (line 311)
This dual-output approach ensures logs are visible during development while enabling centralized capture.
155-159: Performance consideration already addressed by design.The
addEntry()method callsnotifyListeners()on every log entry, which could trigger UI rebuilds for each log. However, the PR objectives indicate this is addressed through:
- A toggle to enable/disable log capture
- A performance warning shown to users
This design appropriately balances functionality with performance control.
273-291: Well-configured logger for development and production.The lazy-initialized logger appropriately adapts to the environment:
- Pretty formatting with colors and emojis for development readability
- Console output disabled in release builds (line 285)
- Higher log level threshold (warning) in production (line 287)
- Respects the
isLoggingEnabledflag via_ProductionOptimizedFilterThis configuration balances developer experience with production performance.
lib/background/mobile_background_service.dart (2)
42-51: Isolate logging correctly integrated.The
loggerSendPortis properly passed to the background service in both start sequences (lines 46, 158), enabling background isolate logs to be captured by the centralized logging system. This integration aligns with the isolate-aware logging architecture introduced inlogger_service.dart.Also applies to: 155-160
48-48: Consistent logging implementation across service lifecycle.All logging statements use appropriate log levels:
- Debug for detailed information like settings
- Info for lifecycle events (start, stop, ready, subscription)
- Error for exception handling
The centralized logger is consistently used throughout the background service lifecycle.
Also applies to: 55-55, 59-59, 71-71, 130-130, 140-140, 155-155
There was a problem hiding this comment.
@BraCR10 Me preocupa que el PR sigue siendo muy grande, 19 archivos modificados son muchos, quizás se pudiera dividir aún más
Al probarlo ocurre esto:
Si habilito capturar los logs y cierro la app, al volver a abrir el botón sale como que sigue habilitado y dice "Capturando registros" pero no lo está.
|
Al cambiar de instancia de Mostro, me aparecieron estos errores en los logs de la app durante la ejecución (en la terminal donde hice flutter run), pero no se muestran en los logs que captura la app. En la app: |
ok entonces voy a cerrar este y dividirlo en fases mas pequeñas.
Esto se debe a que en esta implementación aun no se han migrado todos los logs, solo los de algunos archivos, los logs de restore aun no estan migrados |
2 participants
Phase 1: Core Logging UI
This PR implements basic in-app logging system with UI controls and multi-language support.
Current Implementation
Testing
Navigate to Settings > Dev Tools > Logs to access logging controls.
Next Phases
See
docs/LOGGING_IMPLEMENTATION.mdfor complete documentation.Summary by CodeRabbit
New Features
Settings
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.