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

Bug: CLI - litestar run raises 'Scope' is not defined when using Request in type annotation #3895

Open
1 of 4 tasks
jacopofar opened this issue Dec 8, 2024 · 15 comments
Open
1 of 4 tasks
Labels
Bug 🐛 This is something that is not working as expected Needs MCVE This needs an MCVE

Comments

@jacopofar
Copy link

jacopofar commented Dec 8, 2024

Description

Hello, not 100% sure this is a bug, probably I'm missing something.

My app started breaking when I added an import like from litestar import Request or from litestar.datastructures import State, both imports are used only to add type hints to my app code.

Running litestar run now gives an exception like this:

Complete trace
Traceback (most recent call last):
  File "/home/jacopo/projects/milanorama/.venv/bin/litestar", line 8, in <module>
    sys.exit(run_cli())
             ^^^^^^^^^
  File "/home/jacopo/projects/milanorama/.venv/lib/python3.12/site-packages/litestar/__main__.py", line 6, in run_cli
    litestar_group()
  File "/home/jacopo/projects/milanorama/.venv/lib/python3.12/site-packages/rich_click/rich_command.py", line 367, in __call__
    return super().__call__(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/jacopo/projects/milanorama/.venv/lib/python3.12/site-packages/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/jacopo/projects/milanorama/.venv/lib/python3.12/site-packages/rich_click/rich_command.py", line 151, in main
    with self.make_context(prog_name, args, **extra) as ctx:
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/jacopo/projects/milanorama/.venv/lib/python3.12/site-packages/litestar/cli/_utils.py", line 224, in make_context
    self._prepare(ctx)
  File "/home/jacopo/projects/milanorama/.venv/lib/python3.12/site-packages/litestar/cli/_utils.py", line 206, in _prepare
    env = ctx.obj = LitestarEnv.from_env(ctx.params.get("app_path"), ctx.params.get("app_dir"))
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/jacopo/projects/milanorama/.venv/lib/python3.12/site-packages/litestar/cli/_utils.py", line 114, in from_env
    loaded_app = _autodiscover_app(cwd)
                 ^^^^^^^^^^^^^^^^^^^^^^
  File "/home/jacopo/projects/milanorama/.venv/lib/python3.12/site-packages/litestar/cli/_utils.py", line 342, in _autodiscover_app
    get_type_hints(value, include_extras=True).get("return") if hasattr(value, "__annotations__") else None
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/jacopo/projects/milanorama/.venv/lib/python3.12/site-packages/typing_extensions.py", line 1230, in get_type_hints
    hint = typing.get_type_hints(
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/home/jacopo/.local/share/uv/python/cpython-3.12.7-linux-x86_64-gnu/lib/python3.12/typing.py", line 2275, in get_type_hints
    value = _eval_type(value, base_globals, base_locals, base.__type_params__)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/jacopo/.local/share/uv/python/cpython-3.12.7-linux-x86_64-gnu/lib/python3.12/typing.py", line 415, in _eval_type
    return t._evaluate(globalns, localns, type_params, recursive_guard=recursive_guard)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/jacopo/.local/share/uv/python/cpython-3.12.7-linux-x86_64-gnu/lib/python3.12/typing.py", line 948, in _evaluate
    eval(self.__forward_code__, globalns, localns),
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<string>", line 1, in <module>
NameError: name 'Scope' is not defined. Did you mean: 'scope'?

investigating a bit I found that it fails when I import that module because, for what I understand, Litestar is scanning the modules to find the CLI commands as explained in the docs, and doing so tries to get their types. It invokes typing_extension which calls typing.get_type_hints from the standard library, which iterates over the elements in __mro__ for the Request class and finds ASGIConnection. This class has a scope member of type Scope which is a type that is imported only for type checking so at runtime it cannot be found.

A similar thing happens with from litestar.datastructures import State, which I use for type hints.

I'm very confused by this, I think this is a basic function and indeed if I create an app from scratch (like the hello world on the homepage) and import this there's no issue. Maybe something else is interfering.

URL to code causing the issue

https://github.com/jacopofar/litestar-mcve

MCVE

MCVE here: https://github.com/jacopofar/litestar-mcve
(on the `this-works` branch there's a minimal variation that works)


from litestar import Request
from typing_extensions import get_type_hints


get_type_hints(Request, include_extras=True)

from litestar.datastructures import State

get_type_hints(State, include_extras=True)

Steps to reproduce

1. clone the repo https://github.com/jacopofar/litestar-mcve
2. Run the app (there's a command in the README), the error is immediate
3. See in the `this-works` branch a small variation which runs

Screenshots

No response

Logs

Usage: litestar [OPTIONS] COMMAND [ARGS]...                                                                                                                                                                                                                
                                                                                
 Litestar CLI.                                                                  
                                                                                
╭─ Options ────────────────────────────────────────────────────────────────────╮
│ --app          TEXT       Module path to a Litestar application (TEXT)       │
│ --app-dir      DIRECTORY  Look for APP in the specified directory, by adding │
│                           this to the PYTHONPATH. Defaults to the current    │
│                           working directory.                                 │
│                           (DIRECTORY)                                        │
│ --help     -h             Show this message and exit.                        │
╰──────────────────────────────────────────────────────────────────────────────╯
Traceback (most recent call last):
  File "/opt/breakme/.venv/bin/litestar", line 8, in <module>
    sys.exit(run_cli())
             ^^^^^^^^^
  File "/opt/breakme/.venv/lib/python3.12/site-packages/litestar/__main__.py", line 6, in run_cli
    litestar_group()
  File "/opt/breakme/.venv/lib/python3.12/site-packages/rich_click/rich_command.py", line 367, in __call__
    return super().__call__(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/breakme/.venv/lib/python3.12/site-packages/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/breakme/.venv/lib/python3.12/site-packages/rich_click/rich_command.py", line 151, in main
    with self.make_context(prog_name, args, **extra) as ctx:
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/breakme/.venv/lib/python3.12/site-packages/litestar/cli/_utils.py", line 223, in make_context
    ctx = super().make_context(info_name, args, parent, **extra)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/breakme/.venv/lib/python3.12/site-packages/rich_click/rich_command.py", line 260, in make_context
    return super().make_context(info_name, args, parent, **extra)  # type: ignore[return-value]
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/breakme/.venv/lib/python3.12/site-packages/click/core.py", line 943, in make_context
    self.parse_args(ctx, args)
  File "/opt/breakme/.venv/lib/python3.12/site-packages/click/core.py", line 1644, in parse_args
    rest = super().parse_args(ctx, args)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/breakme/.venv/lib/python3.12/site-packages/click/core.py", line 1408, in parse_args
    value, args = param.handle_parse_result(ctx, opts, args)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/breakme/.venv/lib/python3.12/site-packages/click/core.py", line 2400, in handle_parse_result
    value = self.process_value(ctx, value)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/breakme/.venv/lib/python3.12/site-packages/click/core.py", line 2362, in process_value
    value = self.callback(ctx, self, value)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/breakme/.venv/lib/python3.12/site-packages/click/core.py", line 1300, in show_help
    echo(ctx.get_help(), color=ctx.color)
         ^^^^^^^^^^^^^^
  File "/opt/breakme/.venv/lib/python3.12/site-packages/click/core.py", line 704, in get_help
    return self.command.get_help(self)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/breakme/.venv/lib/python3.12/site-packages/click/core.py", line 1325, in get_help
    self.format_help(ctx, formatter)
  File "/opt/breakme/.venv/lib/python3.12/site-packages/rich_click/rich_command.py", line 359, in format_help
    self.format_options(ctx, formatter)
  File "/opt/breakme/.venv/lib/python3.12/site-packages/rich_click/rich_command.py", line 296, in format_options
    self.format_commands(ctx, formatter)  # type: ignore[arg-type]
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/breakme/.venv/lib/python3.12/site-packages/rich_click/rich_command.py", line 289, in format_commands
    get_rich_commands(self, ctx, formatter)
  File "/opt/breakme/.venv/lib/python3.12/site-packages/rich_click/rich_help_rendering.py", line 616, in get_rich_commands
    for command in obj.list_commands(ctx):
                   ^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/breakme/.venv/lib/python3.12/site-packages/litestar/cli/_utils.py", line 228, in list_commands
    self._prepare(ctx)
  File "/opt/breakme/.venv/lib/python3.12/site-packages/litestar/cli/_utils.py", line 206, in _prepare
    env = ctx.obj = LitestarEnv.from_env(ctx.params.get("app_path"), ctx.params.get("app_dir"))
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/breakme/.venv/lib/python3.12/site-packages/litestar/cli/_utils.py", line 114, in from_env
    loaded_app = _autodiscover_app(cwd)
                 ^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/breakme/.venv/lib/python3.12/site-packages/litestar/cli/_utils.py", line 340, in _autodiscover_app
    get_type_hints(value, include_extras=True).get("return") if hasattr(value, "__annotations__") else None
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/breakme/.venv/lib/python3.12/site-packages/typing_extensions.py", line 1230, in get_type_hints
    hint = typing.get_type_hints(
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/typing.py", line 2263, in get_type_hints
    value = _eval_type(value, base_globals, base_locals, base.__type_params__)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/typing.py", line 415, in _eval_type
    return t._evaluate(globalns, localns, type_params, recursive_guard=recursive_guard)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/typing.py", line 938, in _evaluate
    eval(self.__forward_code__, globalns, locals_to_pass),
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<string>", line 1, in <module>
NameError: name 'Scope' is not defined. Did you mean: 'scope'?

Litestar Version

2.13.0

Platform

  • Linux
  • Mac
  • Windows
  • Other (Please specify in the description above)

Note

While we are open for sponsoring on GitHub Sponsors and
OpenCollective, we also utilize Polar.sh to engage in pledge-based sponsorship.

Check out all issues funded or available for funding on our Polar.sh dashboard

  • If you would like to see an issue prioritized, make a pledge towards it!
  • We receive the pledge once the issue is completed & verified
  • This, along with engagement in the community, helps us know which features are a priority to our users.
Fund with Polar
@jacopofar jacopofar added the Bug 🐛 This is something that is not working as expected label Dec 8, 2024
@provinzkraut
Copy link
Member

I am not able to reproduce this, neither by cloning the MCVE repo and running the suggested docker command nor by trying to replicate the steps I assumed are relevant.

Could you please provide a self-contained example (ideally just a snippet of code) that produces the described behaviour?

@provinzkraut provinzkraut changed the title Bug: 'Scope' is not defined. Did you mean: 'scope' when an app module is importing Request Bug: CLI - litestar run raises 'Scope' is not defined when using Request in type annotation Dec 9, 2024
@provinzkraut provinzkraut added the Needs MCVE This needs an MCVE label Dec 9, 2024
@Seemone
Copy link

Seemone commented Dec 9, 2024

I wasn't able to reproduce on MacOS but Centos7 showed me the same error (3.10.0-1160.118.1.el7.x86_64)

@Seemone
Copy link

Seemone commented Dec 9, 2024

that Centos7 is on an AMD cpu, but I can reproduce on an Intel CPU with Debian 12.5 (6.1.0-21-amd64)

@euri10
Copy link
Contributor

euri10 commented Dec 9, 2024

I can reproduce just running locally, still not sure what's happening but in the meantime if you modify your app folder name to something else it will be ok

@euri10
Copy link
Contributor

euri10 commented Dec 11, 2024

so in essence the way I understand what's happening here is:

  • litestar command has no --app option set
  • autodiscovery kicks in
  • AUTODISCOVERY_FILE_NAMES has app hardcoded, the name of your module.
  • it loads an incorrect autodiscoverd app and you get that trace

i'm not sure how to tackle this:

  • try to guess better the app name (small gain lot of efforts imho)
  • remove entirely that autodicovery feature (is that breaking ?)
  • somehting else ?

@provinzkraut
Copy link
Member

* it loads an incorrect autodiscoverd app and you get that trace

It doesn't load the wrong thing really. It does what it's supposed to do. The logic it follows is this:

  • Does the module have an app or application variable in the global scope that's an instance of Litestar? If yes, use that if not
  • Does the module have a create_app function in the global scope? If yes, call it and use the return value, if not
  • Is any function in the module annotated to return Litestar? If yes, use that. If not, fail

The issue occurs within the last step, specifically on this line:

return_annotation = (
get_type_hints(value, include_extras=True).get("return") if hasattr(value, "__annotations__") else None
)

We're using include_extras=True, which will try to resolve those names, but fail. What we could do here is include the default Litestar namespace, which would fix the immediate issue.

However, there's a more fundamental issue with this approach as well: We can never be sure that get_type_hints does not return any error when running against user code, and it might be okay that it does, because some things not being available at runtime might be perfectly fine. We could do instead just fail silently and go on, but that might lead to unexpected behaviour, e.g. if in a factory function's signature there's a type that's not available at runtime.

Maybe we should just avoid get_type_hints entirely, coerce the return annotation to a string, and compare it against "Litestar". I think the failure rate for autodiscovery where someone has imported something that's not the Litestar type as Litestar and used that as their return type should be fairly low.

@Seemone
Copy link

Seemone commented Dec 11, 2024

@provinzkraut it's still not clear to me what's wrong with the MCVE or why it only happens in certain environments

@provinzkraut
Copy link
Member

@provinzkraut it's still not clear to me what's wrong with the MCVE or why it only happens in certain environments

I'm not sure about that either. Especially running in docker, I would absolutely expect this to work and, unless I'm severely mistaken, the cause should absolutely be independent of platform or even Python version

@jacopofar
Copy link
Author

yeah it's quite a mistery 🤷 at first I thought I had something weird on my pip cache or some hidden file or environment variable that was causing the issue.

I can add that I see the exception regardless of the value of the parameter include_extras. Looking at the code I think it makes sense that the exception is being raised in this case, because running outside a type checker that Scope is indeed not defined, what I don't understand is how can it work on other systems.

@jacopofar
Copy link
Author

jacopofar commented Dec 17, 2024

I found the reason for the inconsistent behavior across the systems: it's the order of the files found when iterating over the app folder.

I found that on an ARM machine the bug does not happen, while on my computer does, so I used a debugger to run the code side by side to find where they diverge.

Litestar by default checks the app/application folder to perform the autodiscovery, as expected, iterating over the python files in those folders.

Doing so it calls iterdir which according to Python documentation has an "arbitrary" order.

On the ARM instance the order of the iteration for the MCVE repo is this:
[PosixPath('/opt/breakme/app'), PosixPath('/opt/breakme/app/main.py'), PosixPath('/opt/breakme/app/location.py')]
and on my computer it is:
[PosixPath('/opt/breakme/app'), PosixPath('/opt/breakme/app/location.py'), PosixPath('/opt/breakme/app/main.py')]
when it finds the main.py first the code returns and stops the iteration, while on my computer it finds location.py first and breaks trying to examine its imports.

This explains why the bug cannot be reproduced reliably, it depends on the order in which the filesystem returns the files which is arbitrary. It can change even by moving the folder around, which caused me a lot of confusion during troubleshooting :)

The behavior can be made predictable by replacing the iterdir call above with something like sorted(path.glob('*')) but then the iteration would not be lazy and potentially slower on large folders.

For my case, I "solved" by importing the modules this way:

from typing import TYPE_CHECKING

from litestar import Router, post

if TYPE_CHECKING:
    from litestar import Request
    from litestar.datastructures import State

and then using string for the type hints:

@post("/log")
async def log(
    data: dict[str, str], request: "Request[None, None, State]", headers: dict[str, str]
) -> dict[str, bool]:

this way mypy --strict can still do the type checking but Litestar can safely iterate over it.

I think a relatively safe way to avoid this behavior (which can be very nasty to google and troubleshoot, since the stack trace doesn't even show where the import is happening) is to add this before the call to get_type_hints:

  if value.__module__.startswith("litestar."):
      # maybe also show a warning?
      continue

this way any import to Litestar itself will not be examined for autodiscovery. Still it creates an hidden dependency in case in the future a Litestar function is supposed to be "autodiscovered".

A nicer solution would be to be able to reproduce the typing.TYPE_CHECKING == True condition also when invoking get_type_hints, but it doesn't seem like this is possible.

@jacopofar
Copy link
Author

jacopofar commented Dec 17, 2024

Thinking more about it, it could be better to just catch this exception during discovery and show a warning when there's a NameError, adding the module and function name to hint users about what is happening. Or raise the exception again but this time the error message would be easier to act upon. I can go on with a PR if it sounds good

@Seemone
Copy link

Seemone commented Dec 17, 2024

Kudos for the low level debugging @jacopofar!
My 2cents: if the correctness of the autodiscovery depends on the order the files are listed, iit has avproblem

@jacopofar
Copy link
Author

Another possible way to make autodiscovery more reliable is to collect all of the apps found and check that there's only one after the folders have been scanned. This would avoid both this unpredictable behavior (not the error itself, just the fact it manifests randomly), detect the case in which there are multiple apps defined (which also is unpredictable at the moment) and keep the lazy iteration. I can do a PR for that too, it's a small change.

@provinzkraut
Copy link
Member

provinzkraut commented Dec 20, 2024

Thanks @jacopofar for the great write up!

I think there are two things to consider here:

  1. There is a possible case where multiple apps / factories are available on the same level, and running in different environments will result in different apps being used. IMO this is an edge case and not that critical, as you shouldn't use autodiscovery in production systems anyway, but something we can fix nevertheless.
  2. The way we handle the type hints isn't really reliable. In particular, it should
    • Only inspect things defined in the current module
    • Only inspect callables
    • When inspecting type annotations:
      • Try to resolve the return annotation with the default litestar namespace
      • If that fails, check the stringised return annotation (i.e. does it return 'Litestar')

@jacopofar
Copy link
Author

Why should one not use autodiscovery in production?

Indeed setting the variable with export LITESTAR_APP=app.main:app avoids autodiscovery completly, but I'd assume it to have the same behavior. Maybe something to add to the docs (the tutorial uses it, while the large example app seems not to but I'm not sure).

I think at this point the bug is understood and can be closed, I'd like to go on and implement some check to better report the issue to the user (i.e. report which file caused the issue), even without avoiding it, because this is almost inevitable if using mypy --strict and Litestar Request type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 🐛 This is something that is not working as expected Needs MCVE This needs an MCVE
Projects
None yet
Development

No branches or pull requests

4 participants