Skip to content

fix(loader): surface template/checks/metrics load errors as ERRORs and expose count in /api/system/health #37

Description

@SckyzO

fix(loader): surface template/checks/metrics load errors as ERRORs and expose count in /api/system/health

Symptom

load_catalog(), load_checks_library(), and load_metrics_library() (in src/rackscope/model/loader.py) each iterate over user-provided YAML files. When any individual file fails to parse, the loader logs a WARNING and continues — silently dropping the failed file and any data that depended on it.

# src/rackscope/model/loader.py:60-67 (load_catalog), :345-352 (load_checks),
#                                     :448-455 (load_metrics)
for path in candidate_files:
    try:
        # parse and append
        ...
    except Exception:
        logger.warning("Failed to load %s: ...", path)
        continue

Concrete failure scenarios this masks:

  1. Corrupt YAML in a template file — the device that references this template is loaded without its template. Downstream consumers (check planner, telemetry queries, frontend rendering) hit a KeyError or render a placeholder, with no trace back to the actual cause.
  2. Schema drift after an upgrade — a template file written for v0.X uses a field removed in v1.0. The file is silently skipped. The operator notices days later that "some devices don't have specs anymore".
  3. File permissions change after apt upgrade or volume remount — a single file becomes unreadable. The loader skips it. The dashboard shows fewer items than yesterday, with no error.

In every case, the system looks healthy (no ERROR, no traceback, the API returns 200) while data integrity is degraded.

Technical Analysis

Three problems with the current pattern:

  1. logger.warning is the wrong level. A failed load is not "something to check at your leisure" — it is an active configuration error producing downstream data corruption. The right level is ERROR with exc_info=True.

  2. The error count is not exposed anywhere observable. An operator running curl /api/system/health sees green. There is no way to know "this instance is running with 3 templates failing to load".

  3. No fail-fast option. For strict environments (HPC clusters, regulated deployments) where partial loads are unacceptable, there is no way to make the loader refuse to start with a degraded catalog.

This is the loader-side equivalent of the AP-02 pattern fixed in Issue #32 (silent except: pass on plugin hot-reload) — same "swallow + continue, no signal" anti-pattern in a different module.

Proposed Fix

1. Upgrade the log level and preserve the traceback

# In each of the three loaders
for path in candidate_files:
    try:
        # parse and append
        ...
    except Exception as exc:
        load_errors.append(LoadError(path=path, exc=exc))
        logger.error(
            "Failed to load %s (file skipped, dependent records may be incomplete): %s",
            path,
            exc,
            exc_info=True,
        )
        continue

load_errors is a new module-level list (or, cleaner, an attribute on the loader result object). The traceback in exc_info=True lands in LogBuffer and is visible in the Logs UI.

2. Expose load errors in /api/system/health

Extend the existing health response (or create a new /api/system/load-status if separation is preferred):

{
    "status": "ok",
    "uptime_seconds": 12345,
    "prometheus_reachable": true,
    "load_errors": {
        "catalog": [
            {"path": "config/catalog/server-x.yaml", "error": "YAMLError: ..."},
        ],
        "checks": [],
        "metrics": [],
    }
}

load_errors is an empty dict (or all empty lists) on a healthy load. The shape is forward-compatible: future loaders (rack templates, …) add their own bucket.

The frontend Dashboard can surface a small "⚠️ N load errors" banner linking to Logs when load_errors is non-empty. Out of scope for this PR (file as follow-up) — the API surface is the gating change here.

3. Optional strict mode

Add a config flag in app.yaml:

loader:
  strict: false   # default — current lenient behaviour, with ERROR logs
                  # true → any failure to load a single file aborts startup

strict: true is for regulated environments where partial loads are unacceptable. Default is false to preserve backward compatibility: existing deployments keep their lenient behaviour, only the log level is raised.

This flag is opt-in and additive — fully consistent with the project's OSS philosophy ("ne pas modifier le comportement par défaut").

Test Checklist

Unit tests

  • tests/unit/model/test_loader_errors.py: load_catalog with a YAML-syntax-error file → returns successfully, error in load_errors, log captured at level ERROR with exc_info
  • Same for load_checks_library and load_metrics_library — three parallel test cases
  • Strict mode: loader.strict=true + corrupt file → loader raises, backend refuses to start with a clear error message naming the file
  • Lenient mode: loader.strict=false + corrupt file → loader succeeds with the rest of the catalog (non-regression of current behaviour)

Functional tests

  • GET /api/system/health with a clean catalog → load_errors is empty
  • GET /api/system/health after loading with a deliberately corrupt file → load_errors.catalog lists the file with its error message
  • Health endpoint shape is backward-compatible: existing fields unchanged, only adds the new load_errors key

Non-regression

  • Existing deployments with clean catalogs see no log level change (no spurious ERROR entries)
  • Frontend that ignores unknown response fields keeps working (forward-compat shape)

Impact and Severity

  • Audience affected: every operator running with at least one YAML file that has gone bad — most often after an upgrade or filesystem migration. Today, they have no signal until they notice missing data; after the fix, they see an ERROR in logs and a flag in /api/system/health.
  • Severity: medium. Operational visibility. Not a security issue; a reliability and observability one.
  • Priority: Sprint 3 of the audit roadmap. Ships when convenient, no blocking dependency.

Breaking Changes

None at the API contract level. Behavioural:

  • Logs now contain ERROR entries where they previously contained WARNING entries. Log-scraping rules that count ERROR-level messages will see an uptick for affected deployments (which is exactly the point — those entries describe real bugs).
  • /api/system/health response adds a new load_errors field. Existing consumers that ignore extra fields keep working.

Release notes:

The catalog / checks / metrics loaders now log file-level failures at ERROR level instead of WARNING. The /api/system/health endpoint also exposes the per-loader list of failures so they can be acted on without grepping logs.

Related

  • Audit ref: MAIN-04 in AUDIT_ARCHITECTURAL.md
  • Same anti-pattern as AP-02 (Issue fix(plugins): log hot-reload failures instead of silencing them #32, plugin hot-reload silencing) — fix is consistent with that PR's resolution
  • Future follow-up: frontend Dashboard banner surfacing load_errors count to the operator at-a-glance — separate UI issue if escalated

Out of Scope

  • Auto-correction or auto-repair of corrupt files — out of scope, would be unsafe.
  • Validation against a schema before load — Pydantic already enforces structural validation; this issue only changes the visibility of failures, not the validation logic.
  • Adding a CLI command to validate the catalog without booting the API — possible future addition, not in scope.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions