8/8 Correctness: fix /health/, narrow bare except clauses - #642
Open
mcfrank wants to merge 2 commits into
Open
Conversation
The health view listed psutil/celery/kafka/rabbitmq/rss check backends whose packages are not installed, so every request to /health/ 500'd — useless for the EB load balancer probe it exists for. Narrow it to the three checks bundled with this health_check package (Database is the meaningful one; Cache and Storage are cheap). Also align PrimaryHostRedirectMiddleware's skip guard, which looked for "healthcheck" in the URL name but the route is named health_check_home. Fixes #629. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Eight bare excepts swallowed everything, including real bugs (the pandas-3 download crash fixed in #621 hid behind this pattern for a while). Each is narrowed to what it actually guards: ObjectDoesNotExist for a missing backgroundinfo relation, KeyError for optional log-message keys, and KeyError for the empty-DataFrame merge/pivot fallbacks. No behavior change on the happy path; unexpected errors now surface instead of producing silently-empty exports. While here, filed #640 for a genuine pre-existing StudyAPI crash (pandas-3 to_json duplicate-index) surfaced by running the api test suite — left for developer review since the fix can change export shape. Fixes #631. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Final PR of the stack (targets the security branch; merge last). Closes #629, #631.
/health/was 500ing on every request — the view listed psutil/celery/kafka/rabbitmq/rss check backends whose packages aren't installed (ModuleNotFoundError), so the EB load-balancer probe it exists for never worked. Narrowed to the three checks bundled with thishealth_checkpackage (Database is the meaningful one). Also alignedPrimaryHostRedirectMiddleware's skip guard, which looked for "healthcheck" but the route is namedhealth_check_home. Verified returning 200 with the checks running.ObjectDoesNotExist/KeyErroras appropriate — the pandas-3 download crash (fixed in 5/6 Console dashboard + study page rework; test suite green and 3.6x faster #621) hid behind this pattern. No happy-path behavior change; targeted download suite green (38 tests).Filed #640 for a genuine pre-existing bug this surfaced: the
/api/StudyAPI export crashes on pandas 3 (to_jsonduplicate-index). Left for review since the fix can change export shape.🤖 Generated with Claude Code