Repository navigation
fix: report all dependency failures instead of only the first - #3074
intertermux-code wants to merge 2 commits into
Conversation
When dependencies run in parallel without failfast, every dep runs to completion, but errgroup.Wait only surfaced the first error - the rest were silently dropped. Collect each dep's failure and return them joined so a single run shows everything that went wrong.
|
Bumping this one — it's been a week, happy to rework if the design needs it. |
vmaerten
left a comment
vmaerten
left a comment
There was a problem hiding this comment.
Thanks !
The approach is right, one thing to fix before merging: the order of the joined errors currently depends on which dep finishes first, and #3050 explicitly asks for a deterministic message.
The suggestions below collect the errors by index instead, so they always come out in declaration order.
They depend on each other, so please apply them as a single batch, and add the small Join wrapper to errors/errors.go.
With the order fixed, the test could also assert the exact message instead of two Contains.
| if failfast { | ||
| return err | ||
| } | ||
| return stderrors.Join(errs...) |
There was a problem hiding this comment.
errors as you aliased to stderrors cannot be imported.
Please use github.com/go-task/task/v3/errors
| if !failfast && err != nil { | ||
| mu.Lock() | ||
| errs = append(errs, err) | ||
| mu.Unlock() | ||
| } |
There was a problem hiding this comment.
| if !failfast && err != nil { | |
| mu.Lock() | |
| errs = append(errs, err) | |
| mu.Unlock() | |
| } | |
| errs[i] = err |
Collect parallel dep errors by index instead of completion order, so the joined message is deterministic. Use the repo's errors package (with a new Join wrapper) instead of stdlib errors, which depguard forbids. Strengthen the test to assert the exact joined message.
When dependencies run in parallel without failfast, every dependency runs to completion, but errgroup.Wait() only surfaces the first error — any other failures were silently dropped, leaving the user with no idea that more than one dep failed.
Each goroutine now records its error under a mutex, and runDeps returns errors.Join(errs...) so a single run shows every failing dependency. Failfast behaviour is unchanged: it still cancels and returns the first error.
Added TestParallelDepsReportAllFailures with a testdata/deps_fail Taskfile containing two failing deps; it fails without the fix (only one failure surfaced) and passes with it. Full go test ./... is green.
Fixes #3050.