0b01001001 / spectree

API spec validator and OpenAPI document generator for Python web frameworks.
https://0b01001001.github.io/spectree/
Apache License 2.0
316 stars 74 forks source link

bug: annotations NameError issue caused by python bug #312

Open divad opened 1 year ago

divad commented 1 year ago

Describe the bug

Hey :) This isn't strictly a bug in Spectree but I wanted to raise it nonetheless (see below as to why).

The combination of Spectree, annotations mode and Flask's ResponseReturnValue does not work:

   File "/path/to/viewfuncs.py", line 28, in <module>
     @api.validate(
      ^^^^^^^^^^^^^
   File "/usr/local/lib/python3.11/site-packages/spectree/spec.py", line 221, in decorate_validation
     annotations = get_type_hints(func)
                   ^^^^^^^^^^^^^^^^^^^^
   File "/usr/local/lib/python3.11/typing.py", line 2361, in get_type_hints
     hints[name] = _eval_type(value, globalns, localns)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   File "/usr/local/lib/python3.11/typing.py", line 380, in _eval_type
     ev_args = tuple(_eval_type(a, globalns, localns, recursive_guard) for a in t.__args__)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   File "/usr/local/lib/python3.11/typing.py", line 380, in <genexpr>
     ev_args = tuple(_eval_type(a, globalns, localns, recursive_guard) for a in t.__args__)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   File "/usr/local/lib/python3.11/typing.py", line 366, in _eval_type
     return t._evaluate(globalns, localns, recursive_guard)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   File "/usr/local/lib/python3.11/typing.py", line 864, in _evaluate
     eval(self.__forward_code__, globalns, localns),
     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   File "<string>", line 1, in <module>
 NameError: name 'Response' is not defined

The cause is this bug in Python: https://bugs.python.org/issue43463

The tl;dr is: spectree calls get_type_hints, that in turn evaluates the function's return type, in this case Flask's ResponseReturnValue, this refers to a Response class, but it is done so via a string:

# The possible types that are directly convertible or are a Response object.
ResponseValue = t.Union[
    "Response",
    str,
    bytes,
    t.List[t.Any],
    # Only dict is actually accepted, but Mapping allows for TypedDict.
    t.Mapping[str, t.Any],
    t.Iterator[str],
    t.Iterator[bytes],
]

The Response class is not imported at runtime, it's behind a TYPE_CHECKING guard:

if t.TYPE_CHECKING:  # pragma: no cover
    from werkzeug.wrappers import Response  # noqa: F401

Thus, Spectree's annotations feature won't work if the function has a type hint where one of the constituent parts is only imported behind TYPE_CHECKING. The python bug doesn't appear as if it will have a resolution any time soon, sadly.

I am not very experienced with python typing, but, I wonder if spectree should be using get_type_hints. Underneath get_type_hints uses .__annotations__ which appears to already have the types spectree needs. Thus I wonder if spectree could use that rather than get_type_hints, thus sidestepping the issue. WDYT?

p.s many thanks for creating and maintaining spectree <3

To Reproduce

from flask.typing import ResponseReturnValue

# opt in into annotations feature
spec = SpecTree("flask", annotations=True)

@app.route("/api/user", methods=["POST"])
@spec.validate(resp=Response(HTTP_200=Message, HTTP_403=None), tags=["api"])
def user_profile(json: Profile) -> ResponseReturnValue:
    """
    verify user profile (summary of this endpoint)

    user's name, user's age, ... (long description)
    """
    print(json)  # or `request.json`
    return jsonify(text="it works")  # or `Message(text='it works')`

Expected behavior

Spectree/Flask starts up

The spectree version

Additional context

No response

kemingy commented 1 year ago

Hi @divad thanks for your feedback. This is an interesting bug.

Underneath get_type_hints uses .annotations which appears to already have the types spectree needs. Thus I wonder if spectree could use that rather than get_type_hints, thus sidestepping the issue. WDYT?

From the get_type_hint doc:

This is often the same as obj.annotations, but it handles forward references encoded as string literals, adds Optional[t] if a default value equal to None is set and recursively replaces all 'Annotated[T, ...]' with 'T' (unless 'include_extras=True').

Usually, get_type_hint should be a better choice. But we didn't expect this bug. So far, I don't have a better idea. Will take a deep look later.