sdk-python: Deserialize traceback from stack trace string in Temporal failures if able
Is your feature request related to a problem? Please describe.
We chain errors when converting from failures by setting __cause__, but there is a report that the chained errors are not logged like normally chained errors
EDIT: We don’t rehydrate the traceback from the stack trace string
Describe the solution you’d like
Make sure we log chained errors normally and write a test to ensure it
EDIT: Java parses their string stack trace back to stack trace elements, so we should too. See https://github.com/ionelmc/python-tblib/blob/dd926c1e5dc5bbe5e1fc494443bbac8970c7d3ee/src/tblib/__init__.py#L200 for an example of how to do this.
About this issue
- Original URL
- State: closed
- Created 2 years ago
- Comments: 27 (27 by maintainers)
I have opened #75. In addition to other things we have suggested, it contains a test that appends the stack to all Temporal failure errors. Basically you can use this helper:
I am hesitant to add it as a supported public utility at the moment (it’s very simple, only a couple of lines if the formatter didn’t break it out).
Since we cannot put the traceback back on the error and we don’t want stack to be on every string representation of the error, opt-in on the user part is the only way.
I researched custom logging formatters, adapters, etc and it boiled down to just being easier to alter the exceptions in the chain than altering the logging calls. The
traceback.format_exceptionand similar calls by logging and others handle the chain for you, so you can’t add the stack after the fact there. And I don’t want to recreate my own chaining string formatter because I want to reuse Python’s. I would have preferred shallow copying all the exceptions (e.g.copy.copy()) and only adding stack to the shallow copies, but that had problems maintaining the chain too. Same with customizing log record and other approaches.So basically, altering the exception is easiest. I chose to put the stack after the message instead of before because the internal Python exception formatter always put’s the exception class name first which means I can’t inject anything before that.
Using that helper above, given the following:
Once serialized and sent back from Temporal, after running through
append_temporal_stack, your output might look like:Which I think is about the best we can do.
I guess I personally don’t care so much that the stack traces I see are client side vs. just seeing stack traces by default when testing, but I can see how it’s important to have both. Well actually, it might get a little weird if we have both (duplicate output for testing case) but it’s important to at least have both options!
You and the rest of the team are doing such a great job of nailing all the details, I really enjoy following along. Thank you for engaging with my questions/issues.
Java’s stack trace element string format is a bit more stable from version to version.
Regardless, yeah we can do this. I think we’d just take
tblib.Traceback.from_stringwhich seems to have an expectation on Python traceback string format. So maybe it is more stable than I realize. And of course we’d have some tests to ensure it continues to work. I will change the title of this issue to add this functionality. This shouldn’t be hard to implement, I’ll probably get to it before next release.