opentelemetry-python
opentelemetry-python copied to clipboard
TypeError from opentelemetry.context.contextvars_context in detach
Describe your environment
Python version: 3.12.4 SDK version: 1.25.0 API version: 1.25.0
What happened?
TypeError from opentelemetry.context.contextvars_context in detach. The function detach expected an instance of Token, but it got None. Thus, it fails to detach context.
Steps to Reproduce
Pass None to _RUNTIME_CONTEXT.detach
Expected Result
This kind of error should be elaborated, without avoiding mypy
Actual Result
TypeError occurs
Additional context
No response
Would you like to implement a fix?
Yes
Why do you think this should handled instead of delegating to mypy?
Hi, afaik, the related line here is ignoring mypy. In opentelemetry-api/src/opentelemetry/context/contextvars_context.py#50
self._current_context.reset(token) # type: ignore
Due to this comment, mypy cannot detect TypeError.
If you are suggesting to uncomment the ignore, I'm ok, the draft PR is just my initial proposal.
Hi, afaik, the related line here is ignoring mypy. In opentelemetry-api/src/opentelemetry/context/contextvars_context.py#50
self._current_context.reset(token) # type: ignoreDue to this comment, mypy cannot detect TypeError.
If you are suggesting to uncomment the ignore, I'm ok, the draft PR is just my initial proposal.
Missed that, thanks. Yeah, I think it would make sense to add type annotations here if feasible
I added type annotations - could you have a check on the latest PR of this issue?
@xrmx I managed to reproduce a problem with a 'None' token using a partially instrumented celery (sender not instrumented, worker instrumented) as seen above. This causes the context.detach to fail. I'm not sure if the fix is simply to to check if the token is not None before calling detach ?
Looks like this was fixed in https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2927
I think we (thanks to all who helped in getting this merged!) made the type annotations here clearer in https://github.com/open-telemetry/opentelemetry-python/pull/4346 so this can be caught using mypy etc. now.
Right, closing this since annotations are in. Thanks!