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

HTTPConnection::__get_item__ has incorrect type signature #1117

Closed
2 tasks done
sburba opened this issue Dec 28, 2020 · 0 comments · Fixed by #1118
Closed
2 tasks done

HTTPConnection::__get_item__ has incorrect type signature #1117

sburba opened this issue Dec 28, 2020 · 0 comments · Fixed by #1118
Labels
typing Type annotations or mypy issues

Comments

@sburba
Copy link
Contributor

sburba commented Dec 28, 2020

Checklist

  • The bug is reproducible against the latest release and/or master.
  • There are no similar issues or pull requests to fix it yet.

Describe the bug

HTTPConnection's __get_item__ has the return type str. However, it returns the value for the provided key in the scope. Not all values in the scope are strings, (See documentation here: https://asgi.readthedocs.io/en/latest/specs/www.html#http-connection-scope

Since WebSocket inherits this method it also has the wrong return type.

To reproduce

This example uses WebSocket because that's how I originally found the issue.

Run mypy

mypy --pretty main.py

On the following code

from starlette.types import Scope, Receive, Send
from starlette.websockets import WebSocket


async def app(scope: Scope, receive: Receive, send: Send) -> None:
    websocket = WebSocket(scope=scope, receive=receive, send=send)
    await websocket.accept()
    ip, port = websocket['client']  # mypy fails to typecheck, because websocket['client'] typed as a str
    # Succeeds at runtime, because websocket['client'] is not a string
    assert isinstance(ip, str)
    assert isinstance(port, int)

Expected behavior

mypy typecheck should pass for valid code

Actual behavior

Mypy fails to typecheck with the following error:

main.py:8: error: Unpacking a string is disallowed
        ip, port = websocket['client']  # mypy fails to typecheck, because webso...
                   ^

Environment

  • OS: linux/windows/mac
  • Python version: 3.9
  • Starlette version: 0.14.1

Additional context

Changing the return type of __get_item__ to Any is likely the right thing to do here, which would be backwards-compatible for those currently using mypy.

As an extra solution to this problem, it would be great to expose the correct types for the scope using a TypedDict (couldn't be done for __getitem__, but could be done for the scope attribute on WebSocket and HTTPConnection), although that would be a breaking change for MyPy users, and also complicate the middleware story.

I'd be happy to work on either/both of these solutions if there's a consensus on what the right direction is.

sburba added a commit to sburba/starlette that referenced this issue Dec 28, 2020
@JayH5 JayH5 added the typing Type annotations or mypy issues label Feb 4, 2021
Kludex added a commit that referenced this issue Sep 18, 2021
Scope does not only contain strings, see full spec here:
https://asgi.readthedocs.io/en/latest/specs/www.html#websocket-connection-scope

Fixes #1117

Co-authored-by: Marcelo Trylesinski <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
typing Type annotations or mypy issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants