Refactor step 4: narrow broad except Exception catches
Replaced 28 bare `except Exception` catches across 8 files with specific exception types reflecting the actual failure modes: - JSON file reads → (OSError, json.JSONDecodeError) - datetime parsing → ValueError - base64 decoding → ValueError - YAML parsing → (OSError, yaml.YAMLError); import moved above try - GeoJSON coord extraction → (TypeError, IndexError, AttributeError) - Startup temp-file cleanup → OSError - Single JSON line parsing (SSE batch) → json.JSONDecodeError Kept broad catches only where intentional: - Background thread top-level guards (tasks.py, admin.py) with log.exception - SSE stream generator tops (strava.py, garmin.py, uploads.py) - Per-item batch loops that must not abort the whole operation - Explicitly non-fatal post-upload merge steps with log.warning
This commit is contained in:
Davide Scaini
@@ -192,7 +192,7 @@ async def delete_activity(
|
||||
idx = json.loads(index_path.read_text(encoding="utf-8"))
|
||||
idx["activities"] = [a for a in idx.get("activities", []) if a.get("id") != activity_id]
|
||||
index_path.write_text(json.dumps(idx, indent=2, ensure_ascii=False))
|
||||
except Exception:
|
||||
except (OSError, json.JSONDecodeError):
|
||||
pass # corrupt index — merge_all will clean up on next run
|
||||
|
||||
# Remove from dedup cache so the file can be re-uploaded if needed
|
||||
@@ -205,7 +205,7 @@ async def delete_activity(
|
||||
a for a in cache["activities"] if a.get("id") != activity_id
|
||||
]
|
||||
cache_path.write_text(json.dumps(cache, indent=2, ensure_ascii=False))
|
||||
except Exception:
|
||||
except (OSError, json.JSONDecodeError):
|
||||
pass # corrupt cache — leave it; next extract will rebuild
|
||||
|
||||
# Full merge needed: activity removed from index
|
||||
@@ -289,13 +289,13 @@ async def get_athlete(bincio_session: str | None = Cookie(default=None)) -> JSON
|
||||
# Layer edits/athlete.yaml on top
|
||||
edits_path = dd / "edits" / "athlete.yaml"
|
||||
if edits_path.exists():
|
||||
try:
|
||||
import yaml
|
||||
try:
|
||||
edits = yaml.safe_load(edits_path.read_text(encoding="utf-8")) or {}
|
||||
for k in ("max_hr", "ftp_w", "hr_zones", "power_zones", "seasons", "gear"):
|
||||
if k in edits:
|
||||
data[k] = edits[k]
|
||||
except Exception:
|
||||
except (OSError, yaml.YAMLError):
|
||||
pass
|
||||
return JSONResponse(data)
|
||||
|
||||
|
||||
@@ -331,7 +331,7 @@ async def admin_reextract_originals(
|
||||
total_imported += evt.get("imported", 0)
|
||||
total_skipped += evt.get("skipped", 0)
|
||||
total_errors += evt.get("errors", 0)
|
||||
except Exception:
|
||||
except json.JSONDecodeError:
|
||||
pass
|
||||
|
||||
await proc.wait()
|
||||
@@ -390,7 +390,7 @@ async def admin_diag(
|
||||
try:
|
||||
idx = json.loads(merged_index.read_text())
|
||||
merged_activity_count = len(idx.get("activities", []))
|
||||
except Exception:
|
||||
except (OSError, json.JSONDecodeError):
|
||||
merged_activity_count = -1
|
||||
|
||||
root_activity_count: int | None = None
|
||||
@@ -398,7 +398,7 @@ async def admin_diag(
|
||||
try:
|
||||
idx = json.loads(root_index.read_text())
|
||||
root_activity_count = len(idx.get("activities", []))
|
||||
except Exception:
|
||||
except (OSError, json.JSONDecodeError):
|
||||
root_activity_count = -1
|
||||
|
||||
# Peek at a few filenames in activities/ to understand the actual state
|
||||
@@ -497,7 +497,7 @@ async def admin_delete_user_directory(
|
||||
from bincio.render.cli import _write_root_manifest
|
||||
try:
|
||||
_write_root_manifest(deps._get_data_dir())
|
||||
except Exception:
|
||||
except (OSError, json.JSONDecodeError):
|
||||
pass
|
||||
return JSONResponse({"ok": True})
|
||||
|
||||
@@ -523,7 +523,7 @@ async def admin_strava_sync_status(
|
||||
sc = json.loads(sync_path.read_text(encoding="utf-8"))
|
||||
last_sync = sc.get("last_sync")
|
||||
total_imported = len(sc.get("imported_ids", []))
|
||||
except Exception:
|
||||
except (OSError, json.JSONDecodeError):
|
||||
pass
|
||||
|
||||
run_status: str | None = None
|
||||
@@ -540,7 +540,7 @@ async def admin_strava_sync_status(
|
||||
run_errors = ss.get("errors", 0)
|
||||
run_error_message = ss.get("error_message")
|
||||
last_run = ss.get("last_run")
|
||||
except Exception:
|
||||
except (OSError, json.JSONDecodeError):
|
||||
pass
|
||||
|
||||
users.append({
|
||||
|
||||
@@ -37,7 +37,7 @@ async def list_ideas(
|
||||
for path in sorted(_ideas_dir(dd).glob("*.json")):
|
||||
try:
|
||||
idea = json.loads(path.read_text(encoding="utf-8"))
|
||||
except Exception:
|
||||
except (OSError, json.JSONDecodeError):
|
||||
continue
|
||||
votes = idea.get("votes", [])
|
||||
idea["vote_count"] = len(votes)
|
||||
@@ -161,7 +161,7 @@ async def delete_idea(
|
||||
raise HTTPException(404, "Not found")
|
||||
try:
|
||||
idea = json.loads(path.read_text(encoding="utf-8"))
|
||||
except Exception:
|
||||
except (OSError, json.JSONDecodeError):
|
||||
raise HTTPException(500, "Could not read idea")
|
||||
if not user.is_admin and idea.get("author") != user.handle:
|
||||
raise HTTPException(403, "Forbidden")
|
||||
@@ -221,7 +221,7 @@ async def submit_feedback(
|
||||
if log_file.exists():
|
||||
try:
|
||||
existing = json.loads(log_file.read_text())
|
||||
except Exception:
|
||||
except (OSError, json.JSONDecodeError):
|
||||
existing = []
|
||||
existing.append(entry)
|
||||
log_file.write_text(json.dumps(existing, indent=2))
|
||||
|
||||
@@ -151,7 +151,7 @@ async def me_delete_account(
|
||||
from bincio.render.cli import _write_root_manifest
|
||||
try:
|
||||
_write_root_manifest(deps._get_data_dir())
|
||||
except Exception:
|
||||
except (OSError, json.JSONDecodeError):
|
||||
pass
|
||||
|
||||
resp = JSONResponse({"ok": True})
|
||||
@@ -214,7 +214,7 @@ async def me_get_strava_credentials(bincio_session: str | None = Cookie(default=
|
||||
if cid and csec:
|
||||
has_user_creds = True
|
||||
client_id_hint = cid
|
||||
except Exception:
|
||||
except (OSError, json.JSONDecodeError):
|
||||
pass
|
||||
return JSONResponse({
|
||||
"has_user_creds": has_user_creds,
|
||||
@@ -242,7 +242,7 @@ async def me_set_strava_credentials(
|
||||
try:
|
||||
existing = json.loads(creds_path.read_text(encoding="utf-8"))
|
||||
csec = str(existing.get("client_secret", "")).strip()
|
||||
except Exception:
|
||||
except (OSError, json.JSONDecodeError):
|
||||
pass
|
||||
if not csec:
|
||||
raise HTTPException(400, "client_secret is required (no existing secret to preserve)")
|
||||
@@ -255,7 +255,7 @@ async def me_set_strava_credentials(
|
||||
old_cid = str(json.loads(creds_path.read_text(encoding="utf-8")).get("client_id", "")).strip()
|
||||
if old_cid and old_cid != cid:
|
||||
token_path.unlink(missing_ok=True)
|
||||
except Exception:
|
||||
except (OSError, json.JSONDecodeError):
|
||||
pass
|
||||
|
||||
creds_path.write_text(
|
||||
|
||||
@@ -32,7 +32,7 @@ def _scan_segment_for_user(dd: Path, handle: str, segment_id: str) -> int:
|
||||
continue
|
||||
try:
|
||||
detail = json.loads(detail_path.read_text(encoding="utf-8"))
|
||||
except Exception:
|
||||
except (OSError, json.JSONDecodeError, ValueError):
|
||||
continue
|
||||
ts_url = detail.get("timeseries_url")
|
||||
if not ts_url:
|
||||
@@ -42,14 +42,14 @@ def _scan_segment_for_user(dd: Path, handle: str, segment_id: str) -> int:
|
||||
continue
|
||||
try:
|
||||
ts = json.loads(ts_path.read_text(encoding="utf-8"))
|
||||
except Exception:
|
||||
except (OSError, json.JSONDecodeError, ValueError):
|
||||
continue
|
||||
started_raw = detail.get("started_at")
|
||||
if not started_raw:
|
||||
continue
|
||||
try:
|
||||
started_at = _datetime.fromisoformat(started_raw.replace("Z", "+00:00"))
|
||||
except Exception:
|
||||
except ValueError:
|
||||
continue
|
||||
track = track_from_timeseries_json(ts, detail.get("id", detail_path.stem),
|
||||
detail.get("sport", "other"), started_at)
|
||||
@@ -228,7 +228,7 @@ async def me_segment_rescan(
|
||||
continue
|
||||
try:
|
||||
detail = _json.loads(detail_path.read_text(encoding="utf-8"))
|
||||
except Exception:
|
||||
except (OSError, _json.JSONDecodeError, ValueError):
|
||||
continue
|
||||
ts_url = detail.get("timeseries_url")
|
||||
if not ts_url:
|
||||
@@ -238,14 +238,14 @@ async def me_segment_rescan(
|
||||
continue
|
||||
try:
|
||||
ts = _json.loads(ts_path.read_text(encoding="utf-8"))
|
||||
except Exception:
|
||||
except (OSError, _json.JSONDecodeError, ValueError):
|
||||
continue
|
||||
started_raw = detail.get("started_at")
|
||||
if not started_raw:
|
||||
continue
|
||||
try:
|
||||
started_at = _datetime.fromisoformat(started_raw.replace("Z", "+00:00"))
|
||||
except Exception:
|
||||
except ValueError:
|
||||
continue
|
||||
track = track_from_timeseries_json(
|
||||
ts, detail.get("id", detail_path.stem),
|
||||
|
||||
@@ -82,7 +82,7 @@ async def strava_reset(request: Request, bincio_session: Optional[str] = Cookie(
|
||||
dt = datetime.fromisoformat(latest.replace("Z", "+00:00"))
|
||||
last_ts = int(dt.astimezone(timezone.utc).timestamp())
|
||||
break
|
||||
except Exception:
|
||||
except (OSError, json.JSONDecodeError, ValueError):
|
||||
continue
|
||||
|
||||
if last_ts is None:
|
||||
|
||||
@@ -42,7 +42,7 @@ def _upsert_index_summary(user_dir: Path, activity_id: str, activity: dict, geoj
|
||||
if coords:
|
||||
step = max(1, len(coords) // 9)
|
||||
preview = [[c[1], c[0]] for c in coords[::step]][:9]
|
||||
except Exception:
|
||||
except (TypeError, IndexError, AttributeError):
|
||||
pass
|
||||
|
||||
has_track = (user_dir / "activities" / f"{activity_id}.geojson").exists()
|
||||
@@ -183,7 +183,7 @@ async def upload_raw_activity(
|
||||
|
||||
try:
|
||||
raw = _b64.b64decode(b64)
|
||||
except Exception:
|
||||
except ValueError:
|
||||
raise HTTPException(400, "Invalid base64 encoding")
|
||||
|
||||
source_hash = hashlib.sha256(raw).hexdigest()
|
||||
@@ -378,7 +378,7 @@ async def upload_activity(
|
||||
cache = json.loads(cache_path.read_text(encoding="utf-8"))
|
||||
cache.pop(activity_id, None)
|
||||
cache_path.write_text(json.dumps(cache, ensure_ascii=False))
|
||||
except Exception:
|
||||
except (OSError, json.JSONDecodeError):
|
||||
pass
|
||||
# Remove merged copies (merge_all will regenerate them after ingest)
|
||||
merged_acts = dd / "_merged" / "activities"
|
||||
|
||||
@@ -39,7 +39,7 @@ async def _on_startup() -> None:
|
||||
for p in _glob.glob(str(data_dir / "*" / "tmp*.zip")):
|
||||
try:
|
||||
Path(p).unlink()
|
||||
except Exception:
|
||||
except OSError:
|
||||
pass
|
||||
if deps.webroot is not None:
|
||||
threading.Thread(target=tasks._site_rebuild_worker, daemon=True, name="site-rebuild").start()
|
||||
|
||||
+1
-1
@@ -492,6 +492,6 @@ def test_activity_geojson_missing_geometry(client, tmp_path, authenticated_sessi
|
||||
| 1 | Extract shared image utilities → `bincio/shared/images.py` | Done |
|
||||
| 2 | Extract HTML template → `bincio/edit/templates/edit.html` | Done |
|
||||
| 3 | Split `serve/server.py` into `deps.py` + `routers/*` | Done |
|
||||
| 4 | Narrow broad `except Exception:` catches | Not started |
|
||||
| 4 | Narrow broad `except Exception:` catches | Done |
|
||||
|
||||
> **Note on dependency pinning**: not included. `uv.lock` already pins every dependency (including transitives) to exact versions, which is strictly stronger than switching `>=` to `~=` in `pyproject.toml`. The lockfile is the right mechanism for this concern.
|
||||
|
||||
Reference in New Issue
Block a user