Repository navigation
fix(engine): stop leaking errors from the operator - #5212
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
 [Medium impact] Changes error messages returned to clients from the orchestration engine. The PR appears safe to merge, with a non-blocking gap in tests for DAG error messages. SummaryThe PR keeps operator details out of task errors.
The main improvement needed is direct regression coverage for the new DAG message rules. Reviews (1) · Last reviewed commit: "fix: stop leaking errors over the operat..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| func clientErrorMessageForDagErr(err error) (message string, ok bool) { | ||
| var childFailedErr *dagChildFailedError | ||
| var cancelledErr *dagCancelledError | ||
| var conditionEvalErr *dagConditionEvalError | ||
| var celFailureErr *operator.DAGStepCELFailureError | ||
|
|
||
| return isDagChildFailedErr(err) || | ||
| isDagConditionEvalErr(err) || | ||
| isDagEngineErr(err) || | ||
| errors.As(err, &celFailureErr) || | ||
| errors.As(err, &nonDeterminismErr) | ||
| var nonDeterminismErr *repository.NonDeterminismError | ||
| var engineErr *dagEngineError |
There was a problem hiding this comment.
clientErrorMessageForDagErr changes which errors reach users, but no DAG tests cover its new behavior. Add table-driven tests for wrapped user-facing errors, wrapped internal errors, and both engine error types. Check that internal wrapper text stays hidden, nondeterminism details remain visible, and unspecified engine errors use the generic message. Without these checks, later changes could expose internal details or hide useful errors unnoticed. The updated host tests only cover refused actions.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
This branch was successfully deployed
2 participants
Description
We keep leaking errors from the operator into task run errors, so this should help with that by just more explicitly handling more cases of errors
Type of change
Checklist
Changes have been:
🤖 AI Disclosure