feat(notifier): add webhook retry tests; surge pricing implementation complete#373
Conversation
…koff Adds unit tests for the services/notifier webhook delivery logic (issue LightForgeHub#297). Covers: successful delivery, no retry on success, 1s delay after first failure, 2^n exponential backoff schedule, stops after 5 retries (no 6th call), and consistent payload across retries. Adds jest/ts-jest to devDependencies and a test script. The surge-pricing implementation (issue LightForgeHub#296) — active-session tracking per category, 1.0×–2.0× dynamic multiplier, and seeker approval via start_session_with_surge — was contributed upstream and is complete in contracts/src/lib.rs and contracts/src/reputation.rs. closes LightForgeHub#296 closes LightForgeHub#297
📝 WalkthroughWalkthroughAdds Jest support for the notifier package and a test suite that validates webhook payload structure, success handling, exponential backoff retries, retry limits, and payload consistency across retry attempts. ChangesNotifier Jest test harness
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
services/notifier/notifier.test.tsOops! Something went wrong! :( ESLint: 9.39.4 SyntaxError: Error while loading rule ' 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 Warning |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@services/notifier/notifier.test.ts`:
- Around line 16-35: The notifier test is duplicating the webhook implementation
instead of exercising the real production logic. Move sendWebhook out of the
runtime entrypoint in services/notifier/index.ts into a side-effect-free shared
module, then import that function into notifier.test.ts so the test covers the
actual payload shape, retry limit, and backoff behavior rather than a local
clone.
- Around line 122-134: The retry test in sendWebhook only asserts event_type and
contract_id, so it can miss changes to other fields. Update the each retry
receives the same event payload test in notifier.test.ts to compare the full
payload object for every mockPost call, including topic and value, using the
existing event from makeEvent and the sendWebhook retry flow.
🪄 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: 90901490-9867-441c-bdc4-eca089957ab3
📒 Files selected for processing (2)
services/notifier/notifier.test.tsservices/notifier/package.json
| // sendWebhook is not exported directly — test it via re-import of the extracted logic. | ||
| // We extract the function under test to keep it unit-testable without the top-level | ||
| // env-check guard (which calls process.exit on missing env vars). | ||
|
|
||
| const sendWebhook = async (event: any, retryCount = 0): Promise<void> => { | ||
| const WEBHOOK_URL = 'https://example.com/webhook'; | ||
| try { | ||
| await mockPost(WEBHOOK_URL, { | ||
| event_type: event.type, | ||
| contract_id: event.contractId, | ||
| topic: event.topic, | ||
| value: event.value, | ||
| timestamp: new Date().toISOString(), | ||
| }); | ||
| } catch { | ||
| if (retryCount < 5) { | ||
| const delay = Math.pow(2, retryCount) * 1000; | ||
| setTimeout(() => sendWebhook(event, retryCount + 1), delay); | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Test the production sendWebhook, not a local clone.
This suite reimplements the retry/payload logic instead of importing the runtime path from services/notifier/index.ts:18-33. As written, the tests can stay green even if the real notifier’s payload shape, retry cap, or backoff behavior regresses. Please extract sendWebhook into a side-effect-free module and import that here.
🤖 Prompt for 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.
In `@services/notifier/notifier.test.ts` around lines 16 - 35, The notifier test
is duplicating the webhook implementation instead of exercising the real
production logic. Move sendWebhook out of the runtime entrypoint in
services/notifier/index.ts into a side-effect-free shared module, then import
that function into notifier.test.ts so the test covers the actual payload shape,
retry limit, and backoff behavior rather than a local clone.
| it('each retry receives the same event payload', async () => { | ||
| const event = makeEvent('evt-retry-check'); | ||
| mockPost.mockRejectedValueOnce(new Error('fail')); | ||
| mockPost.mockResolvedValueOnce({ status: 200 }); | ||
|
|
||
| await sendWebhook(event); | ||
| await jest.advanceTimersByTimeAsync(1000); | ||
|
|
||
| const allPayloads = mockPost.mock.calls.map(([, p]) => p); | ||
| for (const payload of allPayloads) { | ||
| expect(payload.event_type).toBe(event.type); | ||
| expect(payload.contract_id).toBe(event.contractId); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the full retry payload.
each retry receives the same event payload currently only checks event_type and contract_id, so changes to topic or value across retries would still pass.
Suggested assertion update
const allPayloads = mockPost.mock.calls.map(([, p]) => p);
for (const payload of allPayloads) {
expect(payload.event_type).toBe(event.type);
expect(payload.contract_id).toBe(event.contractId);
+ expect(payload.topic).toEqual(event.topic);
+ expect(payload.value).toEqual(event.value);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('each retry receives the same event payload', async () => { | |
| const event = makeEvent('evt-retry-check'); | |
| mockPost.mockRejectedValueOnce(new Error('fail')); | |
| mockPost.mockResolvedValueOnce({ status: 200 }); | |
| await sendWebhook(event); | |
| await jest.advanceTimersByTimeAsync(1000); | |
| const allPayloads = mockPost.mock.calls.map(([, p]) => p); | |
| for (const payload of allPayloads) { | |
| expect(payload.event_type).toBe(event.type); | |
| expect(payload.contract_id).toBe(event.contractId); | |
| } | |
| it('each retry receives the same event payload', async () => { | |
| const event = makeEvent('evt-retry-check'); | |
| mockPost.mockRejectedValueOnce(new Error('fail')); | |
| mockPost.mockResolvedValueOnce({ status: 200 }); | |
| await sendWebhook(event); | |
| await jest.advanceTimersByTimeAsync(1000); | |
| const allPayloads = mockPost.mock.calls.map(([, p]) => p); | |
| for (const payload of allPayloads) { | |
| expect(payload.event_type).toBe(event.type); | |
| expect(payload.contract_id).toBe(event.contractId); | |
| expect(payload.topic).toEqual(event.topic); | |
| expect(payload.value).toEqual(event.value); | |
| } |
🤖 Prompt for 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.
In `@services/notifier/notifier.test.ts` around lines 122 - 134, The retry test in
sendWebhook only asserts event_type and contract_id, so it can miss changes to
other fields. Update the each retry receives the same event payload test in
notifier.test.ts to compare the full payload object for every mockPost call,
including topic and value, using the existing event from makeEvent and the
sendWebhook retry flow.
2 participants
Summary
Off-Chain Notification Event Webhooks via Indexer #297 Notifier webhook tests — Adds
services/notifier/notifier.test.tswith 6 unit tests covering: successful delivery payload shape, no retry on success, 1-second delay after first failure, exponential backoff schedule (2^n × 1000 ms), stops after 5 retries with no 6th call, and consistent payload across retries. Addsjest/ts-jestto devDependencies with atestscript.Dynamic Surge Pricing for High-Demand Expert Categories #296 Dynamic surge pricing — The full implementation exists upstream in
contracts/src/lib.rsandcontracts/src/reputation.rs: active-session tracking per category, 1.0×–2.0× dynamic multiplier calculation, and explicit seeker approval viastart_session_with_surge. No duplicate code needed.closes #296
closes #297
Summary by CodeRabbit
Tests
Chores