Skip to content

Celery: RetryableError with string retry_after crashes the retry path — run stuck suspended, singleton lock leaks #554

Description

@LivinTribunal

Severity: P1 — the documented retry feature is broken on Celery runtime whenever retry_after is a string (as the shipped example uses).

What happens

A step raising RetryableError(..., retry_after="5s") never retries. The run gets stuck in suspended forever, and the singleton step lock leaks until TTL (~1 h).

Root cause

pyworkflow/celery/tasks.py:424-431:

countdown = e.retry_after          # raw attribute — may be the string "5s"
...
f"Step failed (retriable): {step_name}, retrying in {countdown:.1f}s...",

RetryableError.__init__ stores retry_after unconverted (core/exceptions.py:116) and the converter get_retry_delay_seconds() (core/exceptions.py:119) is never called at this use site. With a string, the f-string raises ValueError: Unknown format code 'f' for object of type 'str' before self.retry() is reached. The task dies via on_failure instead of on_retry, the step failure is never recorded to the event log, so the workflow stays suspended indefinitely.

Lock leak: SingletonWorkflowTask.on_failure (celery/singleton.py:471-492) only releases when release_lock_on_failure (False for execute_step_task) or max_retries_exceeded (retries=0 < 3 → False), so the pyworkflow:lock:* key survives to TTL.

The shipped example triggers this: examples/celery/durable/workflows/retries.py:45 raises RetryableError(..., retry_after="5s").

Executed evidence (v0.3.8, isolated redis, file storage, --pool solo)

Controlled A/B, string vs int the only variable:

leg outcome
retry_after="5s" run suspended at 15/30/62 s snapshots; step attempted once; no "retrying in" log; leaked pyworkflow:lock:5e327… (ttl 3535 s)
retry_after=5 (int) execute_step_task:429 … retrying in 5.0s...; attempt count 2; run completed in ~5 s; no leaked lock

Direct execution of the exact branch: f"...{'5s':.1f}..."ValueError before self.retry().

Suggested fix

Use countdown = e.get_retry_delay_seconds() (already exists) instead of the raw attribute, and consider releasing the singleton lock in on_failure for non-retried failures.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions