From b0d65581df59a749ab347fe5d97b60e8ed0dd6ee Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Sat, 27 Dec 2025 18:00:32 -0500 Subject: [PATCH] Fix/config save error handling (#150) * fix(docs): Add trailing newlines to documentation files * fix(web): Resolve font configuration loading error on first page load - Remove ineffective DOMContentLoaded listener from fonts partial (loads via HTMX after main page DOMContentLoaded) - Add proper HTMX event handling with htmx:afterSettle for reliable initialization - Add duplicate initialization protection flag - Improve error handling with response validation and clearer error messages - Add fallback initialization check for edge cases - Ensure DOM elements exist before attempting initialization Fixes issue where 'Error loading font configuration' appeared on first web UI load when opening fonts tab. * fix(config): Update plugins_directory to plugin-repos in config template The web-ui-info plugin is located in plugin-repos/ directory, but the config template was pointing to plugins/ directory. This caused the plugin to not be discovered on fresh installations. - Changed plugins_directory from 'plugins' to 'plugin-repos' in config.template.json - Matches actual plugin location and code default behavior - Ensures web-ui-info plugin is available by default on fresh installs * fix(config): Improve config save error handling - Make load_config() failure non-fatal in save_raw_file_content - Wrapped reload in try-except to prevent save failures when reload fails - File save is atomic and successful even if reload fails - Logs warning when reload fails but doesn't fail the operation - Improve error messages in API endpoints - Added detailed error logging with full traceback for debugging - Extract specific error messages from ConfigError exceptions - Include config_path in error messages when available - Provide fallback messages for empty error strings - Enhance frontend error handling - Check response status before parsing JSON - Better handling of non-JSON error responses - Fallback error messages if error details are missing Fixes issue where 'Error saving config.json: an error occured' was shown even when the file was saved successfully but reload failed. --------- Co-authored-by: Chuck --- src/config_manager.py | 12 ++- web_interface/blueprints/api_v3.py | 93 ++++++++++++++++++- .../templates/v3/partials/raw_json.html | 34 +++++-- 3 files changed, 129 insertions(+), 10 deletions(-) diff --git a/src/config_manager.py b/src/config_manager.py index e20d2fa2..fd93f00c 100644 --- a/src/config_manager.py +++ b/src/config_manager.py @@ -448,8 +448,18 @@ class ConfigManager: # If we just saved the main config or secrets, the merged self.config might be stale. # Reload it to reflect the new state. + # Note: We wrap this in try-except because reload failures (e.g., migration errors) + # should not cause the save operation to fail - the file was saved successfully. if file_type == "main" or file_type == "secrets": - self.load_config() + try: + self.load_config() + except Exception as reload_error: + # Log the reload error but don't fail the save operation + # The file was saved successfully, reload is just for in-memory consistency + self.logger.warning( + f"Configuration file saved successfully, but reload failed: {reload_error}. " + f"The file on disk is valid, but in-memory config may be stale." + ) except (IOError, OSError, PermissionError) as e: error_msg = f"Error writing {file_type} configuration to file {os.path.abspath(path_to_save)}" diff --git a/web_interface/blueprints/api_v3.py b/web_interface/blueprints/api_v3.py index 6ef0ccb0..473689da 100644 --- a/web_interface/blueprints/api_v3.py +++ b/web_interface/blueprints/api_v3.py @@ -665,7 +665,24 @@ def save_raw_main_config(): except json.JSONDecodeError as e: return jsonify({'status': 'error', 'message': f'Invalid JSON: {str(e)}'}), 400 except Exception as e: - return jsonify({'status': 'error', 'message': str(e)}), 500 + import logging + import traceback + from src.exceptions import ConfigError + + # Log the full error for debugging + error_msg = f"Error saving raw main config: {str(e)}\n{traceback.format_exc()}" + logging.error(error_msg) + + # Extract more specific error message if it's a ConfigError + if isinstance(e, ConfigError): + # ConfigError has a message attribute and may have context + error_message = str(e) + if hasattr(e, 'config_path') and e.config_path: + error_message = f"{error_message} (config_path: {e.config_path})" + else: + error_message = str(e) if str(e) else "An unexpected error occurred while saving the configuration" + + return jsonify({'status': 'error', 'message': error_message}), 500 @api_v3.route('/config/raw/secrets', methods=['POST']) def save_raw_secrets_config(): @@ -689,7 +706,24 @@ def save_raw_secrets_config(): except json.JSONDecodeError as e: return jsonify({'status': 'error', 'message': f'Invalid JSON: {str(e)}'}), 400 except Exception as e: - return jsonify({'status': 'error', 'message': str(e)}), 500 + import logging + import traceback + from src.exceptions import ConfigError + + # Log the full error for debugging + error_msg = f"Error saving raw secrets config: {str(e)}\n{traceback.format_exc()}" + logging.error(error_msg) + + # Extract more specific error message if it's a ConfigError + if isinstance(e, ConfigError): + # ConfigError has a message attribute and may have context + error_message = str(e) + if hasattr(e, 'config_path') and e.config_path: + error_message = f"{error_message} (config_path: {e.config_path})" + else: + error_message = str(e) if str(e) else "An unexpected error occurred while saving the configuration" + + return jsonify({'status': 'error', 'message': error_message}), 500 @api_v3.route('/system/status', methods=['GET']) def get_system_status(): @@ -2999,6 +3033,57 @@ def _set_nested_value(config, key_path, value): current[parts[-1]] = value +def _enhance_schema_with_core_properties(schema): + """ + Enhance schema with core plugin properties (enabled, display_duration, live_priority). + These properties are system-managed and should always be allowed even if not in the plugin's schema. + + Args: + schema: The original JSON schema dict + + Returns: + Enhanced schema dict with core properties injected + """ + import copy + + if not schema: + return schema + + # Core plugin properties that should always be allowed + # These match the definitions in SchemaManager.validate_config_against_schema() + core_properties = { + "enabled": { + "type": "boolean", + "default": True, + "description": "Enable or disable this plugin" + }, + "display_duration": { + "type": "number", + "default": 15, + "minimum": 1, + "maximum": 300, + "description": "How long to display this plugin in seconds" + }, + "live_priority": { + "type": "boolean", + "default": False, + "description": "Enable live priority takeover when plugin has live content" + } + } + + # Create a deep copy of the schema to modify (to avoid mutating the original) + enhanced_schema = copy.deepcopy(schema) + if "properties" not in enhanced_schema: + enhanced_schema["properties"] = {} + + # Inject core properties if they're not already defined in the schema + for prop_name, prop_def in core_properties.items(): + if prop_name not in enhanced_schema["properties"]: + enhanced_schema["properties"][prop_name] = copy.deepcopy(prop_def) + + return enhanced_schema + + def _filter_config_by_schema(config, schema, prefix=''): """ Filter config to only include fields defined in the schema. @@ -3664,8 +3749,10 @@ def save_plugin_config(): plugin_config = normalize_config_values(plugin_config, schema['properties']) # Filter config to only include schema-defined fields (important when additionalProperties is false) + # Use enhanced schema with core properties to ensure core properties are preserved during filtering if schema and 'properties' in schema: - plugin_config = _filter_config_by_schema(plugin_config, schema) + enhanced_schema_for_filtering = _enhance_schema_with_core_properties(schema) + plugin_config = _filter_config_by_schema(plugin_config, enhanced_schema_for_filtering) # Debug logging for union type fields (temporary) if 'rotation_settings' in plugin_config and 'random_seed' in plugin_config.get('rotation_settings', {}): diff --git a/web_interface/templates/v3/partials/raw_json.html b/web_interface/templates/v3/partials/raw_json.html index d92406bd..fa2cb9c8 100644 --- a/web_interface/templates/v3/partials/raw_json.html +++ b/web_interface/templates/v3/partials/raw_json.html @@ -218,16 +218,27 @@ function saveMainConfig() { }, body: JSON.stringify(config) }) - .then(response => response.json()) + .then(response => { + // Check if response is OK before parsing JSON + if (!response.ok) { + // Try to parse error response as JSON, fallback to status text + return response.json().then(data => { + throw new Error(data.message || response.statusText); + }).catch(() => { + throw new Error(`HTTP ${response.status}: ${response.statusText}`); + }); + } + return response.json(); + }) .then(data => { if (data.status === 'success') { showNotification('config.json saved successfully!', 'success'); } else { - showNotification('Error saving config.json: ' + data.message, 'error'); + showNotification('Error saving config.json: ' + (data.message || 'Unknown error'), 'error'); } }) .catch(error => { - showNotification('Error saving config.json: ' + error.message, 'error'); + showNotification('Error saving config.json: ' + (error.message || 'An error occurred'), 'error'); }); } catch (e) { showNotification('Invalid JSON: ' + e.message, 'error'); @@ -254,16 +265,27 @@ function saveSecretsConfig() { }, body: JSON.stringify(config) }) - .then(response => response.json()) + .then(response => { + // Check if response is OK before parsing JSON + if (!response.ok) { + // Try to parse error response as JSON, fallback to status text + return response.json().then(data => { + throw new Error(data.message || response.statusText); + }).catch(() => { + throw new Error(`HTTP ${response.status}: ${response.statusText}`); + }); + } + return response.json(); + }) .then(data => { if (data.status === 'success') { showNotification('config_secrets.json saved successfully!', 'success'); } else { - showNotification('Error saving config_secrets.json: ' + data.message, 'error'); + showNotification('Error saving config_secrets.json: ' + (data.message || 'Unknown error'), 'error'); } }) .catch(error => { - showNotification('Error saving config_secrets.json: ' + error.message, 'error'); + showNotification('Error saving config_secrets.json: ' + (error.message || 'An error occurred'), 'error'); }); } catch (e) { showNotification('Invalid JSON: ' + e.message, 'error');