Skip to content

fix(catch_error): re-stamp on_failure message so a late callback is not age-dropped - #405

Open
alienard-wiremind wants to merge 1 commit into
masterfrom
fix/catch-error/restamp-on-failure-message
Open

fix(catch_error): re-stamp on_failure message so a late callback is not age-dropped#405
alienard-wiremind wants to merge 1 commit into
masterfrom
fix/catch-error/restamp-on-failure-message

Conversation

@alienard-wiremind

Copy link
Copy Markdown
Contributor

Problem

CatchError.after_process_message re-enqueues the on_failure message from a dict re-materialised via Message(**on_failure).copy(...). copy (attr.evolve + option-merge) preserves the original message_timestamp and message_id — the ones from when the on_failure message was built, which can be long before the failure it reports.

When the on_failure message carries its own max_age (a common pattern for batch callbacks) and is attached to a message that retried or aged in a delay queue for hours, the re-enqueued callback is born already past its age limit. AgeLimit.before_process_message then drops it at consume time and the callback never runs — silently.

This was hit in production: a batch's ~884 hard failures escalated/retried into a delay queue overnight; when they finally failed terminally the next morning, every on_failure callback was enqueued with its ~13–20h-old build timestamp and immediately age-dropped into the dead-letter queue, so the downstream system was never notified.

Fix

Re-stamp the re-enqueued on_failure message with a fresh message_id and message_timestamp so it is treated as freshly enqueued. This matches the string-actor-name on_failure path, which already builds a fresh message via actor.send. The callback is a leaf message (no pipe_target), so a fresh id is safe.

Test

test_on_failure_stale_message_is_restamped reproduces the bug: an on_failure message with a small max_age and a backdated message_timestamp is dropped by AgeLimit before the fix and runs after it. Verified it fails on the pre-fix code and passes on the fixed code; full test_catch_error.py suite green; ruff + mypy clean.

Notes

  • Changelog: added an Unreleased / Fixed entry — please assign a version at tag time.
  • The success-path pipe_target message (Pipelines) re-materialises its dict with the original timestamp too; the same staleness can affect long-backlogged pipelines. Not addressed here — flagging for a possible follow-up.

…ot age-dropped

CatchError re-enqueues the on_failure message from a dict re-materialised via
Message(**on_failure).copy(...), which preserved the original message_timestamp
(and message_id). When the on_failure message carries a max_age and is attached
to a message that retried or aged for a long time, it was born already expired,
so AgeLimit dropped it at consume time and the callback never ran.

Re-stamp the re-enqueued message with a fresh message_id and message_timestamp
so it is treated as freshly enqueued, matching the string-actor-name on_failure
path (which builds a fresh message via actor.send).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@alienard-wiremind

Copy link
Copy Markdown
Contributor Author

T'en penses quoi @thomasLeMeur ?

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