Fix OnMessageDeliveryStarting godoc: hooks stop on first error - #1152
PratikDhanave (PratikDhanave) wants to merge 1 commit into
Conversation
Composed OnMessageDeliveryStarting hooks are chained by extendContextHook, which returns immediately when the first hook errors without running the second. The doc said it "invokes all" hooks; unlike OnMessageDeliveryFinished (extendFinishedHook), it does not. Clarify the short-circuit behavior.
github-actions
Bot
added
area:workflow
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The comment-only change accurately matches the existing hook composition behavior.
Review effort: Lite
Findings: None
What changed in this PR
Clarifies that OnMessageDeliveryStarting hooks execute in order and stop when the first hook returns an error.
Changes:
- Updated the method godoc to accurately describe short-circuit behavior.
| File | Description |
|---|---|
workflow/executor.go |
Corrects OnMessageDeliveryStarting documentation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Scope: internal-only (documentation/comment fix) This PR only clarifies wording in an existing godoc comment; it does not add, remove, or change any public API or observable runtime behavior, so no cross-repo parity review is warranted.
|
The comment says OnMessageDeliveryStarting "invokes all configured OnMessageDeliveryStarting hooks," but the composed hook is built by
extendContextHook, which runs the first hook and, on its error, returns immediately without running the second. This contrasts withOnMessageDeliveryFinished(built byextendFinishedHook), whose doc correctly says "All hooks are run." Clarify the short-circuit behavior. Comment-only fix.