fix(starlark): critical bug fixes and code quality improvements

Critical fixes:
- Fix stack overflow in safeLocalStorage (was recursively calling itself)
- Fix duplicate event listeners on Starlark grid (added sentinel check)
- Fix JSON validation to fail fast on malformed data instead of silently passing

Error handling improvements:
- Narrow exception catches to specific types (OSError, json.JSONDecodeError, ValueError)
- Use logger.exception() with exc_info=True for better stack traces
- Replace generic "except Exception" with specific exception types

Logging improvements:
- Add "[Starlark Pixlet]" context tags to pixlet_renderer logs
- Redact sensitive config values from debug logs (API keys, etc.)
- Add file_path context to schema parsing warnings

Documentation:
- Fix markdown lint issues (add language tags to code blocks)
- Fix time unit spacing: "(5min)" -> "(5 min)"

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
Chuck
2026-02-19 21:32:45 -05:00
parent 6a60a57421
commit 36da426c29
4 changed files with 48 additions and 33 deletions

View File

@@ -27,7 +27,7 @@ The Starlark Apps plugin for LEDMatrix enables you to run **Tidbyt/Tronbyte comm
### Architecture ### Architecture
``` ```text
┌─────────────────────────────────────────────────────────┐ ┌─────────────────────────────────────────────────────────┐
│ LEDMatrix System │ │ LEDMatrix System │
│ ┌────────────────────────────────────────────────────┐ │ │ ┌────────────────────────────────────────────────────┐ │
@@ -199,7 +199,7 @@ Tronbyte/Tidbyt apps are designed for **64×32 displays**. LEDMatrix automatical
The plugin calculates optimal magnification based on your display: The plugin calculates optimal magnification based on your display:
``` ```text
magnify = floor(min(display_width / 64, display_height / 32)) magnify = floor(min(display_width / 64, display_height / 32))
``` ```
@@ -250,7 +250,7 @@ Apps without extracted schemas can still run with default settings.
## File Structure ## File Structure
``` ```text
LEDMatrix/ LEDMatrix/
├── plugin-repos/starlark-apps/ # Plugin source code ├── plugin-repos/starlark-apps/ # Plugin source code
│ ├── manager.py # Main plugin logic │ ├── manager.py # Main plugin logic
@@ -378,7 +378,7 @@ Cached WebP files are stored in `starlark-apps/{app-id}/cached_render.webp`
Balance number of enabled apps with display duration: Balance number of enabled apps with display duration:
- 5 apps × 15s = 75s full cycle - 5 apps × 15s = 75s full cycle
- 20 apps × 15s = 300s (5min) cycle - 20 apps × 15s = 300s (5 min) cycle
Long cycles may cause apps to render before being displayed. Long cycles may cause apps to render before being displayed.

View File

@@ -63,7 +63,7 @@ class StarlarkApp:
try: try:
with open(self.config_file, 'r') as f: with open(self.config_file, 'r') as f:
return json.load(f) return json.load(f)
except Exception as e: except (OSError, json.JSONDecodeError) as e:
logger.warning(f"Could not load config for {self.app_id}: {e}") logger.warning(f"Could not load config for {self.app_id}: {e}")
return {} return {}
@@ -73,7 +73,7 @@ class StarlarkApp:
try: try:
with open(self.schema_file, 'r') as f: with open(self.schema_file, 'r') as f:
return json.load(f) return json.load(f)
except Exception as e: except (OSError, json.JSONDecodeError) as e:
logger.warning(f"Could not load schema for {self.app_id}: {e}") logger.warning(f"Could not load schema for {self.app_id}: {e}")
return None return None
@@ -118,6 +118,11 @@ class StarlarkApp:
if isinstance(value, str) and value.strip().startswith('{'): if isinstance(value, str) and value.strip().startswith('{'):
try: try:
loc = json.loads(value) loc = json.loads(value)
except json.JSONDecodeError as e:
return f"Invalid JSON for key {key}: {e}"
# Validate lat/lng if present
try:
if 'lat' in loc: if 'lat' in loc:
lat = float(loc['lat']) lat = float(loc['lat'])
if not -90 <= lat <= 90: if not -90 <= lat <= 90:
@@ -126,9 +131,8 @@ class StarlarkApp:
lng = float(loc['lng']) lng = float(loc['lng'])
if not -180 <= lng <= 180: if not -180 <= lng <= 180:
return f"Longitude {lng} out of range [-180, 180] for key {key}" return f"Longitude {lng} out of range [-180, 180] for key {key}"
except (json.JSONDecodeError, ValueError, KeyError) as e: except ValueError as e:
# Not a location field, that's fine return f"Invalid numeric value for {key}: {e}"
pass
return None return None
@@ -584,8 +588,8 @@ class StarlarkAppsPlugin(BasePlugin):
fcntl.flock(lock_fd, fcntl.LOCK_UN) fcntl.flock(lock_fd, fcntl.LOCK_UN)
os.close(lock_fd) os.close(lock_fd)
except Exception as e: except (OSError, IOError, json.JSONDecodeError, ValueError) as e:
self.logger.error(f"Error saving manifest: {e}") self.logger.exception("Error saving manifest while writing manifest file", exc_info=True)
# Clean up temp file if it exists # Clean up temp file if it exists
if temp_file is not None and temp_file.exists(): if temp_file is not None and temp_file.exists():
try: try:
@@ -651,8 +655,8 @@ class StarlarkAppsPlugin(BasePlugin):
fcntl.flock(lock_fd, fcntl.LOCK_UN) fcntl.flock(lock_fd, fcntl.LOCK_UN)
os.close(lock_fd) os.close(lock_fd)
except Exception as e: except (OSError, IOError, json.JSONDecodeError, ValueError) as e:
self.logger.error(f"Error updating manifest: {e}") self.logger.exception("Error updating manifest during read-modify-write cycle", exc_info=True)
# Clean up temp file if it exists # Clean up temp file if it exists
if temp_file is not None and temp_file.exists(): if temp_file is not None and temp_file.exists():
try: try:

View File

@@ -41,9 +41,9 @@ class PixletRenderer:
self.pixlet_binary = self._find_pixlet_binary(pixlet_path) self.pixlet_binary = self._find_pixlet_binary(pixlet_path)
if self.pixlet_binary: if self.pixlet_binary:
logger.info(f"Pixlet renderer initialized with binary: {self.pixlet_binary}") logger.info(f"[Starlark Pixlet] Pixlet renderer initialized with binary: {self.pixlet_binary}")
else: else:
logger.warning("Pixlet binary not found - rendering will fail") logger.warning("[Starlark Pixlet] Pixlet binary not found - rendering will fail")
def _find_pixlet_binary(self, explicit_path: Optional[str] = None) -> Optional[str]: def _find_pixlet_binary(self, explicit_path: Optional[str] = None) -> Optional[str]:
""" """
@@ -280,7 +280,13 @@ class PixletRenderer:
"-m", str(magnify) "-m", str(magnify)
]) ])
logger.debug(f"Executing Pixlet: {' '.join(cmd)}") # Build sanitized command for logging (redact sensitive values)
sanitized_cmd = [self.pixlet_binary, "render", star_file]
if config:
config_keys = list(config.keys())
sanitized_cmd.append(f"[{len(config_keys)} config entries: {', '.join(config_keys)}]")
sanitized_cmd.extend(["-o", output_path, "-m", str(magnify)])
logger.debug(f"Executing Pixlet: {' '.join(sanitized_cmd)}")
# Execute rendering # Execute rendering
safe_cwd = self._get_safe_working_directory(star_file) safe_cwd = self._get_safe_working_directory(star_file)
@@ -373,6 +379,7 @@ class PixletRenderer:
# Extract get_schema() function body # Extract get_schema() function body
schema_body = self._extract_get_schema_body(content) schema_body = self._extract_get_schema_body(content)
if not schema_body: if not schema_body:
logger.debug(f"No get_schema() function found in {file_path}")
return None return None
# Extract version # Extract version
@@ -397,6 +404,7 @@ class PixletRenderer:
if bracket_count != 0: if bracket_count != 0:
# Unmatched brackets # Unmatched brackets
logger.warning(f"Unmatched brackets in schema fields for {file_path}")
return {"version": version, "schema": []} return {"version": version, "schema": []}
fields_text = schema_body[fields_start_match.end():i-1] fields_text = schema_body[fields_start_match.end():i-1]

View File

@@ -4,7 +4,7 @@ const safeLocalStorage = {
getItem(key) { getItem(key) {
try { try {
if (typeof localStorage !== 'undefined') { if (typeof localStorage !== 'undefined') {
return safeLocalStorage.getItem(key); return localStorage.getItem(key);
} }
} catch (e) { } catch (e) {
console.warn(`safeLocalStorage.getItem failed for key "${key}":`, e.message); console.warn(`safeLocalStorage.getItem failed for key "${key}":`, e.message);
@@ -14,7 +14,7 @@ const safeLocalStorage = {
setItem(key, value) { setItem(key, value) {
try { try {
if (typeof localStorage !== 'undefined') { if (typeof localStorage !== 'undefined') {
safeLocalStorage.setItem(key, value); localStorage.setItem(key, value);
return true; return true;
} }
} catch (e) { } catch (e) {
@@ -7924,24 +7924,27 @@ setTimeout(function() {
</div>`; </div>`;
}).join(''); }).join('');
// Add delegated event listeners for install and view buttons // Add delegated event listener only once (prevent duplicate handlers)
grid.addEventListener('click', function(e) { if (!grid.dataset.starlarkHandlerAttached) {
const button = e.target.closest('button[data-action]'); grid.addEventListener('click', function handleStarlarkGridClick(e) {
if (!button) return; const button = e.target.closest('button[data-action]');
if (!button) return;
const card = button.closest('.plugin-card'); const card = button.closest('.plugin-card');
if (!card) return; if (!card) return;
const appId = card.dataset.appId; const appId = card.dataset.appId;
if (!appId) return; if (!appId) return;
const action = button.dataset.action; const action = button.dataset.action;
if (action === 'install') { if (action === 'install') {
window.installStarlarkApp(appId); window.installStarlarkApp(appId);
} else if (action === 'view') { } else if (action === 'view') {
window.open('https://github.com/tronbyt/apps/tree/main/apps/' + encodeURIComponent(appId), '_blank'); window.open('https://github.com/tronbyt/apps/tree/main/apps/' + encodeURIComponent(appId), '_blank');
} }
}); });
grid.dataset.starlarkHandlerAttached = 'true';
}
} }
// ── Filter UI Updates ─────────────────────────────────────────────────── // ── Filter UI Updates ───────────────────────────────────────────────────