Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use __traceback_hide__ to skip frames in tracebacks #383

Merged
merged 1 commit into from
Mar 31, 2023

Conversation

bigfootjon
Copy link
Contributor

@bigfootjon bigfootjon commented Mar 31, 2023

This idea came from ongoing work in django/django#16636 to try and hide sensitive variables in an async wrapper for a synchronous function in the auth contrib app in Django.

Basically, Django supports skipping frames that have a truthy __traceback_hide__ value in the frame's locals:

https://github.com/django/django/blob/85b52d22fd2841c34e95b3a80d6f2b668ce2f160/django/views/debug.py#L523-L527

We could take advantage of this pre-existing logic in Django and hide the internals of the asgiref.sync machinery.

Adopting this idea has 2 advantages:

  1. Users don't need to see frames they don't care about, this leans more in to the "just call async_to_sync/sync_to_async functions and don't worry about the rough edges" concept discussed in the README
  2. Hide potentially sensitive variables from tracebacks / error logs.

To elaborate on (2), django/django#16668 was an attempt to hide sensitive variables from async functions and failed, and lots of the discussion in django/django#16636 has been around trying to hide the sensitive variables within asgiref's internal machinery.

Disadvantages that I see:

  1. This is lying to the user. Right now frames internal to Django are shown on tracebacks, but are less eye-catching (see screenshots below) which is a compelling argument against this.
  2. Due to some calls from asgiref to built-in Python code there are a few frame we can't hide, which leads to a disjointed experience (see second set of screenshots).
  3. This is basically only useful in Django (and maybe some testing frameworks?) and otherwise is just noise

The big idea is that from an user's perspective they call async_to_sync or sync_to_async and magic happens and they get the result they want.

Another thought is that we could make this system opt-in:

# asgiref/__init__.py

hide_internals_in_tracebacks = False

# asgiref/sync.py
from asgiref import hide_internals_in_tracebacks

...
def internal_func():
  __traceback_hide__ = hide_internals_in_tracebacks
...

# <user code>.py

import asgiref

asgiref.hide_internals_in_tracebacks = True

from asgiref.sync import sync_to_async

async def foo():
  return await sync_to_async(sync_function)()

Then perhaps Django would opt-in by default, but I'm just designing this on the fly. I'm not committed to this idea but saw an opportunity.

I've prepared some screenshots showing this in-action to get an idea how this would feel for users:

(note, these screenshots were prepared on top of #382, so add one more frame if that PR is not accepted)

Type Current Proposed
sync_to_async current_sync_to_async after_sync_to_async
async_to_sync current_async_to_sync after_async_to_sync

@andrewgodwin
Copy link
Member

Makes sense to me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants