← Back to PRs

#19284: fix(delivery): treat AbortErrors as failures for retry

by EdGuan open 2026-02-17 16:29 View on GitHub →
size: XS
## Summary When an `AbortError` occurs during message delivery (e.g., network timeout mid-send), the queue entry was being acknowledged as successful, causing the message to be **silently dropped without retry**. ## Problem From the logs in #18574: ``` [telegram] network error: Request to 'getUpdates' timed out after 500 seconds [openclaw] Suppressed AbortError: This operation was aborted ``` The current code in `deliver.ts` treats `AbortError` specially: ```typescript if (isAbortError(err)) { await ackDelivery(queueId); // ← Marks as SUCCESS! } ``` This means: 1. Network timeout causes AbortError 2. Delivery is acked (removed from queue) 3. Message is lost forever — no retry, no error logged ## Fix Treat `AbortError` the same as other errors: call `failDelivery()` so the message stays in the queue for retry on gateway restart. ```typescript // AbortErrors (e.g., network timeouts) should NOT ack the delivery. // Treat them as failures so the message stays in the queue for retry. await failDelivery(queueId, err.message); ``` ## Test Changes Updated the existing test to expect `failDelivery` instead of `ackDelivery` on abort, with a comment explaining the reasoning. ## Related Fixes #18574 <!-- greptile_comment --> <h3>Greptile Summary</h3> Fixes a bug where `AbortError` during message delivery (e.g., network timeout mid-send) was incorrectly acknowledged as a successful delivery via `ackDelivery()`, causing the message to be silently dropped from the queue with no retry. The fix removes the special-case `isAbortError` branch and treats all errors uniformly by calling `failDelivery()`, which keeps the message in the write-ahead queue for retry on gateway restart. - Removed the `isAbortError` helper (local to `deliver.ts`, unused elsewhere) and its special-case `ackDelivery` call in the catch block - All errors in the catch block now uniformly call `failDelivery()` with the error message, which is consistent with how `bestEffort` partial failures and recovery retries already work - Updated the abort test to assert `failDelivery` is called instead of `ackDelivery`, with clear comments explaining the rationale <h3>Confidence Score: 5/5</h3> - This PR is safe to merge — it fixes a clear bug with a minimal, well-tested change. - The change is a straightforward bug fix that removes a single erroneous branch (`isAbortError` → `ackDelivery`) and lets all errors fall through to the existing `failDelivery` path. The removed `isAbortError` helper was local to this file with no external consumers. The fix aligns with how the delivery queue already handles errors in other paths (bestEffort partial failures, recovery retries). The test correctly validates the new behavior. - No files require special attention. <sub>Last reviewed commit: 19396c4</sub> <!-- greptile_other_comments_section --> <sub>(2/5) Greptile learns from your feedback when you react with thumbs up/down!</sub> <!-- /greptile_comment -->

Most Similar PRs