Skip to content

Fix/on failed look for message in all queues - #18

Open
bobbybol wants to merge 5 commits into
mainfrom
fix/on-failed-look-for-message-in-all-queues
Open

bobbybol wants to merge 5 commits into
mainfrom
fix/on-failed-look-for-message-in-all-queues

Conversation

@bobbybol

@bobbybol bobbybol commented Sep 4, 2026 •

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • Bug Fixes

    • Improved PUSH ingress retry handling by determining the message’s current delivery stage before scheduling a retry.
    • Failed deliveries now transition correctly to the appropriate retry state, with outdated stage references and external-delivery tracking removed.
    • Messages that cannot be found or have an unrecognized stage are safely marked as orphaned.
    • Improved handling and reporting of connection, timeout, and aborted request failures.
  • Documentation

    • Updated the decisions log to record resolution of the parked PUSH ingress retry issue.

@coderabbitai

coderabbitai Bot commented Sep 4, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Essentials

Run ID: 8a8ed89b-bd29-428b-a6fd-b27ed2c8be02

📥 Commits

Reviewing files that changed from the base of the PR and between 869833d and 3e3a25c.

📒 Files selected for processing (6)
  • src/engine/incoming.ts
  • src/engine/lifecycle/moves.ts
  • src/lib/redis-repository/admission-store.ts
  • src/plugins/calin-api-v1/incoming.ts
  • src/plugins/calin-api-v1/lib/repo.ts
  • test/unit/plugins/calin-api-v1-repo.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/engine/incoming.ts

Included review availability: 4 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour.


📝 Walkthrough

Walkthrough

processEvent now derives retry placement from the stored message hash and lifecycle stage. PUSH callers no longer pass queue keys. CALIN transport failures use centralized classification with added timeout coverage.

Changes

PUSH ingress retry flow

Layer / File(s) Summary
Stage derivation and caller wiring
src/engine/incoming.ts, src/engine/lifecycle/actions.ts
processEvent no longer accepts a queue key. Failure handling loads the stored message, derives its lifecycle stage, and computes the plugin-specific retry key. Missing messages return orphaned.
Integration validation and decision record
test/integration/incoming-ingress.smoke.spec.ts, docs/decisions-log.md
The smoke test covers relay-node delivery failure, retry transition, stage cleanup, and external-delivery index removal. The decision log records the resolved behavior.
Lifecycle diagnostic logging
src/engine/lifecycle/moves.ts, src/lib/redis-repository/admission-store.ts, src/plugins/calin-api-v1/incoming.ts
Non-actionable claim, cleanup, and status-check events now use debug logging. Existing control flow remains unchanged.

CALIN transport error classification

Layer / File(s) Summary
Transport classification and error mapping
src/plugins/calin-api-v1/lib/repo.ts, test/unit/plugins/calin-api-v1-repo.spec.ts
Recognized connection, undici, timeout, and abort failures receive specialized messages and preserved error codes. Unit tests cover connection timeout and abort failures.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Merge Risk: ⚪ Minimal · up to 3e3a2

This change derives retry placement from stored message state and improves CALIN transport-failure mapping. The supplied coverage indicates the changed behavior is ready to merge with no current actionable risk identified.

Sequence Diagram(s)

sequenceDiagram
  participant PUSHIngress
  participant IncomingService
  participant MessageStore
  participant retryOrFail
  PUSHIngress->>IncomingService: processEvent(parsedEvent, plugin)
  IncomingService->>MessageStore: load message by hash
  MessageStore-->>IncomingService: delivery status and plugin pattern
  IncomingService->>retryOrFail: retry with derived stage key
  retryOrFail-->>PUSHIngress: retry or orphaned result
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 71.43% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 8 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title describes the main change: locating the stored message across queues after a failed delivery. The wording is awkward, but the intent is clear and directly related to the changes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/on-failed-look-for-message-in-all-queues

Comment @coderabbitai help to get the list of available commands.

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.

1 participant