Skip to content

fix(notifications): add stable Idempotency-Key header to outbound web… - #101

Open
udeachudivine-spec wants to merge 1 commit into
ecotask-network:mainfrom
udeachudivine-spec:fix/webhook-idempotency-key-59
Open

fix(notifications): add stable Idempotency-Key header to outbound web…#101
udeachudivine-spec wants to merge 1 commit into
ecotask-network:mainfrom
udeachudivine-spec:fix/webhook-idempotency-key-59

Conversation

@udeachudivine-spec

Copy link
Copy Markdown

…hook dispatch (#59)

  • Export buildWebhookIdempotencyKey() helper that derives a SHA-256 hex digest of outboxId:webhookUrl — stable across BullMQ retries of the same job, unique per outbox row and per subscriber URL
  • Add Idempotency-Key header alongside Content-Type on every outbound webhook POST in sendWebhook()
  • Thread outboxId through dispatchNotification(notificationId, outboxId?) and the BullMQ worker processor so the stable outbox row ID drives the key
  • Falls back to notificationId as key base when outboxId is not supplied (direct/admin calls) to avoid any random/time-based value
  • Add tests/services/notificationDispatchService.test.ts covering: key determinism, subscriber-uniqueness, retry-stability, header presence, Content-Type co-existence, and already-delivered short-circuit path

NOTE: the current NotificationOutbox schema has no delivery-generation column, so deliberate re-sends (DEAD_LETTER reset) after a permanent failure will reuse the same key as the original delivery's retries. A follow-up is needed: add deliveryGeneration Int @default(0) to NotificationOutbox and incorporate it into the key construction.

Description

Please include a summary of the change and which issue is fixed.

Fixes #(issue)

Type of Change

  • Bug fix (non-breaking change fixing an issue)
  • New feature (non-breaking change adding functionality)
  • Breaking change (fix or feature that breaks existing behaviour)
  • Refactor (code change that neither fixes a bug nor adds a feature)
  • Test (adding or updating tests)
  • Docs (documentation-only changes)
  • Chore (tooling, CI/CD, build process)

How Has This Been Tested?

  • npm test passes
  • npm run build succeeds
  • Manual testing steps described below

Checklist

  • My code follows the project's coding conventions
  • I have added tests that prove my fix/feature works
  • New and existing tests pass locally
  • I have updated the documentation if needed
  • My changes generate no new warnings or errors

Additional Context

Add any other context about the PR here.

…hook dispatch (ecotask-network#59)

- Export buildWebhookIdempotencyKey() helper that derives a SHA-256 hex
  digest of outboxId:webhookUrl — stable across BullMQ retries of the
  same job, unique per outbox row and per subscriber URL
- Add Idempotency-Key header alongside Content-Type on every outbound
  webhook POST in sendWebhook()
- Thread outboxId through dispatchNotification(notificationId, outboxId?)
  and the BullMQ worker processor so the stable outbox row ID drives the key
- Falls back to notificationId as key base when outboxId is not supplied
  (direct/admin calls) to avoid any random/time-based value
- Add tests/services/notificationDispatchService.test.ts covering:
  key determinism, subscriber-uniqueness, retry-stability, header presence,
  Content-Type co-existence, and already-delivered short-circuit path

NOTE: the current NotificationOutbox schema has no delivery-generation
column, so deliberate re-sends (DEAD_LETTER reset) after a permanent
failure will reuse the same key as the original delivery's retries.
A follow-up is needed: add deliveryGeneration Int @default(0) to
NotificationOutbox and incorporate it into the key construction.
@udeachudivine-spec

Copy link
Copy Markdown
Author

close issue #59

@cybermax4200 cybermax4200 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix workflow

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants