fix(plugins): log hot-reload failures instead of silencing them

[SckyzO]

fix(plugins): log hot-reload failures instead of silencing them

Symptom

When an admin updates a plugin's configuration via PUT /api/plugins/{plugin_id}/config, the backend:

1. Writes the new config to the plugin's config file on disk (succeeds, persisted).
2. Calls plugin.on_config_reload() to apply the change at runtime (may fail).
3. Returns 200 to the caller with the new config payload.

Step 2 swallows all exceptions silently:

# src/rackscope/api/routers/plugins.py:166-172
# Hot-reload the plugin with the new config
try:
    from rackscope.api.app import APP_CONFIG

    if APP_CONFIG:
        await plugin.on_config_reload(APP_CONFIG)
except Exception:
    pass

return {"config": body.config, "source": "file", "path": config_path, "status": "saved"}

The result is an inconsistent state without any trace:

• File on disk: new config persisted ✓
• Plugin in memory: still running with the old config ✗
• Response: 200 OK, looks like everything worked ✓
• Logs: nothing ✗
• User: assumes the change is applied, may take wrong follow-up decisions

This violates the project principle: "Les exceptions doivent être capturées au bon niveau et signaler la vraie cause. Un except qui avale silencieusement une erreur et retourne une valeur par défaut cache des bugs réels."

Technical Analysis

The current code matches the audit finding AP-02. Two distinct problems:

1. The exception is not logged — even if reload failure is acceptable as non-fatal, the operator must be able to trace it. Currently impossible without attaching a debugger.

2. The HTTP response does not surface the failure — the caller has no signal at all that the in-memory plugin diverges from the persisted config. Even a soft warning in the response payload would be enough.

Why this matters in practice

• A plugin author makes a typo in a new config field validator. The admin saves the config. The file is written, but on_config_reload() raises a ValidationError. The admin sees green, refreshes, the metric / behaviour does not change. They blame caching, then the network, then their understanding of the feature.

• A plugin's reload triggers an upstream call (Slurm REST, Prometheus, etc.) that times out. The reload is partial. Same opacity.

Proposed Fix

1. Replace except: pass with structured logging

# src/rackscope/api/routers/plugins.py:166-172
reload_warning: Optional[str] = None
try:
    from rackscope.api.app import APP_CONFIG
    if APP_CONFIG:
        await plugin.on_config_reload(APP_CONFIG)
except Exception as exc:
    logger.error(
        "Plugin %s hot-reload failed after config write (config saved to disk, "
        "in-memory plugin still running with previous config): %s",
        plugin_id,
        exc,
        exc_info=True,
    )
    reload_warning = (
        f"Config persisted to disk, but in-memory hot-reload failed: {exc}. "
        "The new config will take effect on the next restart."
    )

return {
    "config": body.config,
    "source": "file",
    "path": config_path,
    "status": "saved",
    **({"reload_warning": reload_warning} if reload_warning else {}),
}

Decisions captured here:

• Level error — this is a real operational issue, not a routine event. warning would be too quiet for a state-inconsistency bug.
• exc_info=True — preserves the full traceback in the log. The LogBuffer (api/log_buffer.py) will surface it in the Logs UI.
• Non-fatal — the file write succeeded, the user's intent is preserved, the next restart will apply it. Returning a 500 would be more disruptive than informative.
• Surfacing in the response — reload_warning field added only when a failure occurred. Existing consumers ignoring extra fields keep working.
• Operator-friendly wording — the message tells the operator both what happened and what they can do (restart to apply).

2. Frontend: display the reload_warning if present

In the Settings > Plugins panel, the "Save config" button currently shows a green success toast. Update to:

• If response has no reload_warning → green toast "Plugin config saved and applied."
• If response has reload_warning → amber toast (warning variant) with the message, plus a "Restart Rackscope" suggestion linking to POST /api/system/restart.

Use the existing AlertBanner / toast component, no new design system addition.

3. Audit other except: pass patterns (optional, recommended)

This issue is the trigger for a wider one-time audit:

grep -rn "except Exception:$" src/ | xargs -I{} sh -c 'echo {}; grep -A 2 "except Exception:" $(echo {} | cut -d: -f1) | grep -B 1 "pass"'

Any other except Exception: pass discovered should be either fixed in this PR (if trivial and similar) or filed as separate issues (if they need design discussion). Document the result in the PR description so the reviewer can confirm the scope.

Test Checklist

Unit / functional tests

• New test: PUT /api/plugins/{plugin_id}/config with a plugin whose on_config_reload raises a ValueError → response 200 with reload_warning field present, error logged with exc_info
• Existing happy path: PUT /api/plugins/{plugin_id}/config with a plugin whose on_config_reload succeeds → response 200, no reload_warning field (backwards-compat shape)
• Log assertion: when reload fails, LogBuffer contains an entry with level ERROR, the plugin_id, and the exception message
• No regression: LogBuffer does not contain spurious ERROR entries when reload succeeds

Frontend tests

• Settings > Plugins: success path shows green toast (unchanged)
• Settings > Plugins: failure path with reload_warning shows amber AlertBanner with the message + restart suggestion

Security regression test

• The reload_warning field never echoes the secret_key, password_hash, or any sensitive plugin field (the warning message is built from str(exc), which is plugin-author-controlled — flag in the PR review that plugin authors must not include secrets in their exception messages; this is a documentation item, not a code defense)

Impact and Severity

• Audience affected: every admin who edits plugin config via the Settings UI or PUT /api/plugins/{plugin_id}/config and hits a runtime reload failure. Today: silently broken. After fix: visible failure with actionable message.
• Severity: medium. The current behaviour produces state inconsistency invisible to the operator. Not a security issue per se, but a real reliability and observability hole.
• Priority: ship as part of the v1.0.0 hardening series. Low-risk, ~30 lines of code, ~1 hour of work plus tests.

Breaking Changes

None. The API contract on PUT /api/plugins/{plugin_id}/config adds a new optional response field (reload_warning). The frontend handles it; older callers ignore it.

The log message is new — existing log-scraping rules that match on specific patterns are unaffected.

Related

• Audit ref: AP-02 in AUDIT_ARCHITECTURAL.md
• Companion to the v1.0.0 hardening series (Issue A fix(security): add require_admin to mutation endpoints in topology, catalog, and config routers #26, Issue D fix(security): sanitize GET /api/config — exclude TLS file paths and add plugin sanitization contract #30, Issue E for prod compose)
• Does not depend on or block any other issue