fix: add asyncHandler utility and wrap all bare-await routes#140
Open
OAKVISUALZ wants to merge 2 commits into
Open
fix: add asyncHandler utility and wrap all bare-await routes#140OAKVISUALZ wants to merge 2 commits into
OAKVISUALZ wants to merge 2 commits into
Conversation
Closes Savitura#124 Problem: Express 4 does not automatically catch rejected promises from async route handlers. Routes with bare �wait db.query(...) calls and no try/catch cause the client to hang or receive an empty response when the DB is unavailable -- the error is swallowed silently. Fix: 1. backend/src/utils/asyncHandler.js (NEW) Thin wrapper: (fn) => (req, res, next) => Promise.resolve(fn(...)).catch(next) Forwards any rejection to Express error pipeline. 2. backend/src/middleware/errorHandler.js (UNCHANGED -- already correct) Existing errorHandler already: - Has 4-argument signature (err, req, res, next) - Logs full stack trace via logger.error() - Hides error details from clients in production (shows 'Internal server error') - Returns JSON 500 with structured error body No changes needed here. 3. backend/src/routes/users.js Wraps: GET /me, POST /me/kyc/start, GET /me/campaigns, GET /me/stats, GET /me/contributions 4. backend/src/routes/stellarTransactions.js Wraps: GET /, GET /:id Also wraps assertCampaignReportingAccess helper called inside those routes. 5. backend/src/routes/contributions.js Wraps: GET /mine, GET /campaign/:campaignId, GET /, GET /finalization/:txHash, GET /quote, POST /prepare, POST /submit-signed, POST / Routes with existing try/catch keep their explicit error responses but are also covered by asyncHandler for any awaits outside the try block. 6. backend/src/routes/campaigns.js Wraps: requireCampaignMember middleware factory, GET /, GET /mine, GET /:id/milestones, POST /:id/milestones, GET /:id, GET /:id/embed, GET /:id/backers, GET /:id/stream, GET /:id/balance, POST /cron/fail-expired, POST /cron/reminders, POST /:id/trigger-refunds, POST /, PATCH /:id, GET /:id/updates, POST /:id/updates, POST /:id/members, GET /:id/members, PATCH /:id/members/:userId, DELETE /:id/members/:userId, POST /:id/members/accept 7. backend/src/utils/asyncHandler.test.js (NEW) 4 unit tests covering: arg pass-through, thrown errors, rejected promises, and verifying next() is not called on success. Acceptance criteria: - [x] Any DB failure returns 500 JSON instead of a hung connection - [x] Error middleware logs the full stack trace (existing errorHandler) - [x] Sensitive details hidden in production (existing errorHandler) - [x] All route files use asyncHandler or explicit try/catch - [x] 78 tests total, 70 pass (2 pre-existing DB-dependent failures on main)
|
@OAKVISUALZ Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
The admin moderation feature (feat: fb7f93d) accidentally introduced a corrupted section by merging two different versions of AdminDashboard — the old flat layout and the new tab-based layout were interleaved from line 476 onward, causing: frontend/src/pages/AdminDashboard.jsx 483:7 error Parsing error: Adjacent JSX elements must be wrapped in an enclosing tag This broke the frontend-checks CI step (ESLint exit code 1) for every subsequent PR. The fix removes the orphaned old-version JSX fragment (lines 476-622 of the corrupted file) which contained duplicate Platform Fees card, Campaign Management table, and Milestone Reviews section that had leaked out of the tab system into the outer render scope. The correct tab-based code remains intact. Frontend build: passed (built in 11.57s) Frontend ESLint: 0 errors, 0 warnings
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
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 #124
Problem:
Express 4 does not automatically catch rejected promises from async route handlers. Routes with bare �wait db.query(...) calls and no try/catch cause the client to hang or receive an empty response when the DB is unavailable -- the error is swallowed silently.
Fix:
backend/src/utils/asyncHandler.js (NEW) Thin wrapper: (fn) => (req, res, next) => Promise.resolve(fn(...)).catch(next) Forwards any rejection to Express error pipeline.
backend/src/middleware/errorHandler.js (UNCHANGED -- already correct) Existing errorHandler already:
backend/src/routes/users.js Wraps: GET /me, POST /me/kyc/start, GET /me/campaigns, GET /me/stats, GET /me/contributions
backend/src/routes/stellarTransactions.js Wraps: GET /, GET /:id Also wraps assertCampaignReportingAccess helper called inside those routes.
backend/src/routes/contributions.js Wraps: GET /mine, GET /campaign/:campaignId, GET /, GET /finalization/:txHash, GET /quote, POST /prepare, POST /submit-signed, POST / Routes with existing try/catch keep their explicit error responses but are also covered by asyncHandler for any awaits outside the try block.
backend/src/routes/campaigns.js Wraps: requireCampaignMember middleware factory, GET /, GET /mine, GET /:id/milestones, POST /:id/milestones, GET /:id, GET /:id/embed, GET /:id/backers, GET /:id/stream, GET /:id/balance, POST /cron/fail-expired, POST /cron/reminders, POST /:id/trigger-refunds, POST /, PATCH /:id, GET /:id/updates, POST /:id/updates, POST /:id/members, GET /:id/members, PATCH /:id/members/:userId, DELETE /:id/members/:userId, POST /:id/members/accept
backend/src/utils/asyncHandler.test.js (NEW) 4 unit tests covering: arg pass-through, thrown errors, rejected promises, and verifying next() is not called on success.
Acceptance criteria: