[v2-engine errors] Sort suggestions for typo'd targets. dedup errors w/o trace #5413
Conversation
…hen trace is disabled
| raise throw_root_states[0].exc | ||
| else: | ||
| raise ExecutionError('Multiple exceptions encountered:\n {}' | ||
| .format('\n '.join('{}: {}'.format(type(t.exc).__name__, str(t.exc)) | ||
| for t in throw_root_states))) | ||
| .format('\n '.join('{}: {}'.format(type(t).__name__, str(t)) |
There was a problem hiding this comment.
nit: This makes the order of exception reports non-deterministic. That's probably fine (and I expect the engine probably reports them in a non-deterministic order, too), but it may be worth preserving their reported order. Probably not, though :)
There was a problem hiding this comment.
That's fair. I feel like what we need is to have a better approach to reporting errors in general, but I'm still not sure what that looks like. That's why there's a TODO above and #3912. The specific issues in #3912 are better, but not yet good.
The errors appear to be deterministic when you look at them with the full trace, so at least theres that.
There was a problem hiding this comment.
could you just throw in a sorted(unique_exceptions) here, maybe?
| raise throw_root_states[0].exc | ||
| else: | ||
| raise ExecutionError('Multiple exceptions encountered:\n {}' | ||
| .format('\n '.join('{}: {}'.format(type(t.exc).__name__, str(t.exc)) | ||
| for t in throw_root_states))) | ||
| .format('\n '.join('{}: {}'.format(type(t).__name__, str(t)) |
There was a problem hiding this comment.
could you just throw in a sorted(unique_exceptions) here, maybe?
|
@kwlzn I probably could. Missed your comment before I merged though. |
Problem
When there are a large number of suggestions, the lack of alphanumeric sorting is frustrating. #4788
Solution
Sort the suggestions. Additionally, I noticed that because of where this happens, there may be multiple copies of the error message that show up. I changed the default behavior to dedup common errors.
Result
Instead of