Skip to content

fix(server): log an unreadable config at warn, not error - #33

Merged
hyperpolymath merged 1 commit into
mainfrom
fix/log-severity-config-reads
Sep 21, 2026
Merged

hyperpolymath merged 1 commit into
mainfrom
fix/log-severity-config-reads

Conversation

@hyperpolymath

Copy link
Copy Markdown
Owner

What

_read_databases (src/server/routes/databases.jl) and _read_primers
(src/server/routes/config.jl) both log at @error with a full backtrace, then
answer json_error(400, ...). This changes those two to @warn and keeps the
reason string.

Why this is a fix and not a mute

The status the route already assigns is the discriminator. json_error(400, ...)
is the function's own verdict that a malformed config/databases.yml or
config/primers.yml is an operator-fixable input, not a server fault.
Logging that at error severity with a stack trace reports a routine bad file as
if the server had crashed.

The backtrace also carried no diagnostic value here: it pointed into YAML.jl's
parser, never at the offending line. What actually names the defect is
sprint(showerror, e), and that is kept as reason=. Only the backtrace goes.

Deliberately NOT changed — 3 of the 5 route @error sites stay as they are

site status disposition
routes/databases.jl:31 400 → @warn
routes/config.jl:272 400 → @warn
routes/analysis_config.jl:144 500 unchanged
routes/annotations.jl:318 500 unchanged
routes/composition.jl:370 500 unchanged

A 500 is the route saying it did not expect this. There the backtrace is the
only thing separating a data problem from a code bug, so dropping it would be
muting a diagnostic rather than repairing one.

analysis_config.jl already draws exactly this line itself: its ArgumentError
branch returns 400 with no logging at all, while everything else logs at
@error and returns 500. That is the existing precedent this PR follows rather
than overrides.

src/server/routes/pipeline.jl:520 is untouched — it sets merged_results[i] = nothing
inside a Threads.@threads loop with no json_error at all. That is the
silent-degradation shape tracked in #31, not a severity question.

Verification

  • Both files checked with Meta.lower after editing, not just Meta.parseall —
    a lowering error (e.g. an orphaned break) passes parseall silently.
  • No test asserts on these sites. The suite's only two @test_logs are
    (:warn, ...) on filter_table and _apply_single_mapping! in
    TaxonomyTableTools; neither reaches _read_databases or _read_primers, and
    nothing matches databases_unreadable / primers_unreadable / is not readable.
  • Cut from main; touches only src/, so it could not conflict with fix: clear all 27 SonarCloud findings and unblock the Julia analysis-config step #29
    (frontend-only apart from one test file).

🤖 Generated with Claude Code

https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm

`_read_databases` and `_read_primers` both answer `json_error(400, ...)`: the
route's own verdict that a malformed `config/databases.yml` or `config/primers.yml`
is an operator-fixable input, not a server fault. Logging that at `@error` with a
full backtrace reports a routine bad file as if the server had crashed, and the
backtrace pointed into YAML.jl's parser rather than at the offending line, so it
carried no diagnostic value either.

The reason string is kept -- `reason=sprint(showerror, e)` is what actually names
the YAML defect. Only the backtrace goes.

Deliberately NOT changed: the three 500-class sites, which stay at `@error` with
their backtraces --

  src/server/routes/analysis_config.jl:144
  src/server/routes/annotations.jl:318
  src/server/routes/composition.jl:370

A 500 is the route saying it did not expect this, and there the backtrace is the
only thing separating a data problem from a code bug. Dropping it would be muting
a diagnostic rather than fixing one. `analysis_config.jl` already draws this line
itself: its `ArgumentError` branch returns 400 with no logging at all, while
everything else logs at `@error` and returns 500.

`src/server/routes/pipeline.jl:520` is untouched -- it is the silent-degradation
shape tracked in #31, not a severity question.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm
@coderabbitai

coderabbitai Bot commented Sep 21, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 34 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 4c90692f-6e25-41c8-b684-9948cd2a468a

📥 Commits

Reviewing files that changed from the base of the PR and between f699295 and 33aad5a.

📒 Files selected for processing (2)
  • src/server/routes/config.jl
  • src/server/routes/databases.jl

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.

@sonarqubecloud

Copy link
Copy Markdown

@hyperpolymath
hyperpolymath merged commit 76c92f1 into main Sep 21, 2026
4 checks passed
@hyperpolymath
hyperpolymath deleted the fix/log-severity-config-reads branch September 21, 2026 17:24
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