Add webhook sender, fix analytics eviction drift, add batch-completed event #227
Merged
Abd-Standard merged 2 commits intoJun 27, 2026
Merged
Conversation
|
@amanosiadnan-cmyk 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! 🚀 |
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 #2
closes #62
closes #67
closes #182
PR Description
Summary
Implements a reusable webhook sender for POST/JSON delivery with configurable timeouts and graceful failure handling.
Fixes analytics counter drift by adjusting internal counters when old records are evicted from the rolling buffer; adds a regression test.
Adds a contract-side BatchProcessingCompleted event and exposes emit_batch_completed() for off-chain callers to reliably signal batch completion. Adds a unit test validating event emission.
What I changed
Added webhook sender utility
File: webhook-sender.ts
Exposes sendWebhook(url, payload, { timeoutMs, headers }) — POSTs JSON, configurable timeout via AbortController, returns the Response.
Integrated webhook sender into Discord notification flow
File: discord-notification.ts
Replaced the previous local fetch logic with the shared sendWebhook call; preserves timeout/error logging and analytics outcomes.
Fixed analytics aggregator eviction counter drift
File: notification-analytics-aggregator.ts
When rotating/evicting old records due to maxRecords, we now decrement the aggregated counters (successCount, failureCount, retryCount, skippedCount) and adjust totalDurationMs / durationSamples so aggregate counters remain consistent with retained records.
Added regression test to guard against drift
File: notification-analytics-aggregator.test.ts
New test asserts snapshot totals reflect only visible records after eviction (no drift).
Contract: added batch completion event & API
File: events.rs
Added BatchProcessingCompleted event (topics: batch_id, category, priority; data: processed_count).
File: autoshare_logic.rs
Added emit_batch_completed(env, batch_id, processed_count) which publishes BatchProcessingCompleted.
File: lib.rs
Exposed emit_batch_completed() as a public contract method.
File: batch_event_test.rs
Unit test verifying the event is emitted, topics contain the batch_id, and data contains processed_count.
Acceptance criteria mapping
Webhooks are successfully delivered.
Implemented generic sendWebhook() and integrated it into DiscordNotificationService which already handles response OK/failure and deduplication.
Failed requests are logged.
DiscordNotificationService continues to log failures/timeouts; webhook utility raises on abort so the caller logs and records analytics failures.
Timeout configuration is supported.
sendWebhook() accepts timeoutMs; DiscordNotificationService passes configured timeout.
Analytics metrics remain consistent with source records.
Eviction-based counter adjustments ensure no drift between retained records and aggregate counters.
Historical recalculations & regression tests
Snapshot logic unchanged (still computes aggregates from visible records), and added test to prevent future regressions.
Contract: Batch completion events
BatchProcessingCompleted defined and published via emit_batch_completed(); test validates emission and values.
Implementation notes & rationale
Webhook sender:
Small, dependency-free util using Fetch + AbortController to centralize timeout semantics and header handling for any webhook integrations.
Analytics aggregator:
The bug came from evicting records but not decrementing aggregate counters; fixing eviction-side adjustments keeps lifetimeCount semantics (ever seen) while making current aggregates consistent.
Snapshot computation still derives results from visible records — counters kept in sync help other uses and internal metrics (and avoid slow O(N) recomputations in hot paths).
Contract change:
Emitting BatchProcessingCompleted from the contract is provided as a permissionless helper method callable by off-chain processors after successful batches. This avoids coupling batch processing logic into the contract storage/locking and keeps contract surface minimal.
Tests & verification
Added unit tests:
Listener: notification-analytics-aggregator.test.ts (regression for eviction/double-check).
Contract: batch_event_test.rs (event emission shape and data).
NOTE: I could not run tests in this environment:
Listener tests require Node modules; run locally with: