feat: setup logging service#6
Conversation
|
Warning Review limit reached
Next review available in: 53 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR introduces a new ChangesFskin Logging Integration
Estimated code review effort: 2 (Simple) | ~12 minutes Sequence Diagram(s)sequenceDiagram
participant App
participant FlutterSkin
participant FskinSubscriber
participant SkinService
participant FskinLogger
App->>FlutterSkin: init(apiKey)
FlutterSkin->>FskinLogger: logError (if apiKey blank)
FlutterSkin->>FskinSubscriber: _startStream()
FskinLogger--x FlutterSkin: logMessage(starting stream)
FskinSubscriber->>FskinLogger: logMessage(skin update event received)
FskinSubscriber-->>FlutterSkin: onSkinUpdated()
FlutterSkin->>SkinService: fetchData(apiKey)
SkinService->>FskinLogger: logMessage(fetching config)
SkinService-->>FlutterSkin: ProjectConfig or null
SkinService->>FskinLogger: logError (on failure/non-200)
FlutterSkin->>FskinLogger: logWarning (fallback theme)
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
lib/services/skin_service.dart (1)
33-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winException passed as string interpolation instead of
errorObject.Line 42 logs
'Error fetching skin configuration: $e'as a plain string rather than passing the caught exception vialogError'serrorObjectparam. This loses structured error data (type, and stack trace if that param is added) in DevTools' logging view, which is the primary value of using_logger.logErrorover a plain string message here.♻️ Proposed fix
- } catch (e) { - _logger.logError('Error fetching skin configuration: $e'); + } catch (e, stackTrace) { + _logger.logError( + 'Error fetching skin configuration', + errorObject: e, + ); return null;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/services/skin_service.dart` around lines 33 - 46, The catch block in SkinService.fetchSkinConfiguration is logging the exception as plain string interpolation instead of using logError’s errorObject parameter. Update the error handling in fetchSkinConfiguration so the caught exception is passed as structured error data to _logger.logError, keeping the message descriptive while preserving the exception type and any available stack-trace information for DevTools.lib/flutter_skin.dart (1)
75-89: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing log when neither remote colors nor fallback theme exist.
toThemeDatanow logs on the fallback path (line 81) and the remote-theme path (line 85), but thecolors == null && fallbackTheme == nullbranch falls through toreturn null(line 88) with no log entry. This silently drops the "no theme available" case from observability, which is arguably the most useful case to log.♻️ Proposed fix
if (colors == null) { if (fallbackTheme != null) { _logger.logWarning('No active theme found. Returning fallback theme.'); return fallbackTheme; } + _logger.logWarning('No active theme and no fallback theme provided. Returning null.'); } else { _logger.logMessage('Active theme found. Returning remote theme.'); return remoteTheme; } return null;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/flutter_skin.dart` around lines 75 - 89, Add logging for the no-theme-available case in toThemeData: when FskinRemoteConfig.projectConfig.skin.colors is null and fallbackTheme is also null, emit a warning before returning null. Keep the existing logs for the fallbackTheme and remoteTheme branches, and update the control flow in toThemeData so every exit path is observable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@example/lib/main.dart`:
- Around line 8-9: The example app in main.dart is committing a real apiKey as
plaintext, so replace the hardcoded secret with a non-sensitive placeholder or
load it from an untracked runtime source such as a dart-define or
environment-based config. Update the example initialization that passes apiKey
so it no longer embeds a live credential in source control, and rotate the
exposed key on the provider side.
In `@lib/services/fskin_logger.dart`:
- Around line 21-29: The `logError` method in `FskinLogger` drops
`dart:developer`’s `stackTrace`, so update its signature to accept and forward a
`stackTrace` parameter alongside `errorObject`. Pass that stack trace through to
the `log(...)` call so diagnostics preserve full exception context. Then update
the call sites in `fskin_subscriber.dart` and `skin_service.dart` to supply the
caught stack trace when they invoke `logError`.
In `@lib/services/fskin_subscriber.dart`:
- Around line 38-58: The retry logging in FSkinSubscriber is dropping the real
exception because both the Stream.listen onError handler and the future
catchError callback ignore the error value. Update the error handling in
FSkinSubscriber so the caught error is forwarded to _logger.logError via the
errorObject argument, and include stackTrace too if the logger API supports it;
keep the retry behavior unchanged in the onError and catchError paths.
---
Outside diff comments:
In `@lib/flutter_skin.dart`:
- Around line 75-89: Add logging for the no-theme-available case in toThemeData:
when FskinRemoteConfig.projectConfig.skin.colors is null and fallbackTheme is
also null, emit a warning before returning null. Keep the existing logs for the
fallbackTheme and remoteTheme branches, and update the control flow in
toThemeData so every exit path is observable.
In `@lib/services/skin_service.dart`:
- Around line 33-46: The catch block in SkinService.fetchSkinConfiguration is
logging the exception as plain string interpolation instead of using logError’s
errorObject parameter. Update the error handling in fetchSkinConfiguration so
the caught exception is passed as structured error data to _logger.logError,
keeping the message descriptive while preserving the exception type and any
available stack-trace information for DevTools.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8ce24131-aa4e-4a3d-906f-c81b92bd58cf
📒 Files selected for processing (6)
example/ios/Runner.xcodeproj/project.pbxprojexample/lib/main.dartlib/flutter_skin.dartlib/services/fskin_logger.dartlib/services/fskin_subscriber.dartlib/services/skin_service.dart
💤 Files with no reviewable changes (1)
- example/ios/Runner.xcodeproj/project.pbxproj
1 participant
Summary by CodeRabbit