fix: narrow swallowed exception in get_pitching_peripherals() (#10) #35
No reviewers
Labels
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: cal/paper-dynasty-card-creation#35
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "ai/paper-dynasty-card-creation#10"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #10
Summary
except Exception: passtoexcept KeyError: passinget_pitching_peripherals()(creation_helpers.py:536)cell["data-append-csv"]being absent from an HTML table cell — aKeyErroris the only expected failure moderequests.exceptions.ConnectionError), encoding errors, and structural HTML changes would be silently swallowed and produce empty/corrupt dataFiles changed
creation_helpers.py— 1-line change at line 536Test results
No automated test suite; change verified by reading modified file. Diff is exactly one token changed.
Other observations
Two additional broad exception catches exist in this file at lines ~1152 and ~1224 (
except Exception as e: pass). These are out of scope for this issue but may warrant similar attention.AI Code Review
Files Reviewed
creation_helpers.py(modified — line 536)Findings
Correctness
Tag.__getitem__raisesKeyErrorwhen a named attribute is absent — it is the one and only expected failure mode forcell["data-append-csv"]. Narrowing fromexcept Exceptiontoexcept KeyErroris the right fix.requests.get(url)at line 521 andcell["data-stat"]at line 540 are outside the try/except and therefore unaffected by this change — that is correct.Security
Style & Conventions
Suggestions
except Exception as e: passsites at lines ~1152 and ~1224. These are out of scope here but worth follow-up issues — the same silent-failure risk applies.Verdict: APPROVED
Minimal, focused, correct.
KeyErroris the precise exception type for missing BS4 tag attributes. No issues blocking merge.Automated review by Claude PR Reviewer
Approved — safe bug fix per PO triage.