Format python traceback in impl Debug for PyErr.#4900
Conversation
48a7bf4 to
a9ec2c0
Compare
Icxolu
left a comment
There was a problem hiding this comment.
Thanks! This looks reasonable to me. Just a quick thought before landing this: Does it make sense to keep the inner Result here, or should we flatten it into the outer Option for simplicity?
|
I guess if it errors we would try to render the error and maybe get into an infinite descent? I suggest we flatten and use logic similar to the |
|
Sorry, I meant Line 484 in 2c732a7 |
a9ec2c0 to
f6e8e0d
Compare
|
Done. I thought about using tb.format().map_err() instead of the match but the match is more readable IMO. This gives output like Is there a good way to test this? |
CodSpeed Performance ReportMerging #4900 will not alter performanceComparing Summary
|
| Err(err) => { | ||
| err.write_unraisable(py, Some(&tb)); | ||
| format!("<unformattable {:?}>", tb) | ||
| } |
There was a problem hiding this comment.
Can we construct an unraisable traceback in a test to have coverage here?
There was a problem hiding this comment.
Don't know, that's what I meant in my last reply with "Is there a good way to test this?". If anyone here knows how I can construct an unformattable traceback, I'm happy to add a test for that.
There was a problem hiding this comment.
I could do something like mock out io.StringIO in the python, that should cause the format to fail, not sure if we want to go down that route.
There was a problem hiding this comment.
Added a test with that approach.
f6e8e0d to
d3c1782
Compare
d3c1782 to
9546f5f
Compare
Icxolu
left a comment
There was a problem hiding this comment.
Looks good to me now, thank you very much!
Fixes #4863.