fix: improve exception formatting (#146)#363
Conversation
There was a problem hiding this comment.
Code Review
This pull request improves error reporting in src/heretic/main.py and src/heretic/model.py by stripping whitespace from caught exception messages and falling back to the exception's class name if the string representation is empty. No review comments were provided, so there is no feedback to address.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
This approach has been proposed before in #183, but it doesn't really improve things. A real solution might involve walking the stacktrace to find a useful error message, but in any case I'd need to see a concrete error case to see whether it's actually an improvement or not. |
… format_exception
Good point. I've updated the PR to address this and make the error reporting a lot more useful
|
|
Can you show the output of an actual error with this mechanism? |
Yea I can show them Here are a few concrete examples of how the output looks under this mechanism compared to the previous code:
|
|
Those are inferred examples. I'm asking for the actual output of an error that really happened. Also, assuming that "RuntimeError at model.py:153" is what the mechanism might actually produce, it makes no sense. Of course the stack passed through the place where it is printed, but that doesn't tell us anything. We want to know the cause of the error. |
|
I'm quite honestly wondering if we are overthinking this. Perhaps, if there is no message, we should simply print the complete stacktrace. |
…as suggested by maintainer
Good point, printing the complete stacktrace makes a lot more sense if there's no message. I've updated the PR to do exactly that: If there's an underlying exception in the causal chain that does have a message, it still extracts that message cleanly.
|
| current = current.__cause__ or current.__context__ | ||
|
|
||
| # If there is no message in the entire causal chain, fall back to the complete traceback. | ||
| return "\n" + traceback.format_exc().strip() |
There was a problem hiding this comment.
No, the formatted exception shouldn't start with a newline as a signal. That's a semantic mismatch. The newline should be added when printing.
|
Updated the PR to return clean traceback strings from format_exception without the leading newline marker, moving the layout control entirely to the print calls. |
|
Yup, that does the trick. Merged, thanks! |
Falls back to printing the exception's class name in
main.pyandmodel.pyif the exception's string representation is empty, preventing empty parentheses likeFailed ()from being printed for exceptions without messages.Closes #146.