perf: offload sync webhook dispatch to bullmq#223
Conversation
|
Warning Review limit reached
More reviews will be available in 37 minutes and 50 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR migrates webhook dispatching from synchronous calls to asynchronous queue jobs. Controllers enqueue lightweight ChangesWebhook Dispatch Async Queue Migration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/dashboard-api/src/__tests__/project.controller.softDelete.test.js`:
- Around line 164-172: The webhook job assertion in the soft-delete recovery
test is missing the collection field; update the expectation on
mockWebhookQueueAdd (the expect.objectContaining block used when asserting the
'trigger-webhook' call) to include collection: 'posts' alongside event:
'document.recovered', payload: restoredDoc, and projectId: 'proj_1' so the job
data fully matches the webhook payload shape.
In `@apps/public-api/src/__tests__/softDelete.test.js`:
- Around line 141-150: The test assertion for webhookQueue.add is missing
verification of the collection field; update the expect.objectContaining passed
to webhookQueue.add (the call with 'trigger-webhook' and restoredDoc) to include
collection: 'posts' so the job data assertion checks event, payload, projectId
and collection (i.e., add collection: 'posts' to the object passed to
expect.objectContaining in the test).
In `@packages/common/src/queues/webhookQueue.js`:
- Around line 85-102: Wrap the "trigger-webhook" branch (where job.name ===
"trigger-webhook") in a try/catch and call dispatchWebhooks inside the try; on
catch, log a detailed error containing projectId, collection, action, and
documentId using the existing logger in scope (e.g., processLogger or logger)
and then rethrow or fail the job so queue retry behavior is preserved; ensure
you reference dispatchWebhooks and the job.name check when applying the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fdf2da9a-6ee4-404e-81cd-7d7dcdcd9808
📒 Files selected for processing (5)
apps/dashboard-api/src/__tests__/project.controller.softDelete.test.jsapps/dashboard-api/src/controllers/project.controller.jsapps/public-api/src/__tests__/softDelete.test.jsapps/public-api/src/controllers/data.controller.jspackages/common/src/queues/webhookQueue.js
|
@coderabbitai |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
echo "=== PR Status ==="
gh pr view 223 --repo geturbackend/urBackend --json title,state,mergeable,mergeStateStatus,reviewDecision,statusCheckRollup,reviews,comments,isDraft
echo ""
echo "=== PR Checks ==="
gh pr checks 223 --repo geturbackend/urBackend
echo ""
echo "=== Open Review Comments ==="
gh api repos/geturbackend/urBackend/pulls/223/comments --jq '.[] | {id: .id, body: .body, resolved: .position, user: .user.login}' 2>/dev/null | head -100
echo ""
echo "=== PR Reviews ==="
gh api repos/geturbackend/urBackend/pulls/223/reviews --jq '.[] | {user: .user.login, state: .state, submitted_at: .submitted_at}' 2>/dev/nullLength of output: 36314
Everything is in order — no blockers. You're clear to merge. 🐇 |
yash-pouranik
added
enhancement
2 participants
🚀 Pull Request Description
I had resolve a critical bottleneck in public API where synchronous webhook resolution was blocking the main Node.js event loop.
Previously, dispatchWebhooks was executed directly at the tail end of CRUD mutations, which tied API response times to database queries and payload construction logic.
Now It decouple the webhook resolution from the incoming API request by utilizing our BullMQ cluster, ensuring that data routes immediately return an HTTP 2XX response after database mutations are completed.
Fixes #217
🛠️ Type of Change
🧪 Testing & Validation
Backend Verification:
npm testin thebackend/directory and all tests passed.Frontend Verification:
npm run lintin thefrontend/directory.✅ Checklist
Summary by CodeRabbit
Bug Fixes
Tests