Skip to content

refactor(quota-tracker): narrow bare-except to ImportError (closes #1062) - #1180

Closed
bassilkhilo-ag2 wants to merge 2 commits into
sonichi:mainfrom
bassilkhilo-ag2:fix/quota-tracker-narrow-except-1062
Closed

bassilkhilo-ag2 wants to merge 2 commits into
sonichi:mainfrom
bassilkhilo-ag2:fix/quota-tracker-narrow-except-1062

Conversation

@bassilkhilo-ag2

Copy link
Copy Markdown
Collaborator

Problem

skills/quota-tracker/scripts/read-quota.py:31 catches except Exception: around the workspace_default import + status_read_path call. This swallows any exception — not just ImportError — so if status_read_path() ever raises unexpectedly (e.g. a future workspace_default refactor), the error silently falls through to _canonical = None and produces the misleading "No quota-state.json found" message.

This was the exact masking mechanism from the #1055 bug: a path miscalculation raised ImportError, got caught by the broad except Exception:, and froze the quota dashboard for ~12h.

Fix

One-character change: except Exception: → except ImportError:. Only the import-path failure case sets _canonical = None; anything else propagates.

Test

python3 ~/.claude/skills/quota-tracker/scripts/read-quota.py
# → still works: "Status: allowed / 91% remaining"

🤖 Generated with Claude Code

…nichi#1062)

Per Lucy's code review on sonichi#1055: the except Exception on read-quota.py:31
masked a real import path bug for ~12h by silently swallowing any exception
from workspace_default, not just ImportError. Narrow so unexpected exceptions
from status_read_path() (e.g. future workspace_default regressions) propagate
and appear in cron pass logs rather than silently producing the misleading
"No quota-state.json found" message.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@VasiliyRad VasiliyRad left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean re-submission. Single-file, single-line change — exactly what #1165 should have been. The fix is correct: narrowing to ImportError lets any non-import exception from status_read_path() propagate instead of silently collapsing to _canonical = None. Resolves the masking pattern from #1055.

…chi#1062)

5 structural tests verifying the `except ImportError` constraint in
skills/quota-tracker/scripts/read-quota.py. Companion tests to the
1-line fix in the same commit — extracted from the bundled PR sonichi#1165
(which mixed in unrelated changes).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@bassilkhilo-ag2

Copy link
Copy Markdown
Collaborator Author

Added 5 structural tests (tests/quota-bare-except.test.py) to this branch — extracted from PR #1165 which had the same fix bundled with unrelated changes. All tests pass.

Note: VasiliyRad's PR #1094 has the same single-line fix but no tests; PR #1165 has tests but is bundled. This PR (#1180) now has both the clean fix and the tests. PRs #1094, #1165, and this PR all close #1062 — Bassil should pick one to merge.

@bassilkhilo-ag2

Copy link
Copy Markdown
Collaborator Author

Superseded by #1094 (VasiliyRad's fix). Both change the same single line in read-quota.py:25. Deferring to theirs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants