Skip to content

logs: update api response error logs#7

Merged
koukibadr merged 1 commit into
mainfrom
logs/update-error-logging
Jul 5, 2026
Merged

logs: update api response error logs#7
koukibadr merged 1 commit into
mainfrom
logs/update-error-logging

Conversation

@koukibadr

@koukibadr koukibadr commented Jul 5, 2026

Copy link
Copy Markdown
Owner

Summary by CodeRabbit

  • Bug Fixes
    • Theme updates now refresh after remote config fetches even when no cached settings are available, helping the app apply the latest appearance more reliably.
    • Improved handling of missing theme values to avoid interruptions during configuration loading.
  • Chores
    • Added response status logging for network requests to support troubleshooting.

@coderabbitai

coderabbitai Bot commented Jul 5, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The fetchConfig() method now emits ThemeData to _skinController unconditionally, using optional chaining on _cachedConfig instead of a null guard with non-null assertion. Additionally, fetchData now logs the HTTP response status code after the POST request completes.

Changes

Config emission and logging updates

Layer / File(s) Summary
Unconditional theme emission
lib/remote/fskin_remote_config.dart
fetchConfig() emits ThemeData to _skinController regardless of whether _cachedConfig is null, deriving colorScheme via optional chaining instead of a null-checked non-null assertion.
Response status logging
lib/services/skin_service.dart
fetchData logs an informational message with the HTTP response status code after the POST request completes.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

  • koukibadr/flutter_skin#5: Both PRs modify fetchConfig()'s stream emission logic for publishing ThemeData in lib/remote/fskin_remote_config.dart.

Poem

A hop, a skip, a config emitted true,
No more null-checks blocking the view,
Status codes logged with a happy little cheer,
This bunny's code is crisp and clear! 🐰
Thump thump — onward, changes near!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title matches the logging-focused API response change, though it doesn't mention the remote config stream update.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
lib/remote/fskin_remote_config.dart (1)

49-54: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Guard the stream update on a successful theme fetch
fetchConfig() still emits ThemeData(colorScheme: _cachedConfig?.skin?.colors) when fetchData() returns null or the payload has no colors. That can push the default theme to every listener and drop the last known custom theme; only add to _skinController when a valid colorScheme is present.

🤖 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/remote/fskin_remote_config.dart` around lines 49 - 54, Guard the stream
update in fetchConfig() so _skinController is only updated after
SkinService().fetchData(apiKey) returns a valid config with a non-null
colorScheme. If _cachedConfig or _cachedConfig?.skin?.colors is missing, skip
calling _skinController.add to avoid broadcasting a fallback ThemeData and
overwriting the last custom theme.
🤖 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.

Outside diff comments:
In `@lib/remote/fskin_remote_config.dart`:
- Around line 49-54: Guard the stream update in fetchConfig() so _skinController
is only updated after SkinService().fetchData(apiKey) returns a valid config
with a non-null colorScheme. If _cachedConfig or _cachedConfig?.skin?.colors is
missing, skip calling _skinController.add to avoid broadcasting a fallback
ThemeData and overwriting the last custom theme.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 12840f8b-b19e-4175-a943-a3b42a909dda

📥 Commits

Reviewing files that changed from the base of the PR and between 5ed73a9 and e6fa797.

📒 Files selected for processing (2)
  • lib/remote/fskin_remote_config.dart
  • lib/services/skin_service.dart

@koukibadr koukibadr merged commit 194f4e3 into main Jul 5, 2026
2 checks passed
@koukibadr koukibadr deleted the logs/update-error-logging branch July 5, 2026 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant