diff --git a/CHANGELOG.md b/CHANGELOG.md index 5cf803af..7a6f9535 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- Config validation. When loading config files we now check that specified options will work. If not, the frontend will show an error message with details on what's wrong. This applies to `gui` settings (i.e. our own ones, `beets-flask/config.yaml`) and very select ones from native beets (only those which we use directly). Hopefully, this will eventually cover all config options of beets native, but this is more of an upsream task. [#224](https://github.com/pSpitzner/beets-flask/pull/224). +- Config validation (when loading config files): + - Yaml syntax and parser errors are passed to the frontend for user examination instead of crashing before UI startup [#305](https://github.com/pSpitzner/beets-flask/pull/305) + - We now check that specified options will work. If not, the frontend will show an error message with details on what's wrong. This applies to `gui` settings (i.e. our own ones, `beets-flask/config.yaml`) and very select ones from native beets (only those which we use directly). Hopefully, this will eventually cover all config options of beets native, but this is more of an upsream task. [#224](https://github.com/pSpitzner/beets-flask/pull/224). - Upload Files via the WebUI. You can now drag-and-drop single files into an inbox. To upload whole albums, zip them on your host first (uploading of folders directly is not implemented, as it would require a secure context). - New config option `gui.terminal.enabled` (default: true) [#254](https://github.com/pSpitzner/beets-flask/pull/224) - You can now alternatively use `PUID` and `PGID` instead of `GROUP_ID` and `USER_ID` environment variables. Good for [yaml anchors](https://docs.docker.com/reference/compose-file/fragments/). [#260](https://github.com/pSpitzner/beets-flask/issues/260) diff --git a/backend/beets_flask/config/beets_config.py b/backend/beets_flask/config/beets_config.py index 859ea7dd..b5e87f37 100644 --- a/backend/beets_flask/config/beets_config.py +++ b/backend/beets_flask/config/beets_config.py @@ -13,6 +13,8 @@ from eyconf import ConfigExtra from eyconf.asdict import asdict_with_aliases from eyconf.validation import ConfigurationError, MultiConfigurationError +from yaml import YAMLError +from yaml.scanner import ScannerError from beets_flask.logger import log from beets_flask.utility import deprecation_warning @@ -29,6 +31,7 @@ class BeetsFlaskConfig(ConfigExtra[BeetsSchema]): def __init__(self): """Initialize the config object with the default values.""" super().__init__(schema=BeetsSchema, data=BeetsSchema()) + self._config_errors: list[ConfigurationError] = [] BeetsFlaskConfig.write_examples_as_user_defaults() self.reload() self.commit_to_beets() @@ -48,53 +51,79 @@ def get_beets_config_path(cls) -> Path: return Path(beets_folder) / "config.yaml" def reload(self, extra_yaml_path: str | Path | None = None) -> Self: - """Reset the config to default values. + """Reload the config. This loads the user config from yaml files after resetting to defaults. The `extra_yaml_path` argument is mainly for testing purposes, to add a last yaml layer with high priority. + + Any error messages (YAML parsing, validation) are stored for upstream handling """ log.debug("Resetting/Reloading config") super().reset() + self._config_errors = [] - # There are 3 potential sources - + # Config sources: # 1. beets defaults - # We do not load them into _out_ config. + # We do not load them into _our_ config. # They are still available in the beets_config property. # But: we want to encourage user to add fields that are accessed # from _our_ config into the schema. # Thus only porting requirement: copy the relevant beets default into the schema - + # # 2. beets user config - if self.get_beets_config_path().exists(): - with open(self.get_beets_config_path()) as f: - loaded = yaml.safe_load(f) - if not isinstance(loaded, dict): - raise ValueError("Beets config is not a valid YAML dictionary.") - # EYConfs update method also validates against the schema - self.update(loaded) - # 3. beets-flask user config - if self.get_beets_flask_config_path().exists(): - with open(self.get_beets_flask_config_path()) as f: - loaded = yaml.safe_load(f) + + for label, path_fn in [ + ("beets", self.get_beets_config_path), + ("beets-flask", self.get_beets_flask_config_path), + ]: + path = path_fn() + if not path.exists(): + continue + try: + with open(path) as f: + loaded = yaml.safe_load(f) if not isinstance(loaded, dict): - raise ValueError( - "Beets flask config is not a valid YAML dictionary." - ) + raise yaml.YAMLError("Config is not a YAML dictionary.") self.update(loaded) + except ScannerError as e: + message = str(e) + # handle using beets template function identifier as the first character + if "'%' that cannot start any token" in str(e): + message += "\nTo use the template function you should surround the content with quotes" + log.error( + f"Unquoted template function starting value in {label} config at {path}: {e}" + ) + else: + log.error(f"Failed to parse {label} config at {path}: {e}") + self._config_errors.append( + ConfigurationError(message=message, section=f"{path}") + ) + except YAMLError as e: + print(e.__class__) + log.error(f"Failed to parse {label} config at {path}: {e}") + self._config_errors.append( + ConfigurationError(message=str(e), section=f"{path}") + ) # extra if extra_yaml_path is not None: + # Dev/test — still raise immediately so CI catches it with open(extra_yaml_path) as f: loaded = yaml.safe_load(f) - if not isinstance(loaded, dict): - raise ValueError("Extra config is not a valid YAML dictionary.") - self.update(loaded) + if not isinstance(loaded, dict): + raise ValueError("Extra config is not a valid YAML dictionary.") + self.update(loaded) + + try: + self.validate() + except MultiConfigurationError as e: + self._config_errors.extend(e.errors) + except ConfigurationError as e: + self._config_errors.append(e) - self.validate() return self def commit_to_beets(self) -> None: @@ -278,6 +307,14 @@ def ignore_globs(self) -> list[str]: gui_globs = self.data.ignore return cast(list[str], gui_globs) + @property + def errors(self) -> list[ConfigurationError]: + return getattr(self, "_config_errors", []) + + @property + def is_healthy(self) -> bool: + return len(self.errors) == 0 + config: BeetsFlaskConfig | None = None diff --git a/backend/beets_flask/server/websocket/__init__.py b/backend/beets_flask/server/websocket/__init__.py index d458ba18..93761c87 100644 --- a/backend/beets_flask/server/websocket/__init__.py +++ b/backend/beets_flask/server/websocket/__init__.py @@ -3,7 +3,6 @@ from typing import cast import socketio -from eyconf.validation import ConfigurationError, MultiConfigurationError from beets_flask.config import get_config from beets_flask.logger import log @@ -41,14 +40,30 @@ def register_socketio(app): register_status() - terminal_enabled = True - try: - terminal_enabled = get_config().data.gui.terminal.enabled - except (MultiConfigurationError, ConfigurationError): - # We don't want to let the exception propagate here as it won't reach the frontend. - log.debug("Encountered config error. Will raise on next call to get_config()") + cfg = get_config() + if not cfg.is_healthy: + log.warning( + f"Config loaded with {len(cfg.errors)} error(s), app running on defaults" + ) - if terminal_enabled: + @sio.on("connect") + async def on_connect(sid, environ): + """Push config health to every client on connection.""" + current_cfg = get_config() + await sio.emit( + "config_status", + { + "healthy": current_cfg.is_healthy, + "errors": [ + {"message": e.message, "section": str(e.section)} + for e in current_cfg.errors + ], + }, + to=sid, + ) + + # Terminal setup uses the already-loaded config + if cfg.data.gui.terminal.enabled: log.info("Setting up Web-Terminal") from .terminal import register_tmux diff --git a/backend/tests/unit/test_config.py b/backend/tests/unit/test_config.py index 04c5d8be..27af68c0 100644 --- a/backend/tests/unit/test_config.py +++ b/backend/tests/unit/test_config.py @@ -106,6 +106,42 @@ def test_reload(self): with open(beets_flask_path, "w") as f: f.write(content) + def test_config_can_handle_yaml_errors(self): + """Test that loading invalid config still works.""" + + config = get_config(force_reload=True) + beets_path = config.get_beets_config_path() + config.write_examples_as_user_defaults() + assert beets_path.exists() + + with open(beets_path) as f: + content = f.read() + # use an unquoted template function in the paths section + invalid_yaml = """ +paths: + albumtype:compilation: %if{$test,Albums/$albumartist/,Compilations/}$album%aunique{}/$title + """ + invalid_content = content + invalid_yaml + with open(beets_path, "w") as f: + f.write(invalid_content) + + config.reload() + assert config.is_healthy == False + assert len(config.errors) == 1 + + # fix invalid value + valid_yaml = """ +paths: + albumtype:compilation: "%if{$test,Albums/$albumartist/,Compilations/}$album%aunique{}/$title" +""" + valid_content = content + valid_yaml + with open(beets_path, "w") as f: + f.write(valid_content) + + config.reload() + assert config.is_healthy == True + assert len(config.errors) == 0 + def test_commit_to_beets(self): """Test that committing to beets config works as expected.""" import beets