[MNT] Add logging for user errors & warnings

[alyssadai]

• Closes Implement logging #467

Changes proposed in this pull request:

• Replace warnings.warn (UserWarning) app logging
• Log an informative error in addition to raising errors when app startup checks fail

Checklist

This section is for the PR reviewer

• PR has an interpretable title with a prefix ([ENH], [FIX], [REF], [TST], [CI], [MNT], [INF], [MODEL], [DOC]) (see our Contributing Guidelines for more info)
• PR has a label for the release changelog or skip-release (to be applied by maintainers only)
• PR links to GitHub issue with mention Closes #XXXX
• Tests pass
• Checks pass
• If the PR changes the SPARQL query template, the default Neurobagel query file has also been regenerated

For new features:

• Tests have been added

For bug fixes:

• There is at least one test that would fail under the original bug conditions.

Summary by Sourcery

Use centralized logging for configuration and runtime user-facing errors and perform configuration and auth checks before the FastAPI lifespan starts to allow clean process termination.

Bug Fixes:

• Ensure failed environment, configuration, or remote data checks terminate the process cleanly instead of raising unhandled runtime errors that surface as FastAPI/Starlette tracebacks.

Enhancements:

• Introduce a shared logger helper module and replace warnings and RuntimeError-based error reporting with structured logging across the app.
• Run environment validation, authentication checks, and vocabulary/context initialization before the FastAPI lifespan to avoid noisy startup tracebacks.

Summary by Sourcery

Use centralized logging for configuration and runtime user-facing errors and ensure startup failures terminate cleanly without noisy tracebacks.

Bug Fixes:

• Ensure missing or invalid environment, configuration, or authentication settings cause a clean startup failure with a logged error instead of unhandled FastAPI tracebacks.
• Handle HTTP errors when fetching remote configuration or vocabulary data by logging a clear message and terminating with a controlled exception.

Enhancements:

• Introduce a shared logger utility and replace warnings and direct RuntimeError raises in core modules with structured logging plus controlled error raising.
• Configure application-wide logging format and level and use it for environment validation, auth checks, and vocabulary-related warnings.
• Tighten startup validation by checking community configuration names and datasets metadata paths up front so misconfiguration is surfaced immediately.

Tests:

• Update existing tests to assert on logged warnings and errors using pytest caplog instead of warning filters, covering environment, configuration, and vocabulary-related conditions.
• Extend settings tests to account for the NB_CONFIG environment variable in the expected configuration.

[sourcery-ai]

Reviewer's Guide

Centralizes logging for configuration and runtime user-facing issues, replaces warnings/naive RuntimeErrors with structured logging plus a helper, and moves critical environment/config/configuration checks out of the FastAPI lifespan while updating tests to assert on logged messages instead of warnings.

Sequence diagram for updated FastAPI startup configuration checks

sequenceDiagram
    actor Operator
    participant Process as uvicorn_process
    participant Main as main_module
    participant Logger as logger_helper
    participant Env as env_settings
    participant Sec as security_module
    participant Util as utility_module
    participant App as FastAPI_app

    Operator->>Process: start app.main:app
    Process->>Main: import main module
    Main->>Logger: logging.basicConfig(...)
    Main->>Logger: get_logger(__name__)
    Main->>Env: validate_environment_variables()
    Env->>Logger: get_logger(__name__)
    Env->>Logger: log_and_raise_error(RuntimeError, message) on invalid env
    Logger-->>Process: log error and raise RuntimeError
    Process-->>Operator: clean termination before FastAPI lifespan

    Note over Main,Sec: If environment variables are valid
    Main->>Sec: check_client_id()
    Sec->>Logger: get_logger(__name__)
    Sec->>Logger: log_and_raise_error(RuntimeError, message) on missing client_id
    Logger-->>Process: log error and raise RuntimeError
    Process-->>Operator: clean termination before FastAPI lifespan

    Note over Main,Util: If auth config is valid
    Main->>Util: request_data(vocabulary_url, err_message)
    Util->>Logger: get_logger(__name__)
    Util->>Util: httpx.Client().get(url)
    Util-->>Main: data or
    Util->>Logger: log_and_raise_error(RuntimeError, message) on HTTPError
    Logger-->>Process: log error and raise RuntimeError

    Note over Main,App: Only when all checks pass
    Main->>App: create FastAPI app instance
    App-->>Process: lifespan startup proceeds without noisy tracebacks

Loading

Class diagram for centralized logging helper and its usage

classDiagram
    class LoggerHelper {
        +get_logger(name: str) logging.Logger
        +log_and_raise_error(logger: logging.Logger, exception_type: Type~Exception~, message: str) None
    }

    class MainModule {
        +logger: logging.Logger
        +validate_environment_variables() None
    }

    class UtilityModule {
        +logger: logging.Logger
        +request_data(url: str, err_message: str) Any
    }

    class SecurityModule {
        +logger: logging.Logger
        +check_client_id() None
    }

    class CrudModule {
        +logger: logging.Logger
        +get_terms(...) Any
    }

    LoggerHelper <.. MainModule : uses
    LoggerHelper <.. UtilityModule : uses
    LoggerHelper <.. SecurityModule : uses
    LoggerHelper <.. CrudModule : uses

    MainModule o-- SecurityModule : calls check_client_id
    MainModule o-- UtilityModule : calls request_data
    CrudModule --> UtilityModule : uses logging for term warnings

    class Logging {
        +basicConfig(level: int, format: str, datefmt: str) None
        +getLogger(name: str) logging.Logger
    }

    Logging <.. LoggerHelper : wraps

Loading

File-Level Changes

Change	Details	Files
Introduce centralized logging utilities and apply them to configuration, security, HTTP request, and CRUD layers.	Add a logger helper module providing get_logger and log_and_raise_error utilities to standardize logging and error raising behavior. Initialize basic logging configuration and a module-level logger in the FastAPI main module. Replace direct warnings.warn usage in CRUD logic with logger-based warnings for unrecognized vocabularies. Use log_and_raise_error instead of direct RuntimeError raises when HTTP requests for configuration data fail in the utility layer. Use log_and_raise_error instead of direct RuntimeError raises for missing client ID when auth is enabled in the security layer.	app/api/logger.py app/main.py app/api/crud.py app/api/utility.py app/api/security.py
Tighten startup/configuration checks and adjust application composition to support clean exits without noisy tracebacks.	Refactor validate_environment_variables to log errors and raise via the logging helper for missing graph credentials, unrecognized configs, and non-existent datasets metadata files. Convert the allowed-origins missing case from a UserWarning to a logged WARNING while preserving the guidance message. Ensure configuration fetching and validation runs outside of FastAPI lifespan context to allow the process to terminate cleanly on startup failures. Keep static app wiring (routers, middleware, docs routes, favicon URL) while cleaning minor formatting of the root HTML response.	app/main.py
Update tests to assert on logged warnings/errors instead of Python warnings and adjust fixtures for new startup behavior.	Replace pytest.warns-based assertions with caplog-based checks for environment warnings and vocabulary lookup warnings. Extend existing tests for missing credentials, unknown config, missing datasets metadata, and missing client ID to assert that a single ERROR is logged with the expected message while still raising RuntimeError. Remove filterwarnings markers that are no longer needed now that user-facing issues are logged rather than emitted as UserWarnings. Adjust tests involving settings and integration behavior to account for NB_CONFIG and the new startup flow, while keeping behavior expectations the same.	tests/test_app_events.py tests/test_attribute_factory_routes.py tests/test_security.py tests/test_query.py tests/test_settings.py

Assessment against linked issues

Issue	Objective	Addressed	Explanation
#467	Set up centralized logging for the app and use it instead of UserWarning/custom RuntimeError/ValueError for user-facing configuration and startup messages (e.g., environment variables, config, vocab fetching).	✅
#467	Replace warning-based user notifications in the request/CRUD layer (e.g., vocabulary-related warnings) with logger-based warnings.	✅
#467	Update tests so they assert on logging behavior (warnings/errors) instead of expecting UserWarnings in the console or uncaught RuntimeErrors/tracebacks from misconfiguration.	✅

Possibly linked issues

• Implement logging #467: They match exactly: the PR adds centralized logging, replaces warnings/errors, and moves startup checks pre-lifespan.

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[codecov]

Codecov Report

❌ Patch coverage is 98.30508% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 97.30%. Comparing base (0026d40) to head (ddaeff6).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
app/api/utility.py	66.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #522      +/-   ##
==========================================
+ Coverage   97.23%   97.30%   +0.07%     
==========================================
  Files          33       34       +1     
  Lines        1267     1301      +34     
==========================================
+ Hits         1232     1266      +34     
  Misses         35       35              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[alyssadai]

@sourcery-ai review

[sourcery-ai]

We've reviewed this pull request using the Sourcery rules engine

[sourcery-ai]

We've reviewed this pull request using the Sourcery rules engine

[surchs]

Thanks @alyssadai, commenting here before I have time for a full review

[surchs]

Thanks @alyssadai for working on this and sorry review took so long.

The actual logging looks great. I'm requesting changes for two reasons:

1. 95% of this PR is about an incidental problem that wasn't part of the issue, i.e. "prevent traceback during app startup". This really should not have been in the same PR. I'm not convinced this is a major problem, but if it is, we should discuss it first as a team
2. The solution you have chosen to prevent "traceback during app startup" is quite complex and introduces a number of additional side effects and workarounds for testing etc. I don't think that's proportional to the issue you are trying to prevent

So I'd say: please revert the workarounds in life-cycle so this PR is about adding consistent logging. And then we can discuss how to deal with startup tracebacks separately

[surchs]

OK, responding to myself here: The original issue was indeed about fixing the startup traceback problem, not just about logging. Sorry for missing that context. I don't know if we can find a simpler solution than what is in the PR atm to achieve this, but I'm still a bit uncomfortable with the complexity to get around the traceback.

[alyssadai]

After discussion, to avoid a difficult-to-maintain workaround, we have decided to keep the old informative error raising behaviour (which will ensure that the app exits when appropriate) and simply emit an additional error log with the same message.

[alyssadai]

@sourcery-ai review

[sourcery-ai]

We've reviewed this pull request using the Sourcery rules engine

[surchs]

Thanks @alyssadai, looks great!

🧑‍🍳

[surchs]

🍒 I actually like lifting the expected message into a constant during test setup, so you don't have to go search for the magic string in the assert.

[surchs]

🍒 same here, I like the expected_message = ... bit better

[surchs]

and more consistent if we do it in all the tests