fix: raise a clear RuntimeError when inject runs without the DI interceptor - #21
Merged
Merged
Conversation
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
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.
Problem
An
@inject-decorated servicer method (or afetch_di_container()call) running on a server built withoutDIInterceptor/DIAioInterceptorfailed with the rawContextVarlookup error, which names nothing about the mistake or the fix:Over the wire the client saw
StatusCode.UNKNOWNwith detailsException calling application: <ContextVar name='modern_di_request_container' at 0x...>(sync) /Unexpected <class 'LookupError'>: <ContextVar ...>(aio).Change
fetch_di_containerand_resolve(the@injectpath) now read the per-RPC child through one private helper,_current_container, which catches theLookupErrorand raises a plainRuntimeErrorfrom None:Behaviour with the interceptor in place is unchanged. The
fetch_di_containerdocstring and the README API row are updated to sayRuntimeError.Behaviour change
fetch_di_container()outside an intercepted RPC now raisesRuntimeErrorinstead ofLookupError. The one test that pinnedLookupError(test_fetch_di_container_raises_outside_rpc) is changed to the newRuntimeError. The two white-box tests that assertLookupErrordirectly on_request_container.get()are untouched, since the ContextVar itself is unchanged.This follows the aiohttp/starlette/aiogram integrations (modern-python/modern-di-aiogram#22) and the integration-guide rule in modern-python/modern-di#490: a package-local
RuntimeError, not aModernDIErrorsubclass.Tests
tests/test_inject.py::test_inject_raises_without_interceptor(new):@injectmethod called with the ContextVar unset raisesRuntimeErrormatchingDIInterceptor.tests/test_inject.py::test_fetch_di_container_raises_outside_rpc: now expectsRuntimeErrormatchingDIInterceptor.tests/test_sync.py::test_inject_without_interceptor_reports_missing_interceptor(new): a realgrpc.serverwith no interceptor; the client getsStatusCode.UNKNOWNwithDIInterceptorin the details.All three were written first and failed with the original
LookupErrorbefore the fix.just lint-ciandjust test-cipass at 100% coverage.