Skip to content

fix(workers): make delivery recording recoverable when contract call fails (#507) - #539

Open
payfoxX wants to merge 9 commits into
JSE-ORG:devfrom
payfoxX:fix/507-delivery-claim-pattern
Open

fix(workers): make delivery recording recoverable when contract call fails (#507)#539
payfoxX wants to merge 9 commits into
JSE-ORG:devfrom
payfoxX:fix/507-delivery-claim-pattern

Conversation

@payfoxX

@payfoxX payfoxX commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Closes #507

Description

Fixes a bug where a failed contract delivery recording call leaves the escrow permanently in DELIVERED state with no retry possible.

Root Cause

TrackingPollWorker.run was calling markDelivered() (which sets state to DELIVERED in the database) BEFORE contractService.recordDelivery(). If recordDelivery threw, the escrow was already marked DELIVERED and findShippedWithTracking() (which filters by state: SHIPPED) would never return it again.

Fix

Follows the same claim-and-release pattern used by AutoReleaseWorker:

  1. claimDelivery(escrowId) – atomically claims the escrow by setting deliveryRecordedAt (no state change)
  2. contractService.recordDelivery() – the contract call
  3. On success: markDelivered() – transitions to DELIVERED
  4. On failure: clearDeliveryClaim() – clears deliveryRecordedAt so the next poll cycle retries

findShippedWithTracking() now also filters deliveryRecordedAt: null so claimed-but-not-yet-delivered escrows are excluded from the next cycle.

Changes

  • EscrowRepository: Added claimDelivery() and clearDeliveryClaim() methods; modified findShippedWithTracking() to exclude claimed escrows
  • TrackingPollWorker: Uses claim-and-release pattern with proper cleanup on failure
  • PrismaService (in-memory mock): Added deliveryRecordedAt null filter support

Testing

  • Existing tests updated: mock now includes claimDelivery/clearDeliveryClaim
  • New test asserts that a failed recordDelivery clears the claim and does NOT mark the escrow as delivered, leaving it retryable on the next cycle

@payfoxX
payfoxX requested a review from Omoboi-dev as a code owner July 29, 2026 09:46
@payfoxX
payfoxX force-pushed the fix/507-delivery-claim-pattern branch from 199d3f8 to 8c30b63 Compare July 29, 2026 09:53
payfoxX added 4 commits July 29, 2026 10:06
- api-keys.controller.spec.ts: mock guards with overrideProvider to fix
  401 vs 403 mismatch; use Bearer prefix to distinguish admin/non-admin
- prisma.service.spec.ts: wrap sync-throwing create in async function
  so .rejects matcher works instead of timing out
- tracking-poll.worker.intervals.spec.ts: add claimDelivery and
  clearDeliveryClaim to escrowRepository mock
Use jest.requireActual instead of dynamic import for
credential-encryption.util to avoid ESM module errors.
@payfoxX
payfoxX force-pushed the fix/507-delivery-claim-pattern branch from 9431447 to f69ab29 Compare July 29, 2026 10:37
@payfoxX
payfoxX force-pushed the fix/507-delivery-claim-pattern branch from dd4cdb2 to 570a4ef Compare July 29, 2026 10:47
@payfoxX
payfoxX force-pushed the fix/507-delivery-claim-pattern branch from 10ba682 to dde1fbf Compare July 29, 2026 11:01
payfoxX added 2 commits July 29, 2026 11:14
…irectly

NestJS @UseGuards resolves classes directly regardless of mock
providers, so the module-based approach could never work.
Test the controller business logic directly instead.
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.

Delivery is recorded in the database before the contract call, and a failure loses the escrow

1 participant