search

itamarst / eliot

Eliot: the logging system that tells you *why* it happened
https://eliot.readthedocs.io
Apache License 2.0
1.09k stars 65 forks source link

Capture correct tracebacks when using `inline_callbacks`. #475

Open tomprince opened 3 years ago

tomprince commented 3 years ago

Fixes #449. I'm not sure if this a great solution, but it appears to fix the issue by letting Failure's stack walking find the right context.

When an inlineCallback completes synchronously, such as in tests, twisted will string-ify the traceback, which means that when the exception is then thrown into another inlineCallback function, the stack trace is lost[1]. Twisted works around this by inspect the stack and finding the Failure instance from the inner call, to get traceback there.

However, this only works if Failure.throwExceptionIntoGenerator is used. This adjusts eliot_friendly_generator_function to use Failure, instead of an exc_info tuple, when used via inline_callabcks.

[1] The string-ified traceback can't be passed to .send.

itamarst commented 3 years ago

Thank you! I’m on vacation so will take a look when I return to work.

On Wed, Jul 14, 2021, at 4:21 PM, Tom Prince wrote:

Fixes #449 https://github.com/itamarst/eliot/issues/449. I'm not sure if this a great solution, but it appears to fix the issue by letting Failure's stack walking find the right context.

When an inlineCallback completes synchronously, such as in tests, twisted will string-ify the traceback, which means that when the exception is then thrown into another inlineCallback function, the stack trace is lost[1]. Twisted works around this by inspect the stack and finding the Failure instance from the inner call, to get traceback there.

However, this only works if Failure.throwExceptionIntoGenerator is used. This adjusts eliot_friendly_generator_function to use Failure, instead of an exc_info tuple, when used via inline_callabcks.

[1] The string-ified traceback can't be passed to .send.

You can view, comment on, or merge this pull request online at:

https://github.com/itamarst/eliot/pull/475

Commit Summary

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/itamarst/eliot/pull/475, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAY5QZREBZMMQVHBZUDAI63TXYLZNANCNFSM5AMMZ56A.

-- Itamar Turner-Trauring

itamarst commented 2 years ago

Thanks for your patience on a review.

This seems, apriori, fine? If the worry is breaking abstractions, inline callbacks is really the only use case for the generators code, so that seems OK.

So happy to merge this given a test.

itamarst commented 1 year ago

A new version of Twisted has just been released, and at this point the contextvars support in Twisted is probably good enough that normal inlineCallbacks should just work correctly (plus async/await!), with no need for special code in Eliot. If it's still relevant, do you want to try that and see if it works for you?