From 0c4af277d133f011cc5dc3e54d196c360558e6d4 Mon Sep 17 00:00:00 2001 From: sushant-suse Date: Tue, 21 Apr 2026 13:17:27 +0530 Subject: [PATCH 1/4] fix(cli) #175: handle TOML syntax errors gracefully Signed-off-by: sushant-suse --- src/docbuild/cli/cmd_cli.py | 76 +++++++++++++++++++++--------------- src/docbuild/utils/errors.py | 29 ++++++++++++++ tests/cli/test_cmd_cli.py | 30 ++++++++++++++ tests/utils/test_errors.py | 21 ++++++++++ 4 files changed, 124 insertions(+), 32 deletions(-) diff --git a/src/docbuild/cli/cmd_cli.py b/src/docbuild/cli/cmd_cli.py index ae127ec5..64ed91a2 100644 --- a/src/docbuild/cli/cmd_cli.py +++ b/src/docbuild/cli/cmd_cli.py @@ -10,6 +10,7 @@ from pydantic import BaseModel, ValidationError import rich.console from rich.traceback import install as install_traceback +import tomllib from ..__about__ import __version__ from ..config.load import handle_config @@ -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,16 +69,16 @@ 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) @@ -171,18 +172,26 @@ def cli( context.dry_run = dry_run context.debug = debug + # --- INITIALIZE TO AVOID UNBOUND ERRORS --- + raw_appconfig: dict[str, Any] = {} + raw_envconfig: dict[str, Any] = {} + # --- 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, - ) + try: + # We cast the return of handle_config to the expected tuple type + 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 + ) + except tomllib.TOMLDecodeError as e: + files = (app_config,) if app_config else None + handle_validation_error(e, AppConfig, files, verbose, ctx) raw_appconfig = cast(dict[str, Any], raw_appconfig) @@ -200,18 +209,21 @@ def cli( ) 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, - ) + # --- PHASE 2: Load Environment Config --- + try: + 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 + ) + except tomllib.TOMLDecodeError as e: + files = (env_config,) if env_config else None + handle_validation_error(e, EnvConfig, files, verbose, ctx) raw_envconfig = cast(dict[str, Any], raw_envconfig) diff --git a/src/docbuild/utils/errors.py b/src/docbuild/utils/errors.py index bd0ea21b..b714b553 100644 --- a/src/docbuild/utils/errors.py +++ b/src/docbuild/utils/errors.py @@ -4,6 +4,7 @@ from pydantic import BaseModel, ValidationError from rich.console import Console from rich.text import Text +import tomllib from ..constants import DEFAULT_ERROR_LIMIT @@ -112,3 +113,31 @@ 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]") + \ No newline at end of file diff --git a/tests/cli/test_cmd_cli.py b/tests/cli/test_cmd_cli.py index ecea9867..746630c1 100644 --- a/tests/cli/test_cmd_cli.py +++ b/tests/cli/test_cmd_cli.py @@ -6,6 +6,7 @@ import click from pydantic import ValidationError import pytest +import tomllib import docbuild.cli.cmd_cli as cli_mod from docbuild.cli.context import DocBuildContext @@ -267,3 +268,32 @@ 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 (Issue #175).""" + + # Define a resolver that raises TOMLDecodeError + def resolver_with_syntax_error(user_path, *args, **kwargs): + # We simulate the exact error raised by tomllib + raise tomllib.TOMLDecodeError("Invalid value", "test.toml", 0) + + fake_handle_config(resolver_with_syntax_error) + + # Invoke the CLI. Whether it's app or env config, the handle_config + # call is now wrapped in a try/except in cmd_cli.py + 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 + # Verify our specific 'Note' from the formatter appears + assert "Booleans must be lowercase" in result.output + # Crucially, ensure no raw Python traceback is leaked to the user + assert "Traceback (most recent call last)" not in result.output diff --git a/tests/utils/test_errors.py b/tests/utils/test_errors.py index f7fa84ca..36f14820 100644 --- a/tests/utils/test_errors.py +++ b/tests/utils/test_errors.py @@ -1,6 +1,7 @@ """Tests for the Pydantic error formatting utility.""" from typing import Any +import tomllib from pydantic import BaseModel, Field, IPvAnyAddress, ValidationError, create_model from rich.console import Console @@ -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 From 33ce337e9cf19edf710efbf8bd83d427a83c3a6e Mon Sep 17 00:00:00 2001 From: sushant-suse Date: Tue, 21 Apr 2026 13:32:48 +0530 Subject: [PATCH 2/4] fix #175: ruff issues fixation Signed-off-by: sushant-suse --- src/docbuild/cli/cmd_cli.py | 151 ++++++++++++++++++++--------------- src/docbuild/utils/errors.py | 6 +- tests/cli/test_cmd_cli.py | 6 +- tests/utils/test_errors.py | 4 +- 4 files changed, 95 insertions(+), 72 deletions(-) diff --git a/src/docbuild/cli/cmd_cli.py b/src/docbuild/cli/cmd_cli.py index 64ed91a2..cea50e3b 100644 --- a/src/docbuild/cli/cmd_cli.py +++ b/src/docbuild/cli/cmd_cli.py @@ -4,13 +4,13 @@ import logging from pathlib import Path import sys +import tomllib from typing import Any, cast import click from pydantic import BaseModel, ValidationError import rich.console from rich.traceback import install as install_traceback -import tomllib from ..__about__ import __version__ from ..config.load import handle_config @@ -84,6 +84,83 @@ def handle_validation_error( ctx.exit(1) +def _load_app_config( + ctx: click.Context, + app_config: Path, + verbose: int, + max_workers: str | None +) -> None: + """Load and validate Application configuration. + + Args: + ctx: The Click context object. + app_config: The path to the application config file provided via CLI. + verbose: The verbosity level from CLI options. + max_workers: The max_workers value from CLI options. + + """ + context = ctx.obj + # Initialize to satisfy Pylance control-flow analysis + raw_appconfig: dict[str, Any] = {} + + try: + 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 + ) + except tomllib.TOMLDecodeError as e: + files = (app_config,) if app_config else None + handle_validation_error(e, AppConfig, files, verbose, ctx) + + if max_workers is not None: + raw_appconfig["max_workers"] = max_workers + + try: + context.appconfig = AppConfig.from_dict(raw_appconfig) + except (ValueError, ValidationError) as e: + handle_validation_error(e, AppConfig, context.appconfigfiles, verbose, ctx) + + +def _load_env_config(ctx: click.Context, env_config: Path, verbose: int) -> None: + """Load and validate Environment configuration. + + Args: + ctx: The Click context object. + env_config: The path to the environment config file provided via CLI. + verbose: The verbosity level from CLI options. + + """ + context = ctx.obj + # Initialize to satisfy Pylance control-flow analysis + raw_envconfig: dict[str, Any] = {} + + try: + 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 + ) + except tomllib.TOMLDecodeError as e: + files = (env_config,) if env_config else None + handle_validation_error(e, EnvConfig, files, verbose, ctx) + + try: + context.envconfig = EnvConfig.from_dict(raw_envconfig) + except (ValueError, ValidationError) as e: + handle_validation_error(e, EnvConfig, context.envconfigfiles, verbose, ctx) + + @click.group( name=APP_NAME, context_settings={"show_default": True, "help_option_names": ["-h", "--help"]}, @@ -168,78 +245,26 @@ def cli( ctx.ensure_object(DocBuildContext) context = ctx.obj - context.verbose = verbose - context.dry_run = dry_run - context.debug = debug - - # --- INITIALIZE TO AVOID UNBOUND ERRORS --- - raw_appconfig: dict[str, Any] = {} - raw_envconfig: dict[str, Any] = {} - - # --- PHASE 1: Load and Validate Application Config --- - try: - # We cast the return of handle_config to the expected tuple type - 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 - ) - except tomllib.TOMLDecodeError as e: - files = (app_config,) if app_config else None - handle_validation_error(e, AppConfig, files, verbose, ctx) + context.verbose, context.dry_run, context.debug = verbose, dry_run, debug - raw_appconfig = cast(dict[str, Any], raw_appconfig) - - if max_workers is not None: - raw_appconfig["max_workers"] = max_workers + # 1. Load Application Config + _load_app_config(ctx, app_config, verbose, max_workers) - try: - context.appconfig = AppConfig.from_dict(raw_appconfig) - except (ValueError, ValidationError) as e: - handle_validation_error(e, AppConfig, context.appconfigfiles, verbose, ctx) - - # 3. Setup logging using the validated config object - logging_config = context.appconfig.logging.model_dump( - by_alias=True, exclude_none=True - ) + # 2. Setup logging + 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 --- - try: - 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 - ) - except tomllib.TOMLDecodeError as e: - files = (env_config,) if env_config else None - handle_validation_error(e, EnvConfig, files, verbose, ctx) - - raw_envconfig = cast(dict[str, Any], raw_envconfig) - - try: - context.envconfig = EnvConfig.from_dict(raw_envconfig) - except (ValueError, ValidationError) as e: - handle_validation_error(e, EnvConfig, context.envconfigfiles, verbose, ctx) + # 3. Load Environment Config + _load_env_config(ctx, env_config, verbose) + # 4. Setup Concurrency Lock 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) @@ -247,8 +272,6 @@ def cli( 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) diff --git a/src/docbuild/utils/errors.py b/src/docbuild/utils/errors.py index b714b553..d7fb4962 100644 --- a/src/docbuild/utils/errors.py +++ b/src/docbuild/utils/errors.py @@ -1,10 +1,11 @@ """Utilities for handling and formatting application errors.""" +import tomllib + from pydantic import BaseModel, ValidationError from rich.console import Console from rich.text import Text -import tomllib from ..constants import DEFAULT_ERROR_LIMIT @@ -134,10 +135,9 @@ def format_toml_error( (":", "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]") - \ No newline at end of file diff --git a/tests/cli/test_cmd_cli.py b/tests/cli/test_cmd_cli.py index 746630c1..17e5f615 100644 --- a/tests/cli/test_cmd_cli.py +++ b/tests/cli/test_cmd_cli.py @@ -1,12 +1,12 @@ """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 from pydantic import ValidationError import pytest -import tomllib import docbuild.cli.cmd_cli as cli_mod from docbuild.cli.context import DocBuildContext @@ -278,7 +278,7 @@ def test_cli_toml_syntax_error( is_app_config_failure, ): """Verify that the CLI handles TOML syntax errors gracefully (Issue #175).""" - + # Define a resolver that raises TOMLDecodeError def resolver_with_syntax_error(user_path, *args, **kwargs): # We simulate the exact error raised by tomllib @@ -286,7 +286,7 @@ def resolver_with_syntax_error(user_path, *args, **kwargs): fake_handle_config(resolver_with_syntax_error) - # Invoke the CLI. Whether it's app or env config, the handle_config + # Invoke the CLI. Whether it's app or env config, the handle_config # call is now wrapped in a try/except in cmd_cli.py result = runner.invoke(cli, ["capture"]) diff --git a/tests/utils/test_errors.py b/tests/utils/test_errors.py index 36f14820..22118eb9 100644 --- a/tests/utils/test_errors.py +++ b/tests/utils/test_errors.py @@ -1,7 +1,7 @@ """Tests for the Pydantic error formatting utility.""" -from typing import Any import tomllib +from typing import Any from pydantic import BaseModel, Field, IPvAnyAddress, ValidationError, create_model from rich.console import Console @@ -100,7 +100,7 @@ class UnionModel(BaseModel): 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: From 75a4c849d09b8095b78c607ebb5bf8a35315d9c3 Mon Sep 17 00:00:00 2001 From: sushant-suse Date: Tue, 21 Apr 2026 21:17:26 +0530 Subject: [PATCH 3/4] fix #175: update private functions Signed-off-by: sushant-suse --- src/docbuild/cli/cmd_cli.py | 105 +++++++++++++++++------------------- tests/cli/test_cmd_cli.py | 31 ++++++++--- 2 files changed, 72 insertions(+), 64 deletions(-) diff --git a/src/docbuild/cli/cmd_cli.py b/src/docbuild/cli/cmd_cli.py index cea50e3b..07ccc33d 100644 --- a/src/docbuild/cli/cmd_cli.py +++ b/src/docbuild/cli/cmd_cli.py @@ -84,10 +84,9 @@ def handle_validation_error( ctx.exit(1) -def _load_app_config( +def load_app_config( ctx: click.Context, app_config: Path, - verbose: int, max_workers: str | None ) -> None: """Load and validate Application configuration. @@ -95,71 +94,48 @@ def _load_app_config( Args: ctx: The Click context object. app_config: The path to the application config file provided via CLI. - verbose: The verbosity level from CLI options. max_workers: The max_workers value from CLI options. """ context = ctx.obj - # Initialize to satisfy Pylance control-flow analysis - raw_appconfig: dict[str, Any] = {} - - try: - 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 - ) - except tomllib.TOMLDecodeError as e: - files = (app_config,) if app_config else None - handle_validation_error(e, AppConfig, files, verbose, ctx) + 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 - try: - context.appconfig = AppConfig.from_dict(raw_appconfig) - except (ValueError, ValidationError) as e: - handle_validation_error(e, AppConfig, context.appconfigfiles, verbose, ctx) + context.appconfig = AppConfig.from_dict(raw_appconfig) -def _load_env_config(ctx: click.Context, env_config: Path, verbose: int) -> None: +def load_env_config(ctx: click.Context, env_config: Path) -> None: """Load and validate Environment configuration. Args: ctx: The Click context object. env_config: The path to the environment config file provided via CLI. - verbose: The verbosity level from CLI options. """ context = ctx.obj - # Initialize to satisfy Pylance control-flow analysis - raw_envconfig: dict[str, Any] = {} - - try: - 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 - ) - except tomllib.TOMLDecodeError as e: - files = (env_config,) if env_config else None - handle_validation_error(e, EnvConfig, files, verbose, ctx) - - try: - context.envconfig = EnvConfig.from_dict(raw_envconfig) - except (ValueError, ValidationError) as e: - handle_validation_error(e, EnvConfig, context.envconfigfiles, verbose, ctx) - + 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, @@ -247,17 +223,32 @@ def cli( context = ctx.obj context.verbose, context.dry_run, context.debug = verbose, dry_run, debug - # 1. Load Application Config - _load_app_config(ctx, app_config, verbose, max_workers) + # State tracking for centralized error handling + current_model: type[BaseModel] = AppConfig + current_files: Sequence[Path] | None = None + + try: + # --- 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) + + # Setup logging + logging_config = context.appconfig.logging.model_dump( + by_alias=True, exclude_none=True + ) + setup_logging(cliverbosity=verbose, user_config={"logging": logging_config}) - # 2. Setup logging - 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 --- + current_model = EnvConfig + current_files = (env_config,) if env_config else None + load_env_config(ctx, env_config) - # 3. Load Environment Config - _load_env_config(ctx, env_config, verbose) + except (ValueError, ValidationError, tomllib.TOMLDecodeError) as e: + handle_validation_error(e, current_model, current_files, verbose, ctx) - # 4. Setup Concurrency Lock + # --- 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] if env_config_path: ctx.obj.env_lock = PidFileLock(resource_path=cast(Path, env_config_path)) diff --git a/tests/cli/test_cmd_cli.py b/tests/cli/test_cmd_cli.py index 17e5f615..b464de78 100644 --- a/tests/cli/test_cmd_cli.py +++ b/tests/cli/test_cmd_cli.py @@ -277,23 +277,40 @@ def test_cli_toml_syntax_error( mock_config_models, is_app_config_failure, ): - """Verify that the CLI handles TOML syntax errors gracefully (Issue #175).""" + """Verify that the CLI handles TOML syntax errors gracefully.""" - # Define a resolver that raises TOMLDecodeError def resolver_with_syntax_error(user_path, *args, **kwargs): - # We simulate the exact error raised by tomllib + # 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) - # Invoke the CLI. Whether it's app or env config, the handle_config - # call is now wrapped in a try/except in cmd_cli.py 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 - # Verify our specific 'Note' from the formatter appears assert "Booleans must be lowercase" in result.output - # Crucially, ensure no raw Python traceback is leaked to the user 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 From 54bc798927758f7e8bd9041ae8dbe18a6d853a81 Mon Sep 17 00:00:00 2001 From: sushant-suse Date: Wed, 22 Apr 2026 14:29:45 +0530 Subject: [PATCH 4/4] docs #175: added fragment file and fixed docstring Signed-off-by: sushant-suse --- changelog.d/175.bugfix.rst | 1 + src/docbuild/cli/cmd_cli.py | 14 +++++--------- 2 files changed, 6 insertions(+), 9 deletions(-) create mode 100644 changelog.d/175.bugfix.rst diff --git a/changelog.d/175.bugfix.rst b/changelog.d/175.bugfix.rst new file mode 100644 index 00000000..34bd5b19 --- /dev/null +++ b/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. \ No newline at end of file diff --git a/src/docbuild/cli/cmd_cli.py b/src/docbuild/cli/cmd_cli.py index 07ccc33d..e8768859 100644 --- a/src/docbuild/cli/cmd_cli.py +++ b/src/docbuild/cli/cmd_cli.py @@ -91,11 +91,9 @@ def load_app_config( ) -> None: """Load and validate Application configuration. - Args: - ctx: The Click context object. - app_config: The path to the application config file provided via CLI. - max_workers: The max_workers value from CLI options. - + :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( @@ -118,10 +116,8 @@ def load_app_config( def load_env_config(ctx: click.Context, env_config: Path) -> None: """Load and validate Environment configuration. - Args: - ctx: The Click context object. - env_config: The path to the environment config file provided via CLI. - + :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(