fix(cli): handle TOML syntax errors gracefully (#221)

Improved error handling for configuration loading. Malformed TOML
files now trigger a formatted diagnostic message with line and
column details instead of a Python traceback.

Signed-off-by: sushant-suse <[email protected]>

Author: sushant-suse

changelog.d/175.bugfix.rst

@@ -0,0 +1 @@
+Improved error handling for configuration loading. Malformed TOML files now trigger a formatted diagnostic message with line and column details instead of a Python traceback.

src/docbuild/cli/cmd_cli.py

@@ -4,6 +4,7 @@
 import logging
 from pathlib import Path
 import sys
+import tomllib
 from typing import Any, cast
 
 import click
@@ -24,7 +25,7 @@
 from ..logging import setup_logging
 from ..models.config.app import AppConfig
 from ..models.config.env import EnvConfig
-from ..utils.errors import format_pydantic_error
+from ..utils.errors import format_pydantic_error, format_toml_error
 from ..utils.pidlock import LockAcquisitionError, PidFileLock
 from .cmd_build import build
 from .cmd_c14n import c14n
@@ -68,21 +69,70 @@ def handle_validation_error(
     :param ctx: The Click context, used to exit the CLI with an appropriate
       status code after handling the error.
     """
-    config_label = "Application" if model_class == AppConfig else "Environment"
+    # Determine which file we were working on
+    config_file = str((config_files or ["unknown"])[0])
 
-    if isinstance(e, ValidationError):
-        # Safely extract the first config file name for the error header
-        config_file = str((config_files or ["unknown"])[0])
-        format_pydantic_error(
-            e, model_class, config_file, verbose, console=CONSOLE
-        )
+    if isinstance(e, tomllib.TOMLDecodeError):
+        format_toml_error(e, config_file, console=CONSOLE)
+    elif isinstance(e, ValidationError):
+        format_pydantic_error(e, model_class, config_file, verbose, console=CONSOLE)
     else:
-        log.error("%s configuration failed validation:", config_label)
+        config_label = "Application" if model_class == AppConfig else "Environment"
+        log.error("%s configuration failed:", config_label)
         log.error("Error in config file(s): %s", config_files)
         log.error(e)
     ctx.exit(1)
 
 
+def load_app_config(
+    ctx: click.Context,
+    app_config: Path,
+    max_workers: str | None
+) -> None:
+    """Load and validate Application configuration.
+
+    :param ctx: The Click context object. The result will be added to ``ctx.obj.appconfig``.
+    :param app_config: The path to the application config file provided via CLI.
+    :param max_workers: The max_workers value from CLI options.
+    """
+    context = ctx.obj
+    result = handle_config(
+        app_config,
+        CONFIG_PATHS,
+        APP_CONFIG_BASENAMES + PROJECT_LEVEL_APP_CONFIG_FILENAMES,
+        None,
+        DEFAULT_APP_CONFIG,
+    )
+    context.appconfigfiles, raw_appconfig, context.appconfig_from_defaults = cast(
+        tuple[tuple[Path, ...] | None, dict[str, Any], bool], result
+    )
+
+    if max_workers is not None:
+        raw_appconfig["max_workers"] = max_workers
+
+    context.appconfig = AppConfig.from_dict(raw_appconfig)
+
+
+def load_env_config(ctx: click.Context, env_config: Path) -> None:
+    """Load and validate Environment configuration.
+
+    :param ctx: The Click context object. The result will be added to ``ctx.obj.envconfig``.
+    :param env_config: The path to the environment config file provided via CLI.
+    """
+    context = ctx.obj
+    result = handle_config(
+        env_config,
+        (PROJECT_DIR,),
+        None,
+        DEFAULT_ENV_CONFIG_FILENAME,
+        DEFAULT_ENV_CONFIG,
+    )
+    context.envconfigfiles, raw_envconfig, context.envconfig_from_defaults = cast(
+        tuple[tuple[Path, ...] | None, dict[str, Any], bool], result
+    )
+
+    context.envconfig = EnvConfig.from_dict(raw_envconfig)
+
 @click.group(
     name=APP_NAME,
     context_settings={"show_default": True, "help_option_names": ["-h", "--help"]},
@@ -167,76 +217,48 @@ def cli(
         ctx.ensure_object(DocBuildContext)
 
     context = ctx.obj
-    context.verbose = verbose
-    context.dry_run = dry_run
-    context.debug = debug
-
-    # --- PHASE 1: Load and Validate Application Config ---
-    (
-        context.appconfigfiles,
-        raw_appconfig,
-        context.appconfig_from_defaults,
-    ) = handle_config(
-        app_config,
-        CONFIG_PATHS,
-        APP_CONFIG_BASENAMES + PROJECT_LEVEL_APP_CONFIG_FILENAMES,
-        None,
-        DEFAULT_APP_CONFIG,
-    )
-
-    raw_appconfig = cast(dict[str, Any], raw_appconfig)
+    context.verbose, context.dry_run, context.debug = verbose, dry_run, debug
 
-    if max_workers is not None:
-        raw_appconfig["max_workers"] = max_workers
+    # State tracking for centralized error handling
+    current_model: type[BaseModel] = AppConfig
+    current_files: Sequence[Path] | None = None
 
     try:
-        context.appconfig = AppConfig.from_dict(raw_appconfig)
-    except (ValueError, ValidationError) as e:
-        handle_validation_error(e, AppConfig, context.appconfigfiles, verbose, ctx)
+        # --- PHASE 1: Load Application Config ---
+        current_model = AppConfig
+        current_files = (app_config,) if app_config else None
+        load_app_config(ctx, app_config, max_workers)
 
-    # 3. Setup logging using the validated config object
-    logging_config = context.appconfig.logging.model_dump(
-        by_alias=True, exclude_none=True
-    )
-    setup_logging(cliverbosity=verbose, user_config={"logging": logging_config})
-
-    # --- PHASE 2: Load Environment Config, Validate, and Acquire Lock ---
-    (
-        context.envconfigfiles,
-        raw_envconfig,
-        context.envconfig_from_defaults,
-    ) = handle_config(
-        env_config,
-        (PROJECT_DIR,),
-        None,
-        DEFAULT_ENV_CONFIG_FILENAME,
-        DEFAULT_ENV_CONFIG,
-    )
+        # Setup logging
+        logging_config = context.appconfig.logging.model_dump(
+            by_alias=True, exclude_none=True
+        )
+        setup_logging(cliverbosity=verbose, user_config={"logging": logging_config})
 
-    raw_envconfig = cast(dict[str, Any], raw_envconfig)
+        # --- PHASE 2: Load Environment Config ---
+        current_model = EnvConfig
+        current_files = (env_config,) if env_config else None
+        load_env_config(ctx, env_config)
 
-    try:
-        context.envconfig = EnvConfig.from_dict(raw_envconfig)
-    except (ValueError, ValidationError) as e:
-        handle_validation_error(e, EnvConfig, context.envconfigfiles, verbose, ctx)
+    except (ValueError, ValidationError, tomllib.TOMLDecodeError) as e:
+        handle_validation_error(e, current_model, current_files, verbose, ctx)
 
+    # --- PHASE 3: Setup Concurrency Lock ---
+    # (Remains outside the try block as it has its own specialized error handling)
     env_config_path = (context.envconfigfiles or [None])[0]
-
-    # --- CONCURRENCY CONTROL ---
     if env_config_path:
         ctx.obj.env_lock = PidFileLock(resource_path=cast(Path, env_config_path))
         try:
             ctx.obj.env_lock.__enter__()
             log.info("Acquired lock for environment config: %r", env_config_path.name)
+            ctx.call_on_close(lambda: ctx.obj.env_lock.__exit__(None, None, None))
         except LockAcquisitionError as e:
             log.error(str(e))
             ctx.exit(1)
         except Exception as e:
             log.error("Failed to set up environment lock: %s", e)
             ctx.exit(1)
 
-        ctx.call_on_close(lambda: ctx.obj.env_lock.__exit__(None, None, None))
-
 
 # Add subcommands
 cli.add_command(build)

src/docbuild/utils/errors.py

@@ -1,6 +1,8 @@
 """Utilities for handling and formatting application errors."""
 
 
+import tomllib
+
 from pydantic import BaseModel, ValidationError
 from rich.console import Console
 from rich.text import Text
@@ -112,3 +114,30 @@ def format_pydantic_error(
             f"[dim]... and {error_count - max_display} more errors. "
             "Use '-vv' to see all errors.[/dim]\n"
         )
+
+def format_toml_error(
+    error: tomllib.TOMLDecodeError,
+    config_file: str,
+    console: Console | None = None,
+) -> None:
+    """Format TOML syntax errors using Rich.
+
+    :param error: The caught TOMLDecodeError object.
+    :param config_file: The name/path of the config file with the syntax error.
+    :param console: Optional Rich console object.
+    """
+    con = console or Console(stderr=True)
+
+    header = Text.assemble(
+        ("Syntax error ", "bold red"),
+        ("in config file ", "white"),
+        (f"'{config_file}'", "bold cyan"),
+        (":", "white")
+    )
+    con.print(header)
+
+    # tomllib error messages include the line and column info naturally
+    con.print(f"    [red]{error}[/red]")
+    con.print()
+    con.print("    [dim]Please verify that the file is a valid TOML file.[/dim]")
+    con.print("    [dim]Note: Booleans must be lowercase (true/false) in TOML.[/dim]")

tests/cli/test_cmd_cli.py

@@ -1,6 +1,7 @@
 """Tests for the main CLI entry point and configuration loading flow."""
 
 from pathlib import Path
+import tomllib
 from unittest.mock import Mock, patch
 
 import click
@@ -267,3 +268,49 @@ def resolver(user_path, *args, **kwargs):
     assert result.exit_code == 0
     assert context.verbose == 3
     assert context.debug is True
+
+
+@pytest.mark.parametrize("is_app_config_failure", [True, False])
+def test_cli_toml_syntax_error(
+    runner,
+    fake_handle_config,
+    mock_config_models,
+    is_app_config_failure,
+):
+    """Verify that the CLI handles TOML syntax errors gracefully."""
+
+    def resolver_with_syntax_error(user_path, *args, **kwargs):
+        # If we are testing Env failure, let the App config pass first
+        if not is_app_config_failure and "default_app" in str(user_path):
+             return (Path("app.toml"),), {"logging": {"version": 1}}, False
+
+        raise tomllib.TOMLDecodeError("Invalid value", "test.toml", 0)
+
+    fake_handle_config(resolver_with_syntax_error)
+
+    result = runner.invoke(cli, ["capture"])
+
+    assert result.exit_code == 1
+    assert "Syntax error in config file" in result.output
+    assert "Invalid value" in result.output
+    assert "Booleans must be lowercase" in result.output
+    assert "Traceback (most recent call last)" not in result.output
+
+
+def test_load_config_helpers_populate_context(fake_handle_config, mock_config_models):
+    """Verify that the refactored helper functions correctly update the context object."""
+    from docbuild.cli.cmd_cli import load_app_config, load_env_config
+
+    # Setup mock resolver
+    fake_handle_config(lambda *a, **k: ((Path("test.toml"),), {"key": "val"}, False))
+
+    mock_ctx = Mock()
+    mock_ctx.obj = DocBuildContext()
+
+    # Test App Loader
+    load_app_config(mock_ctx, Path("app.toml"), max_workers="5")
+    assert mock_ctx.obj.appconfig is not None
+
+    # Test Env Loader
+    load_env_config(mock_ctx, Path("env.toml"))
+    assert mock_ctx.obj.envconfig is not None

tests/utils/test_errors.py

@@ -1,5 +1,6 @@
 """Tests for the Pydantic error formatting utility."""
 
+import tomllib
 from typing import Any
 
 from pydantic import BaseModel, Field, IPvAnyAddress, ValidationError, create_model
@@ -94,3 +95,23 @@ class UnionModel(BaseModel):
     assert "In 'host':" in captured.err
     # Verify no bracketed pydantic internals leaked in the path part
     assert "[" not in captured.err.split("In '")[1].split("':")[0]
+
+
+def test_format_toml_error_smoke(capsys):
+    """Verify that the TOML syntax error formatter prints correctly."""
+    from docbuild.utils.errors import format_toml_error
+
+    # 1. Manually trigger a TOML syntax error
+    bad_toml_content = "enable_mail = True"  # Invalid: Capital T
+    try:
+        tomllib.loads(bad_toml_content)
+    except tomllib.TOMLDecodeError as e:
+        # 2. Call our new formatter
+        format_toml_error(e, "env.devel.toml")
+
+    captured = capsys.readouterr()
+
+    # 3. Assertions to verify the "Rich" output content
+    assert "Syntax error in config file 'env.devel.toml'" in captured.err
+    assert "Invalid value" in captured.err
+    assert "Booleans must be lowercase" in captured.err