fix(cli): don't crash autodiscovery on unresolvable type hints - #4943
Open
Gooh456 wants to merge 1 commit into
Open
fix(cli): don't crash autodiscovery on unresolvable type hints#4943Gooh456 wants to merge 1 commit into
Gooh456 wants to merge 1 commit into
Conversation
_autodiscover_app() calls get_type_hints() on every callable in a scanned module's __dict__ while looking for a factory function. If a module just imports something like Request for annotation purposes (no app/application/create_app defined there), that import itself gets probed too, since classes are callable. Request pulls in ASGIConnection, whose `scope: Scope` annotation only has Scope importable under TYPE_CHECKING, so get_type_hints() raises a bare NameError that propagates straight out of the CLI instead of just skipping that attr and moving on. Pulled the get_type_hints call into a small helper that swallows NameError and returns None, same as "no return annotation" -- keeps _autodiscover_app's complexity under the ruff C901 limit too. Fixes litestar-org#3895 Signed-off-by: Kyue <164024549+Gooh456@users.noreply.github.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4943 +/- ##
=======================================
Coverage 67.31% 67.31%
=======================================
Files 293 293
Lines 15233 15240 +7
Branches 1728 1729 +1
=======================================
+ Hits 10254 10259 +5
- Misses 4832 4833 +1
- Partials 147 148 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
provinzkraut
requested changes
Jul 24, 2026
Comment on lines
+353
to
+360
| """Return the resolved ``"return"`` type hint for ``value``, or ``None`` if it has none or can't be resolved. | ||
|
|
||
| ``value`` may be an unrelated callable/class pulled in via a module-level import (e.g. | ||
| ``from litestar import Request``) rather than something the user actually intends as an app | ||
| factory. Its annotations can reference names that only exist under ``TYPE_CHECKING`` in their | ||
| defining module, so resolving them can raise a ``NameError`` even though it has nothing to do | ||
| with the app factory we're looking for. | ||
| """ |
Member
There was a problem hiding this comment.
Way too much docstring for so little functionality :)
Comment on lines
+361
to
+362
| if not hasattr(value, "__annotations__"): | ||
| return None |
Member
There was a problem hiding this comment.
Seems redundant. If you catch errors from get_type_hints, that should be good enough
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ran into this poking at the third loop in
_autodiscover_app(litestar/cli/_utils.py) — it walks every callable in a scanned module's__dict__looking for a factory, and callsget_type_hints()on each one. thing is, if that module doesn't defineapp/application/create_appand just has something likefrom litestar import Requestsitting there for annotations,Requestitself gets probed too since classes count as callable.Requestpulls inASGIConnection, and itsscope: Scopeattribute only hasScopeimportable underTYPE_CHECKING(litestar/connection/base.py). Soget_type_hints(Request)throws a bareNameError, uncaught, and the whole CLI dies instead of just skipping that attr and moving on to the actual factory.confirmed on main:
for a directory containing just:
fix: pulled the
get_type_hintscall into a small helper (_get_return_type_hint) that catchesNameErrorand returnsNone, treated the same as "no return annotation" — so autodiscovery just moves on to the next attr instead of blowing up. also happened to keep_autodiscover_appunder ruff's C901 complexity limit, which the inline try/except pushed it over.added a regression test in
test_env_resolution.pythat reproduces the original scenario (factory function alongside an unrelatedRequestimport) — fails withNameErroron unpatched code, passes with the fix. ran the fulltests/unit/test_cli/suite (876 passed), plus mypy/pyright/ruff clean on the touched files.Fixes #3895
📚 Documentation preview 📚: https://litestar-org.github.io/litestar-docs-preview/4943