fix(workflow): target the -main run in Wait and AbortRun (EN-1220) - #183
fix(workflow): target the -main run in Wait and AbortRun (EN-1220)#183flemzord wants to merge 1 commit into
Conversation
…andling
Initiate starts the real Run workflow as a detached child with id
"<instanceID>-main" (ParentClosePolicy ABANDON) and returns as soon as
that child has started. The Initiate workflow (id == instanceID) is thus
already completed by the time the API calls Wait/AbortRun:
- AbortRun cancelled the completed Initiate execution, so cancellation
never reached the running stages (wait_event/delay were unabortable).
- Wait returned immediately on the completed Initiate execution, so
?wait=true returned a non-terminated instance.
Both now target "<instanceID>-main" (matching ReadInstanceHistory).
Wait also mishandled errors: errors.Is(err, &serviceerror.NotFound{})
can never match (no Is/Unwrap on that type) and errors.Unwrap(err)
returned nil for non-wrapped errors, turning a failure into a success.
Use errors.As for NotFound and return the original error otherwise.
Adds TestWait covering both the terminate-wait and not-found paths.
|
Warning Review limit reached
More reviews will be available in 41 minutes. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
flemzord
changed the title
|
Superseded by #199, which consolidates this change with the related reliability and safety fixes on top of the current main branch. |
1 participant
Problem (C2 + C3 — CRITICAL)
Initiatestarts the realRunworkflow as a detached child (ParentClosePolicy: ABANDON, id<instanceID>-main) and returns as soon as the child has started (run.go:53-64). TheInitiateworkflow (id ==instanceID) is therefore already completed by the time the API callsWait/AbortRun.C2 — wrong target:
AbortRuncalledCancelWorkflow(ctx, instanceID, "")on the completedInitiateexecution → cancellation never reaches the running stages.wait_event/delayinstances were unabortable.WaitcalledGetWorkflow(ctx, instanceID, "").Get()on the completedInitiateexecution → returns immediately, soPOST /workflows/{id}/instances?wait=truereturned a non-terminated instance.C3 — error handling in
Wait:Fix
WaitandAbortRunnow target<instanceID>-main(matching whatReadInstanceHistoryalready does).Waituseserrors.As(err, ¬Found)for the NotFound case and returns the original error otherwise.Test
TestWait(integration): afterWaitreturns, the instance is terminated (would be flaky/false under the old behavior); an unknown instance returnsErrInstanceNotFound.Severity: CRITICAL.