Skip to content

fix: upgrade extensions config save failure log from debug to warn#7876

Closed
jeffa-block wants to merge 1 commit intoblock:mainfrom
jeffa-block:jeffa/upgrade-extensions-save-error-level
Closed

fix: upgrade extensions config save failure log from debug to warn#7876
jeffa-block wants to merge 1 commit intoblock:mainfrom
jeffa-block:jeffa/upgrade-extensions-save-error-level

Conversation

@jeffa-block
Copy link
Copy Markdown
Contributor

Why

save_extensions_map() in extensions.rs catches config save failures but logs them at debug level — effectively swallowing them in production. There's even a TODO from a maintainer asking why:

// TODO(jack) why is this just a debug statement?
tracing::debug!("Failed to save extensions config: {}", e);

Failing to persist extension configuration is a meaningful error that users and developers should see in logs.

What

  • Upgrade tracing::debug!tracing::warn! for the config save failure
  • Remove the stale TODO comment

tracing::warn is already imported and used elsewhere in this file.

How to review

2 lines changed in 1 file. Confirm warn is the right level (not error) — the app can continue functioning even if config save fails, but users should know their changes may not have persisted.

Testing

  • cargo check -p goose — compiles cleanly
  • Trigger: temporarily make the config file read-only and toggle an extension. The warning should now appear in logs instead of being hidden behind debug verbosity.

Failing to save extensions config is a meaningful error that should
be visible in logs. The previous debug-level log (with a TODO asking
why it was debug) meant this failure was silently swallowed in
production.

tracing::warn is already imported and used in this file.
@DOsinga
Copy link
Copy Markdown
Collaborator

DOsinga commented Mar 26, 2026

Squashed into #8142

@DOsinga DOsinga closed this Mar 26, 2026
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.

2 participants