Skip to content

fix: return proper HTTP 500 on GET /rebalance/history errors - #52

Merged
Uchechukwu-Ekezie merged 2 commits into
grantFoxin:mainfrom
brightpixel-dev:fix/issue-46-rebalance-history-500-status
Jul 23, 2026
Merged

fix: return proper HTTP 500 on GET /rebalance/history errors#52
Uchechukwu-Ekezie merged 2 commits into
grantFoxin:mainfrom
brightpixel-dev:fix/issue-46-rebalance-history-500-status

Conversation

@brightpixel-dev

Copy link
Copy Markdown
Contributor

Fixes #46

What changed

  • GET /rebalance/history catch block now returns .status(500) before .json()
  • Checked every other catch block in routes.ts (33 total) for the same missing-status pattern — this was the only one
  • Removed 3 unused imports blocking npm run build in the frontend (unrelated pre-existing TS6133 errors, needed for a clean build per the check requirements)

Why

Without the explicit status, the response defaulted to HTTP 200. The frontend's response.ok check treated the error response as valid data, silently showing an empty history instead of surfacing the failure.

How to test

  • cd backend && npm test — 103 tests pass
  • cd frontend && npm install --legacy-peer-deps && npm run build — builds clean
  • Note: cd contracts && cargo test --all fails on a pre-existing, unrelated issue — no committed Cargo.lock, so a fresh resolve pulls ed25519-dalek 3.0.0, incompatible with soroban-env-host 21.2.1. Nothing in this PR touches contracts/.

@Uchechukwu-Ekezie ready for review.

Missing .status(500) meant the frontend's response.ok check treated
error responses as valid empty history. Checked every other catch
block in routes.ts for the same pattern — this was the only one.

Fixes grantFoxin#46
Pre-existing tsc errors (TS6133) unrelated to grantFoxin#46, but the issue
requires a clean 'npm run build' before the PR can be merged.
@Uchechukwu-Ekezie

Uchechukwu-Ekezie commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

core fix is exactly right, one line, matches the issue precisely.

checked the three unused import removals individually since removing the wrong one would just trade one broken build for a broken render. NotificationTest isn't referenced anywhere else in Dashboard.tsx, Send only shows up as plain text in a sentence not as the icon component in NotificationPreferences, and Activity has zero other references in PriceTracker. all three are genuinely dead, none of this is silently breaking anything.

small thing, not a blocker: NotificationTest.tsx itself is now an orphaned file, nothing imports it anymore. fine to leave for a separate cleanup pass since removing the whole component is outside what this PR is actually fixing.

also checked your cargo claim, contracts has a Cargo.toml but no committed Cargo.lock, so yeah a fresh resolve pulling in an incompatible ed25519-dalek is exactly the kind of thing that happens without a lockfile pinning it. good catch flagging that instead of chasing it.

nice job

@Uchechukwu-Ekezie
Uchechukwu-Ekezie merged commit b548987 into grantFoxin:main Jul 23, 2026
3 checks passed
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.

Return proper HTTP 500 status on GET /rebalance/history errors

2 participants