Skip to content

Comments

Proposal: Add error field to StageRetrying event (Section 9.6)#4

Open
samueljklee wants to merge 1 commit intostrongdm:mainfrom
samueljklee:feat/stage-retrying-error-field
Open

Proposal: Add error field to StageRetrying event (Section 9.6)#4
samueljklee wants to merge 1 commit intostrongdm:mainfrom
samueljklee:feat/stage-retrying-error-field

Conversation

@samueljklee
Copy link

Summary

Add an error field to the StageRetrying event type in Section 9.6, so consumers can see why a stage is retrying.

Motivation

While implementing the Section 9.6 event system, a user reported their pipeline appearing to hang (samueljklee/attractor#36). After adding --verbose output wired to the event system, the user could see stages retrying but not why:

[event] Stage 'scaffold_service' [1] retrying (attempt 1, delay 0.0s)
[event] Stage 'scaffold_service' [2] retrying (attempt 2, delay 0.0s)

Their feedback: "It would be informative if it also showed me why it is retrying."

The gap

The Outcome model (Section 5.2, line 1086) defines failure_reason for both FAIL and RETRY statuses:

failure_reason : String -- reason for failure (when status is FAIL or RETRY)

StageFailed (line 1622) already surfaces this as error:

- `StageFailed(name, index, error, will_retry)` -- stage failed

But StageRetrying (line 1623) does not, even though failure_reason is available at the retry point.

Proposed change

- - `StageRetrying(name, index, attempt, delay)` -- stage retrying
+ - `StageRetrying(name, index, attempt, delay, error)` -- stage retrying

With this change, verbose output becomes:

[event] Stage 'scaffold_service' retrying (attempt 1, delay 0.0s): Invalid API key

Notes

  • Additive and non-breaking (existing consumers that don't use the field are unaffected)
  • Consistent with StageFailed which already has error
  • Outcome.failure_reason is already populated for RETRY status per Section 5.2
  • We've implemented this enhancement in our fork and confirmed it resolves the UX issue

Reference: samueljklee/attractor#36

🤖 Generated with Amplifier

Add `error` parameter to `StageRetrying(name, index, attempt, delay, error)`
so consumers can see why a stage is retrying, consistent with `StageFailed`
which already surfaces `error`.

Motivation: Outcome.failure_reason (Section 5.2) is populated for RETRY
status but StageRetrying did not expose it, making verbose output less
informative than it could be.

Reference: samueljklee/attractor#36

🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
@samueljklee
Copy link
Author

@jmccarthy Hi -- we ran into this while implementing the Section 9.6 event system. Users couldn't see why stages were retrying since StageRetrying only carries attempt and delay. The error is already available from Outcome.failure_reason at the retry point -- this just surfaces it in the event. Would love your thoughts.

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