Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Fix stacktraces when using ObservableDeferred and async/await #6836

Merged
merged 2 commits into from
Feb 3, 2020

Conversation

erikjohnston
Copy link
Member

So __failure__ is a magic (undocumented) thing that twisted will look for when raising exception during an await: https://github.com/twisted/twisted/blob/twisted-19.10.0/src/twisted/internet/defer.py#L748.

Empirically this seems to Do The Right Thing, but it somewhat feels like we're working against Twisted here. Ideally there would be some sort of ObservableDeferred in Twisted that handled this stuff properly, but there doesn't seem to be.

@erikjohnston erikjohnston requested a review from a team February 3, 2020 16:29
@richvdh
Copy link
Member

richvdh commented Feb 3, 2020

For a little more detail: this property is read back when a new Failure is constructed further up the stack: https://github.com/twisted/twisted/blob/twisted-19.10.0/src/twisted/python/failure.py#L302. The net effect is that the stacktrace in that new Failure is that in the original Failure (ie, the one that cased the observed Deferred to fail) rather than the point at which the original Failure was converted into an exception by sending it into an await.

Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@erikjohnston erikjohnston merged commit ae5b310 into develop Feb 3, 2020
@erikjohnston erikjohnston deleted the erikj/fix_stacktraces branch February 5, 2020 17:34
@clokep clokep mentioned this pull request Jul 30, 2020
48 tasks
babolivier pushed a commit that referenced this pull request Sep 1, 2021
* commit 'ae5b3104f':
  Fix stacktraces when using ObservableDeferred and async/await (#6836)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants