Skip to content

Fix test cleanup for uvicorn 0.39+ context isolation#2696

Merged
jlowin merged 1 commit intorelease/2.xfrom
ci-test-flaky
Dec 23, 2025
Merged

Fix test cleanup for uvicorn 0.39+ context isolation#2696
jlowin merged 1 commit intorelease/2.xfrom
ci-test-flaky

Conversation

@jlowin
Copy link
Copy Markdown
Member

@jlowin jlowin commented Dec 23, 2025

Uvicorn 0.39.0 introduced a fix for ContextVars pollution (encode/uvicorn#2742) that wraps ASGI tasks in empty contexts. This is a workaround for a CPython bug (python/cpython#140947) being fixed in Python 3.13+.

The context isolation breaks task cancellation propagation, causing test cleanup to hang when we task.cancel() and await task on uvicorn server tasks.

Fix: Use graceful shutdown (server.should_exit = True) for real uvicorn servers, and add timeouts for mocked server cleanup.

Files changed:

  • tests/server/test_logging.py - Add timeout to mocked server cleanup
  • tests/client/test_streamable_http.py - Use graceful shutdown for nested_server fixture
  • tests/client/test_sse.py - Use graceful shutdown for nested_sse_server fixture

@marvin-context-protocol marvin-context-protocol Bot added bug Something isn't working. Reports of errors, unexpected behavior, or broken functionality. tests http Related to HTTP transport, networking, or web server functionality. labels Dec 23, 2025
@jlowin jlowin mentioned this pull request Dec 23, 2025
@marvin-context-protocol
Copy link
Copy Markdown
Contributor

Test Failure Analysis

Summary: One test is failing: test_validation_error_propagates_from_dependency expects FastMCP's ValidationError to propagate unchanged, but it's being wrapped in a ToolError.

Root Cause: In src/fastmcp/tools/tool_manager.py:161, the code catches Pydantic's ValidationError (imported from pydantic), but not FastMCP's ValidationError (from fastmcp.exceptions). When a tool's dependency raises FastMCP's ValidationError, it propagates through the dependency system (as intended by #2633), but then gets caught by the generic Exception handler at line 167, which wraps it in a ToolError with the message "Error calling tool 'tool_with_validation': Invalid input format" instead of just "Invalid input format".

Suggested Solution: Add an exception handler for FastMCP's error types before the generic Exception handler in tool_manager.py:

# In src/fastmcp/tools/tool_manager.py, around line 161
try:
    return await tool.run(arguments)
except ValidationError as e:  # Pydantic ValidationError
    logger.exception(f"Error validating tool {key!r}: {e}")
    raise e
except ToolError as e:  # Already a ToolError
    logger.exception(f"Error calling tool {key!r}")
    raise e
except FastMCPError as e:  # Add this handler for ValidationError, ResourceError, etc.
    logger.exception(f"Error calling tool {key!r}")
    raise e
except Exception as e:  # Generic exceptions
    logger.exception(f"Error calling tool {key!r}")
    if self.mask_error_details:
        raise ToolError(f"Error calling tool {key!r}") from e
    else:
        raise ToolError(f"Error calling tool {key!r}: {e}") from e

This ensures that FastMCP's ValidationError (and other FastMCPError subclasses) propagate unchanged, as intended by the fix in commit afb22c4.

Detailed Analysis

Test Output

FAILED tests/server/test_dependencies.py::test_validation_error_propagates_from_dependency
AssertionError: assert 'Error calling tool \'tool_with_validation\': Invalid input format' == 'Invalid input format'

Code Flow

  1. Test raises fastmcp.exceptions.ValidationError("Invalid input format") in a dependency
  2. Dependency system correctly propagates it unchanged (per commit afb22c4)
  3. ToolManager.call_tool() catches it as Exception at line 167
  4. Gets wrapped in ToolError(f"Error calling tool {key!r}: {e}")
  5. Test expects just "Invalid input format" but receives "Error calling tool 'tool_with_validation': Invalid input format"

Related Files

  • src/fastmcp/tools/tool_manager.py:161-172 - Exception handling in call_tool()
  • src/fastmcp/exceptions.py:10-11 - FastMCP's ValidationError class
  • tests/server/test_dependencies.py:761-779 - Failing test
  • Commit afb22c4 - Added test expecting ValidationError to propagate
Related Files
  • src/fastmcp/tools/tool_manager.py:161-172 - Exception handling logic that needs to catch FastMCPError subclasses
  • src/fastmcp/exceptions.py:6-19 - Defines FastMCPError base class and subclasses including ValidationError
  • tests/server/test_dependencies.py:761-779 - Test that expects unchanged error propagation
  • src/fastmcp/server/dependencies.py:189-194 - Dependency system that correctly propagates FastMCPError (per commit afb22c4)

Note: This failure is unrelated to the uvicorn context isolation fixes in this PR. The modified test files (test_sse.py, test_streamable_http.py, test_logging.py) all pass successfully.

@jlowin jlowin merged commit 5daabf0 into release/2.x Dec 23, 2025
8 checks passed
@jlowin jlowin deleted the ci-test-flaky branch December 23, 2025 23:40
@Kludex
Copy link
Copy Markdown

Kludex commented Apr 21, 2026

I'm reverting that PR from Uvicorn, not sure how it impacts you. But please ping me if you have any questions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working. Reports of errors, unexpected behavior, or broken functionality. http Related to HTTP transport, networking, or web server functionality. tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants