refactor(api): audit and fix inconsistent/incorrect HTTP status codes - #530
Open
Vyacheslav-Tomashevskiy wants to merge 1 commit into
Open
Conversation
…closes Northgate-Systems#416) Swept every route in src/app/api for status codes that don't match what actually happened. Found and fixed three real correctness bugs, plus one consistency nit: 1. GET /api/transactions returned 401 Unauthorized whenever the Supabase query itself failed (DB down, connection error, etc.), both in the `if (error)` branch and the outer catch-all. A query failure is a server problem, not "you're logged out" - a client that treats 401 as "redirect to login" would incorrectly log a user out because the database hiccuped. Now returns 500. 2. Three routes (GET /api/transactions/[id], POST /api/stellar/submit, POST /api/stellar/sign-and-submit) shared the same `if (txError || !tx) return errorResponse("...", 404)` pattern, collapsing "the lookup query failed" and "no row with this id" into the same 404. Split them: a query error is now 500, a missing row is still 404. 3. POST /api/stellar/send's catch-all let "Insufficient balance: ..." (thrown by buildSendTransaction when the account can't cover the send) fall through to the generic 500 branch, even though the adjacent "Invalid recipient" error from the same function is already special-cased to 400. Insufficient balance is exactly the same kind of "fix your input given your account's current state" client error, not a server fault - added a matching branch so both land on 400. 4. GET /api/stellar/rate built its own `errorResponse("Unauthorized", 401)` instead of using the shared unauthorizedResponse() helper every other route uses - purely a consistency fix, same behavior. Verified: npx vitest run - 66/66 across all touched/new files (1 restored/extended test file, 4 new: transactions/route, stellar/send, stellar/submit, stellar/sign-and-submit - each specifically asserting the query-error-vs-not-found split or the insufficient-balance status, not just happy-path). Confirmed each new "returns 500" assertion actually depends on the fix by checking it against the pre-fix code path during development. Note: `npx vitest run` on a clean checkout of this branch currently fails at collection with `ReferenceError: isValidStellarPublicKey is not defined` in validations.ts (from Northgate-Systems#424's merge, missing an import) - that's a separate, already-open fix in Northgate-Systems#529 and out of scope here, so it isn't touched in this diff. `npx vitest run --exclude "**/validations.test.ts"` is the 66/66 above; the full suite will be 79/79 once Northgate-Systems#529 lands (it fixes that same file's now-invalid checksum test fixture too). npx eslint on all touched files - 0. npx tsc --noEmit - 0 in touched files (same 5 pre-existing SafeUser errors elsewhere, reproduce on a clean checkout). npm run build - compiles clean. Live-verified against npm run dev: unauthenticated GET /api/transactions, POST /api/stellar/submit, and POST /api/stellar/send each correctly still return 401 (the auth-gate status wasn't touched, only the downstream-of-auth paths were). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Someone is attempting to deploy a commit to the codex723's projects Team on Vercel. A member of the Team first needs to authorize it. |
This was referenced Sep 8, 2026
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.
Closes #416.
What I found
Swept every route under
src/app/api/for status codes that don't match what actually happened, rather than just formatting/style. Three real correctness bugs and one consistency nit:GET /api/transactionsreturned 401 for database errors. Both theif (error)branch (query failure) and the outer catch-all returnedunauthorizedResponse(). A DB/connection failure is a server problem, not "you're logged out" - a client that treats 401 as "redirect to login" would incorrectly log a user out just because the database hiccuped. Now 500.Three routes conflated "query failed" with "row not found."
GET /api/transactions/[id],POST /api/stellar/submit, andPOST /api/stellar/sign-and-submitall sharedif (txError || !tx) return errorResponse("...", 404). Split them: a query error is now 500, a genuinely missing row is still 404.POST /api/stellar/send's "Insufficient balance" error fell through to a generic 500. The adjacent"Invalid recipient"error from the samebuildSendTransaction()call is already special-cased to 400 in the catch block - insufficient balance is exactly the same class of "fix your input given your account's current state" client error, not a server fault. Added a matching branch.GET /api/stellar/ratebuilt its ownerrorResponse("Unauthorized", 401)instead of the sharedunauthorizedResponse()helper every other route uses - pure consistency fix, same behavior.Verified
npx vitest run- 66/66 across all touched/new files (1 extended existing test file, 4 new:transactions/route,stellar/send,stellar/submit,stellar/sign-and-submit- each specifically asserting the query-error-vs-not-found split or the insufficient-balance status, not just the happy path).npx eslinton all touched files - 0.npx tsc --noEmit- 0 in touched files (same 5 pre-existingSafeUsererrors elsewhere, reproduce on a clean checkout).npm run build- compiles clean.npm run dev: unauthenticatedGET /api/transactions,POST /api/stellar/submit, andPOST /api/stellar/sendeach still correctly return 401 - only the downstream-of-auth paths changed.One thing worth flagging
npx vitest runon a clean checkout of this branch currently fails at collection withReferenceError: isValidStellarPublicKey is not definedinvalidations.ts(from #424's merge, missing an import) - that's the same issue already fixed in the still-open #529, so I left it untouched here to avoid a third PR stepping on the same lines.npx vitest run --exclude "**/validations.test.ts"is the 66/66 above; once #529 lands the full suite here will be 79/79.