fix(web): repair news ticker custom-feeds save for JSON path (#376)

* feat(install): surface root cause of web dependency install failures install_dependencies_apt.py previously reported only which packages failed, not why - the actual apt/pip error was discarded (apt) or could scroll out of the on_error log tail (pip), leaving "Step 7: Install web interface dependencies (line 915)" as the only visible detail. Capture command output for each install attempt and print a compact DEPENDENCY INSTALLATION FAILURES summary with the last lines of error output per package. Also run the installer with `python3 -u` for real-time, correctly-ordered logging, and widen the on_error tail from 50 to 100 lines so the summary isn't cut off. * fix(web): repair news ticker custom-feeds save for JSON path The JS dotToNested() helper converts indexed form fields like feeds.custom_feeds.0.name into a dict {'0': {name:...}} rather than a proper array. The form-data path already had fix_array_structures() to convert those dicts back to arrays before schema validation, but the JSON path (used by all web-UI saves) never ran that fix, so saving any custom feed produced a schema validation error: "Expected type array, got object". Add _fix_json_arrays() immediately after schema loading on the JSON path, mirroring the existing fix_array_structures() logic. Also fix custom-feeds.js getValue() to omit the logo key entirely when no logo is present instead of returning logo:null, which would fail schema validation (logo expects type object). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(install,tools): address PR 376 review findings - first_time_install.sh: add _clone_rpi_rgb() wrapper so retry() cleans up any partial rpi-rgb-led-matrix-master dir before each clone attempt - first_time_install.sh: use apt-get -o DPkg::Lock::Timeout=180 so apt handles lock contention natively instead of relying solely on flock TOCTOU check - install_dependencies_apt.py: pass DPkg::Lock::Timeout=180 to apt-get install to avoid failing when unattended-upgrades holds the lock - install_dependencies_apt.py: add type annotations to all public helpers - api_v3.py: fix install_plugin_requirements to read plugin_manager from api_v3 blueprint attribute instead of the always-None module variable - tools.html: loadGitInfo() now checks r.ok before parsing JSON and surfaces d.status === 'error' with the server's message in the panel Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(tools,api): address three additional review findings - api_v3.py install_plugin_requirements: replace hardcoded plugin-repos fallback with config-driven resolution (plugin_system.plugins_directory), matching the pattern used elsewhere in the module - api_v3.py _fix_json_arrays: recurse into converted and existing array elements when items.type is object, so nested numeric-keyed dicts inside array items are also normalized - tools.html toolsAction: check r.ok before r.json() and recover gracefully from non-JSON error bodies (HTML 500 pages), consistent with the existing loadGitInfo guard Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-08-01 08:48:05 +00:00 · 2026-06-29 14:21:05 -04:00
parent 6096a22c3d
commit 639e1c3a93
5 changed files with 88 additions and 21 deletions
@@ -227,8 +227,8 @@ wait_for_apt_lock() {
    done
 }

-apt_update() { wait_for_apt_lock; retry apt update; }
-apt_install() { wait_for_apt_lock; retry apt install -y "$@"; }
+apt_update() { wait_for_apt_lock; retry apt-get -o DPkg::Lock::Timeout=180 update; }
+apt_install() { wait_for_apt_lock; retry apt-get -o DPkg::Lock::Timeout=180 install -y "$@"; }
 apt_remove() { apt-get remove -y "$@" || true; }

 check_network() {
@@ -857,24 +857,30 @@ if [ "$_SKIP_BUILD" = "1" ]; then
    echo "rgbmatrix already installed${_skip_suffix}; skipping build (set RPI_RGB_FORCE_REBUILD=1 to force rebuild)."
 else
    # Ensure rpi-rgb-led-matrix submodule is initialized
+    # Wrapper used with retry(): removes any partial clone dir before each attempt
+    # so git clone doesn't fail with "destination path already exists".
+    _clone_rpi_rgb() {
+        rm -rf "$PROJECT_ROOT_DIR/rpi-rgb-led-matrix-master"
+        git clone https://github.com/hzeller/rpi-rgb-led-matrix.git rpi-rgb-led-matrix-master
+    }
    if [ ! -d "$PROJECT_ROOT_DIR/rpi-rgb-led-matrix-master" ]; then
        echo "rpi-rgb-led-matrix-master not found. Initializing git submodule..."
        cd "$PROJECT_ROOT_DIR"
-        
+
        # Try to initialize submodule if .gitmodules exists
        if [ -f "$PROJECT_ROOT_DIR/.gitmodules" ] && grep -q "rpi-rgb-led-matrix" "$PROJECT_ROOT_DIR/.gitmodules"; then
            echo "Initializing rpi-rgb-led-matrix submodule..."
            if ! retry git submodule update --init --recursive rpi-rgb-led-matrix-master; then
                echo "⚠ Submodule init failed, cloning directly from GitHub..."
-                retry git clone https://github.com/hzeller/rpi-rgb-led-matrix.git rpi-rgb-led-matrix-master
+                retry _clone_rpi_rgb
            fi
        else
            # Fallback: clone directly if submodule not configured
            echo "Submodule not configured, cloning directly from GitHub..."
-            retry git clone https://github.com/hzeller/rpi-rgb-led-matrix.git rpi-rgb-led-matrix-master
+            retry _clone_rpi_rgb
        fi
    fi
-    
+
    # Build and install rpi-rgb-led-matrix Python bindings
    if [ -d "$PROJECT_ROOT_DIR/rpi-rgb-led-matrix-master" ]; then
        # Check if submodule is properly initialized (not empty)
@@ -885,7 +891,7 @@ else
            if [ -f "$PROJECT_ROOT_DIR/.gitmodules" ] && grep -q "rpi-rgb-led-matrix" "$PROJECT_ROOT_DIR/.gitmodules"; then
                retry git submodule update --init --recursive rpi-rgb-led-matrix-master
            else
-                retry git clone https://github.com/hzeller/rpi-rgb-led-matrix.git rpi-rgb-led-matrix-master
+                retry _clone_rpi_rgb
            fi
        fi

@@ -10,6 +10,7 @@ import tempfile
 import warnings
 from collections import deque
 from pathlib import Path
+from typing import List, Tuple

 # How many trailing lines of a failed command's output to keep for the
 # end-of-run failure summary. Keeps the root cause near the end of the log,
@@ -17,7 +18,7 @@ from pathlib import Path
 ERROR_TAIL_LINES = 15


-def _run(cmd):
+def _run(cmd: List[str]) -> Tuple[bool, str]:
    """Run a command, streaming combined stdout/stderr to a temp file.

    Returns (success, output) instead of raising, so callers can report
@@ -37,7 +38,7 @@ def _run(cmd):
    return result.returncode == 0, '\n'.join(tail)


-def install_via_apt(package_name):
+def install_via_apt(package_name: str) -> Tuple[bool, str]:
    """Try to install a package via apt. Returns (success, output)."""
    # Map pip package names to apt package names
    apt_package_map = {
@@ -59,7 +60,7 @@ def install_via_apt(package_name):
    apt_package = apt_package_map.get(package_name, f'python3-{package_name}')

    print(f"Trying to install {apt_package} via apt...")
-    success, output = _run(['sudo', 'apt', 'install', '-y', apt_package])
+    success, output = _run(['sudo', 'apt-get', '-o', 'DPkg::Lock::Timeout=180', 'install', '-y', apt_package])
    if success:
        print(f"Successfully installed {apt_package} via apt")
        return True, ""
@@ -68,7 +69,7 @@ def install_via_apt(package_name):
    return False, output


-def install_via_pip(package_name):
+def install_via_pip(package_name: str) -> Tuple[bool, str]:
    """Install a package via pip with --break-system-packages and --prefer-binary.

    --break-system-packages allows pip to install into the system Python on
@@ -105,7 +106,7 @@ IMPORT_NAME_MAP = {
 }


-def check_package_installed(package_name):
+def check_package_installed(package_name: str) -> bool:
    """Check if a package is already installed."""
    import_name = IMPORT_NAME_MAP.get(package_name, package_name)
    # Suppress deprecation warnings when checking if packages are installed
@@ -119,7 +120,7 @@ def check_package_installed(package_name):
            return False


-def print_failure_summary(failed_packages, failure_details):
+def print_failure_summary(failed_packages: List[str], failure_details: dict) -> None:
    print("\n" + "=" * 60)
    print("DEPENDENCY INSTALLATION FAILURES - DETAILS")
    print("=" * 60)
@@ -208,7 +209,7 @@ def main():
            if setup_py.exists():
                # Try installing - use regular install, not editable mode
                # This is optional for web interface and should already be installed in Step 6
-                ok, output = _run([sys.executable, '-m', 'pip', 'install', '--break-system-packages', '--ignore-installed', str(rgbmatrix_path)])
+                ok, output = _run([sys.executable, '-m', 'pip', 'install', '--break-system-packages', str(rgbmatrix_path)])
                if ok:
                    print("rgbmatrix module installed successfully")
                else:
@@ -1682,7 +1682,13 @@ def execute_system_action():
            })
        elif action == 'install_plugin_requirements':
            active_pm = getattr(api_v3, 'plugin_manager', None)
-            plugins_dir = Path(active_pm.plugins_dir) if active_pm else PROJECT_ROOT / 'plugin-repos'
+            if active_pm:
+                plugins_dir = Path(active_pm.plugins_dir)
+            else:
+                _cm = getattr(api_v3, 'config_manager', None)
+                _cfg = _cm.load_config() if _cm else {}
+                _dir_name = _cfg.get('plugin_system', {}).get('plugins_directory', 'plugin-repos')
+                plugins_dir = Path(_dir_name) if os.path.isabs(_dir_name) else PROJECT_ROOT / _dir_name
            results = []
            if plugins_dir.exists():
                for p in sorted(plugins_dir.iterdir()):
@@ -4619,6 +4625,49 @@ def save_plugin_config():
        if 'application/json' in content_type:
            schema = schema_mgr.load_schema(plugin_id, use_cache=False)

+        # JSON path: fix numeric-keyed dicts that should be arrays.
+        # JS dotToNested() converts feeds.custom_feeds.0.name → {'0': {name:...}}
+        # instead of [{name:...}]. The form-data path has fix_array_structures for this;
+        # mirror that logic here for JSON submissions.
+        if 'application/json' in content_type and schema and 'properties' in schema:
+            def _fix_json_arrays(cfg, props):
+                for k, ps in props.items():
+                    if not isinstance(cfg, dict) or k not in cfg:
+                        continue
+                    pt = ps.get('type')
+                    val = cfg[k]
+                    if pt == 'array':
+                        items_schema = ps.get('items', {})
+                        item_type = items_schema.get('type')
+                        if isinstance(val, dict):
+                            keys = list(val.keys())
+                            if keys and all(str(x).isdigit() for x in keys):
+                                sorted_keys = sorted(keys, key=lambda x: int(str(x)))
+                                arr = [val[sk] for sk in sorted_keys]
+                                if item_type in ('integer', 'number'):
+                                    converted = []
+                                    for v in arr:
+                                        if isinstance(v, str):
+                                            try:
+                                                converted.append(int(v) if item_type == 'integer' else float(v))
+                                            except (ValueError, TypeError):
+                                                converted.append(v)
+                                        else:
+                                            converted.append(v)
+                                    arr = converted
+                                cfg[k] = arr
+                            elif not keys:
+                                cfg[k] = []
+                        # Recurse into each element when items are objects with properties,
+                        # covering both freshly-converted and already-list values.
+                        if item_type == 'object' and 'properties' in items_schema:
+                            for elem in (cfg[k] if isinstance(cfg[k], list) else []):
+                                if isinstance(elem, dict):
+                                    _fix_json_arrays(elem, items_schema['properties'])
+                    elif pt == 'object' and 'properties' in ps and isinstance(val, dict):
+                        _fix_json_arrays(val, ps['properties'])
+            _fix_json_arrays(plugin_config, schema['properties'])
+
        # PRE-PROCESSING: Preserve 'enabled' state if not in request
        # This prevents overwriting the enabled state when saving config from a form that doesn't include the toggle
        if 'enabled' not in plugin_config:
@@ -54,15 +54,18 @@
                const logoIdInput = row.querySelector('input[name*=".logo.id"]');
                
                if (nameInput && urlInput) {
-                    feeds.push({
+                    const feedObj = {
                        name: nameInput.value,
                        url: urlInput.value,
-                        enabled: enabledInput ? enabledInput.checked : true,
-                        logo: logoPathInput || logoIdInput ? {
+                        enabled: enabledInput ? enabledInput.checked : true
+                    };
+                    if (logoPathInput || logoIdInput) {
+                        feedObj.logo = {
                            path: logoPathInput ? logoPathInput.value : '',
                            id: logoIdInput ? logoIdInput.value : ''
-                        } : null
-                    });
+                        };
+                    }
+                    feeds.push(feedObj);
                }
            });
            
@@ -229,7 +229,14 @@
            headers: {'Content-Type': 'application/json'},
            body: JSON.stringify({action})
        })
-        .then(r => r.json())
+        .then(r => {
+            if (!r.ok) {
+                return r.json()
+                    .then(d => Promise.reject(new Error(d.message || `HTTP ${r.status}`)))
+                    .catch(() => Promise.reject(new Error(`HTTP ${r.status}`)));
+            }
+            return r.json();
+        })
        .then(data => {
            const ok = data.status === 'success';
            showResult(
@@ -274,6 +281,7 @@
                    return;
                }

+
                const dirtyBadge = d.dirty
                    ? '<span class="inline-flex items-center px-2 py-0.5 rounded text-xs font-medium bg-yellow-100 text-yellow-800">dirty</span>'
                    : '<span class="inline-flex items-center px-2 py-0.5 rounded text-xs font-medium bg-green-100 text-green-800">clean</span>';