fix(basket): preserve retries after interrupted admission - #586
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
📄 Knowledge reviewDosu skipped reviewing this PR because your organization has used its |
Greptile SummaryThis PR centralizes Basket's retryable delivery error, changes unexpected batch-preparation failures to return 503 before publishing, and reduces pending Redis dedup reservations from 120 to 30 seconds.
Confidence Score: 4/5The pending lease race should be fixed before merging because a slow admission followed by a retry can publish the same event twice. The new 30-second Redis TTL is not coupled to or renewed for the full reservation-to-commit interval, so ownership can expire while enrichment or delivery remains active and a second owner can publish the same event. Files Needing Attention: apps/basket/src/lib/security.ts and apps/basket/src/lib/event-service.ts Important Files Changed
Sequence DiagramsequenceDiagram
participant A as Original admission
participant R as Redis dedup
participant P as Producer
participant B as Client retry
A->>R: Reserve event ID (30s pending lease)
A->>A: Enrich and prepare event
A->>P: Begin delivery
R-->>R: Pending lease expires
B->>R: Reserve same event ID
B->>P: Deliver retry
P-->>B: Retry published
P-->>A: Original published
A->>R: Mark delivered with old token
R-->>A: Token mismatch
Note over P: Same event may be published twice
Reviews (1): Last reviewed commit: "fix(basket): preserve retries after inte..." | Re-trigger Greptile |
| // Pending ownership only guards an in-flight admission. It must outlive the | ||
| // 20-second Railway shutdown budget while still letting a crashed owner expire | ||
| // before client retries are suppressed for minutes. | ||
| const PENDING_DEDUP_TTL = 30; |
There was a problem hiding this comment.
Pending lease expires during admission
If enrichment or producer delivery remains in flight for more than 30 seconds, the pending Redis key expires before the original owner marks it delivered. A retry can then reserve the same event ID and publish it again, causing duplicate analytics rows while the original owner's token no longer matches.
Knowledge Base Used: Basket Ingestion Flow
|
The latest updates on your projects. Learn more about Unkey Deploy
|
Summary
/batchitem-preparation failure occurs, before any batch is acknowledged.Why
A process crash or failed release could leave a normal pending reservation for two minutes and suppress client retries. Separately, a transient preparation error could yield an HTTP 200 partial response even though the event was never admitted. Both behaviors risked silent event loss.
Validation
cd apps/basket && bun run test— 598 passedbun run check-types --filter=@databuddy/basketbun run lintbun run formatNo migrations or PostgreSQL delivery state are included.
Summary by cubic
Prevents silent event loss by preserving client retries after interrupted admission, reserving events only at delivery handoff, and returning a structured 503 when preparation fails to avoid partial acknowledgements.
deliveryUnavailableand returned a retryable 503 for unexpected/batchpreparation failures before any item is acknowledged.Written for commit 49ca094. Summary will update on new commits.