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

ASGITransport does not correctly simulate raw_path in the scope #1356

Closed
simonw opened this issue Oct 9, 2020 · 3 comments · Fixed by #1357
Closed

ASGITransport does not correctly simulate raw_path in the scope #1356

simonw opened this issue Oct 9, 2020 · 3 comments · Fixed by #1357

Comments

@simonw
Copy link
Contributor

simonw commented Oct 9, 2020

I'm trying to switch Datasette's internal tests over to using httpx with AsyncClient.

This has almost worked perfectly, but I've run into one problem: it looks like the ASGITransport class used by the AsyncClient(app=asgi_app) mechanism does not correctly simulate the raw_path and path keys.

Here's the code in question:

# ASGI scope.
scheme, host, port, full_path = url
path, _, query = full_path.partition(b"?")
scope = {
"type": "http",
"asgi": {"version": "3.0"},
"http_version": "1.1",
"method": method.decode(),
"headers": [(k.lower(), v) for (k, v) in headers],
"scheme": scheme.decode("ascii"),
"path": unquote(path.decode("ascii")),
"query_string": query,
"server": (host.decode("ascii"), port),
"client": self.client,
"root_path": self.root_path,
}

As you can see, it's not populating raw_path even though that's part of the ASGI spec.

This matters for Datasette because it supports this URL: https://latest.datasette.io/fixtures/table%2Fwith%2Fslashes.csv - which refers to a SQLite database table called table/with/slashes.csv (a weird table name but that's test cases for you). The way it does this is through careful decoding of the raw_path ASGI scope variable.

Here are my notes when I first ran into this limitation of ASGITransport: simonw/datasette#1000 (comment)

@simonw
Copy link
Contributor Author

simonw commented Oct 9, 2020

The fix here may be as straight-forward as adding this line to the code that constructs the scope:

    "raw_path": path,

@tomchristie
Copy link
Member

Since we're here - one related issue.

I'm interested in cases where you'd like to use the client for making the request, but need some information that is lost by the time you've sent an HTTP response. In particular eg. making a request and then verifying that the correct HTML template was used, and contains some particular context.

Have discussed some of this provisionally against the ASGI spec here... django/asgiref#135

Curious if any of that has relevance to any of your Datasette tests?

tomchristie pushed a commit that referenced this issue Oct 9, 2020
* Add raw_path to scope in ASGITransport, closes #1356

* Tweaked test
@simonw
Copy link
Contributor Author

simonw commented Oct 9, 2020

Extra debug/test information in ASGI messages is an interesting idea.

I've been working around that using the ASGI equivalent of threadlocals (context vars etc) - I usually avoid threadlocal-equivalents like the plague, but I've come to accept them for debugging/observability style mechanisms where you need to know what happened for informational puproses without having to re-engineer your entire stack to pass tracking values around.

Likewise, with ASGI I've thought about doing this using request IDs (also known as correlation IDs) that are set in an HTTP header and can be used to establish a key/value store that has been populated with data during the request.

These workarounds are working OK for me at the moment, but I'd certainly be keen to try out a less global-variable approach if one was available.

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 a pull request may close this issue.

2 participants