Add BullMQ background jobs and Swagger OpenAPI docs for backend#54
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (7)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| } | ||
|
|
||
| if (method === 'POST' && path === '/api/jobs/reminders/run') { | ||
| const authedUser = getUserFromAuthHeader(req.headers.authorization); |
There was a problem hiding this comment.
🔴 Missing await on getUserFromAuthHeader makes job endpoints always return 403 Forbidden
At lines 403 and 417, getUserFromAuthHeader (an async function defined at backend/server.js:93) is called without await. This means authedUser is assigned a Promise object, which is always truthy. Since a Promise has no .role property, authedUser.role evaluates to undefined, so the check authedUser.role !== 'admin' is always true, and both endpoints unconditionally return 403 Forbidden.
Root Cause and Impact
Every other auth-protected endpoint in the file uses await getUserFromAuthHeader(...) (e.g., backend/server.js:269, backend/server.js:292, backend/server.js:317). These two new job endpoints omit the await.
Because a Promise is truthy and Promise.role === undefined !== 'admin', the guard condition:
if (!authedUser || authedUser.role !== 'admin') {
sendJson(res, 403, { error: 'Forbidden' });
return;
}always triggers, making POST /api/jobs/reminders/run and POST /api/jobs/cleanup/run completely inaccessible — even for legitimate admin users.
| const authedUser = getUserFromAuthHeader(req.headers.authorization); | |
| const authedUser = await getUserFromAuthHeader(req.headers.authorization); |
Was this helpful? React with 👍 or 👎 to provide feedback.
| } | ||
|
|
||
| if (method === 'POST' && path === '/api/jobs/cleanup/run') { | ||
| const authedUser = getUserFromAuthHeader(req.headers.authorization); |
There was a problem hiding this comment.
🔴 Missing await on getUserFromAuthHeader in cleanup endpoint always returns 403
Same issue as the reminders endpoint: getUserFromAuthHeader is called without await at line 417, so the cleanup endpoint is also permanently inaccessible.
Root Cause
getUserFromAuthHeader is async (backend/server.js:93), so calling it without await returns a Promise. The auth guard always rejects because Promise.role is undefined.
| const authedUser = getUserFromAuthHeader(req.headers.authorization); | |
| const authedUser = await getUserFromAuthHeader(req.headers.authorization); |
Was this helpful? React with 👍 or 👎 to provide feedback.
| } | ||
|
|
||
| await jobSystem.enqueueExpiredSpotCleanup(); | ||
| sendJson(res, 202, { accepted: true, jobsEnabled: jobSystem.enabled }); |
There was a problem hiding this comment.
🔴 Missing return and closing } in cleanup handler causes subsequent routes to be unreachable
After sendJson(res, 202, ...) at line 424, the /api/jobs/cleanup/run handler is missing both a return; statement and the closing } brace for its if block. The next if statement (for /api/presence/heartbeat at line 425) is nested inside the cleanup block instead of being a sibling.
Detailed Explanation
The code at lines 416-425 reads:
if (method === 'POST' && path === '/api/jobs/cleanup/run') {
...
sendJson(res, 202, { accepted: true, jobsEnabled: jobSystem.enabled });
if (method === 'POST' && path === '/api/presence/heartbeat') {The if block for cleanup is never closed. This means:
- For
POST /api/jobs/cleanup/runrequests: After sending the 202 response, execution falls through into the presence/heartbeat code, which will attempt to send a second response on the sameresobject, causing a "headers already sent" error. - For all other requests: The presence heartbeat, presence active, events state PUT/POST, and events state GET routes (lines 425-493) are nested inside the cleanup
ifblock. Since the outer conditionmethod === 'POST' && path === '/api/jobs/cleanup/run'is false for those requests, all those routes become completely unreachable — they will all fall through to the 404 handler.
Impact: The presence and event-state endpoints are broken for all users.
| sendJson(res, 202, { accepted: true, jobsEnabled: jobSystem.enabled }); | |
| sendJson(res, 202, { accepted: true, jobsEnabled: jobSystem.enabled }); | |
| return; | |
| } |
Was this helpful? React with 👍 or 👎 to provide feedback.
| process.on('SIGTERM', async () => { | ||
| await jobSystem.shutdown(); | ||
| process.exit(0); | ||
| console.log(`Cache mode: ${cache.mode}`); |
There was a problem hiding this comment.
🟡 console.log for cache mode placed after process.exit(0) in SIGTERM handler — unreachable and misplaced
At line 512, console.log(\Cache mode: ${cache.mode}`)appears afterprocess.exit(0)` at line 511 inside the SIGTERM handler. This line is unreachable.
Root Cause
This line was originally part of the server.listen callback (visible in the LEFT side of the diff at line 442). During the PR changes, it was accidentally moved into the SIGTERM handler after process.exit(0). As a result:
- The cache mode is never logged at startup (it was removed from the
server.listencallback at lines 498-502). - The line is dead code in the SIGTERM handler since
process.exit(0)terminates the process immediately.
Prompt for agents
In backend/server.js, line 512 (`console.log(\`Cache mode: ${cache.mode}\`);`) is unreachable because it comes after `process.exit(0)` at line 511 inside the SIGTERM handler. This log statement was originally in the `server.listen` callback and should be moved back there. Remove line 512 from the SIGTERM handler and add it to the `server.listen` callback at line 501 (after the Swagger docs log line), so it reads:
server.listen(port, () => {
console.log(`Backend API running on http://localhost:${port}`);
console.log(`Using local database at: ${dbPath}`);
console.log(`Swagger docs available at http://localhost:${port}/api/docs`);
console.log(`Cache mode: ${cache.mode}`);
});
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Motivation
Description
backend/jobs.jsthat scaffolds BullMQ queues/workers for email notifications, spot reminders, and recurring cleanup, and which falls back to a no-op implementation when BullMQ/ioredis are not available.backend/server.jsto initialize jobs on startup, enqueue an email notification after order creation, expose admin endpoints to trigger reminders/cleanup (POST /api/jobs/reminders/runandPOST /api/jobs/cleanup/run), include job status inGET /api/health, and add graceful shutdown hooks.backend/openapi.jsand exposed docs endpoints atGET /api/docs/openapi.jsonandGET /api/docs.backend/db.jswithgetSpotsBetweenfor reminder-window lookups andcleanupExpiredSpotswhich removes expired spots and related orders/order items, and updatedbackend/env.jsand.env.exampleto include Redis/job and other environment settings.Testing
node --check backend/server.js backend/jobs.js backend/openapi.js backend/db.js backend/env.js, which passed.GET /api/healthreturned a health payload includingjobs.enabled,GET /api/docs/openapi.jsonreturned the OpenAPI JSON, andGET /api/docsserved the Swagger HTML; these checks succeeded when run against a live instance.npm install bullmq ioredis swagger-ui-dist dotenv zod) but the install was blocked by the environment (registry returned403 Forbidden), so the job system ran in degraded mode withjobs.enabled: falseas expected.Codex Task