fix(django): make middleware truly hybrid-compatible with sync and async Django stacks #348

webjunkie · 2025-10-24T12:18:43Z

Summary

Make PostHog Django middleware truly hybrid, compatible with both sync (WSGI) and async (ASGI) Django applications without forcing sync-only deployments into async mode.

Context

PR #328 introduced async middleware support but inadvertently broke sync-only Django setups by setting both capability flags, which forces Django to treat the entire middleware chain as async. This breaks compatibility with sync middlewares like sessions and authentication.

Changes

Middleware Implementation

Keep __call__ as sync method that conditionally routes to __acall__ for async paths
Use markcoroutinefunction() to properly mark middleware instances when async is detected
Detect async/sync at initialization via iscoroutinefunction(get_response)
Remove process_exception method - it was non-functional (Django doesn't call it on new-style middleware without MiddlewareMixin)
Exception capture works correctly via contexts.new_context(capture_exceptions=True) which has built-in exception handling

Code Quality Improvements

Fix markcoroutinefunction fallback to be a no-op instead of accessing private API (addresses Copilot review comment)
Add comprehensive test coverage (18 tests total):
- Sync middleware lifecycle tests
- Async middleware lifecycle tests
- Hybrid routing behavior tests
- Exception capture tests for both sync and async paths
- Request filtering tests in both modes
Refactor tests to use proper initialization patterns
Configure Django settings properly in test suite

Technical Details

This follows Django's recommended hybrid middleware pattern where:

Both sync_capable = True and async_capable = True are set
__call__ remains synchronous but routes to async path when needed
Instance is marked with markcoroutinefunction() when async mode detected
Django passes requests without conversion, middleware adapts based on detected mode

The sync path behavior (lines 198-206) is identical to version 6.7.4 (pre-async), ensuring perfect backward compatibility for WSGI deployments.

Test Results

18 tests passed - Full backward compatibility maintained
All new middleware tests pass (sync, async, hybrid paths, exception capture)
Formatted with ruff

Acceptance Criteria

✅ Middleware works with both sync (WSGI) and async (ASGI) Django apps
✅ No runtime errors when mixed with sync middlewares
✅ Tests pass under both sync and async runs
✅ Backward compatibility preserved for users on 6.7.4
✅ Exception capturing works via context manager
✅ Addresses all valid code review comments

Addresses #329
Related to #328

Copilot

Pull Request Overview

This PR fixes the Django middleware to support both sync (WSGI) and async (ASGI) applications by implementing a true hybrid pattern. The previous async-only approach (PR #328) broke compatibility with sync middleware chains. This change restores the process_exception method that was removed, fixing exception capture regression (issue #329).

Key Changes:

Implements hybrid middleware pattern using conditional routing in __call__ to __acall__ for async paths
Restores process_exception method for exception capturing in both sync and async modes
Adds comprehensive test coverage for sync, async, and hybrid middleware behavior

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File	Description
posthog/integrations/django.py	Refactored middleware to support hybrid sync/async operation and restored exception processing
posthog/test/integrations/test_middleware.py	Added comprehensive test suite covering sync, async, and hybrid middleware scenarios
CHANGELOG.md	Documented the middleware fix and exception capture restoration

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

posthog/integrations/django.py

posthog/test/integrations/test_middleware.py

greptile-apps

_{3 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

posthog/test/integrations/test_middleware.py

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

posthog/integrations/django.py

…ync Django stacks Address code review feedback and critical issues from PR #328. Changes: - Keep __call__ as sync method that conditionally routes to __acall__ for async paths - Use markcoroutinefunction() to properly mark instances when async is detected - Detect async/sync at init time via iscoroutinefunction(get_response) - Remove process_exception method - it was non-functional (Django doesn't call it on new-style middleware without MiddlewareMixin) - Fix markcoroutinefunction fallback to be a simple no-op instead of accessing private API - Exception capture works correctly via contexts.new_context() which has built-in exception handling - Add comprehensive test coverage for sync, async, and hybrid middleware behavior - Add async exception capture tests - Refactor tests to use proper middleware initialization This implementation follows Django's recommended hybrid middleware pattern where both sync_capable and async_capable are True, allowing Django to pass requests without conversion while the middleware adapts based on the detected mode. The sync path behavior is identical to version 6.7.4 (pre-async), ensuring perfect backward compatibility for WSGI deployments. Addresses #329 Related to #328

Copilot

Pull Request Overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

webjunkie · 2025-10-24T12:54:30Z

@codex review

chatgpt-codex-connector · 2025-10-24T13:00:57Z

Codex Review: Didn't find any major issues. 🚀

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

skoob13

Works both in WSGI and ASGI. Thank you!

Restores the process_exception method that was removed in v6.7.5 (PR #328), which broke exception capture from Django views and downstream middleware. Django converts view exceptions into responses before they propagate through the middleware stack's __call__ method, so the context manager's exception handler never sees them. Django provides these exceptions via the process_exception hook instead. Changes: - Add process_exception method to capture exceptions from views and downstream middleware with proper request context and tags - Add tests verifying process_exception behavior and settings (capture_exceptions, request_filter) - Update version to 6.7.11 - Update CHANGELOG to accurately reflect the fix Context: - v6.7.4 and earlier: no process_exception - exceptions not captured (#286) - v6.7.4 fix: added process_exception - worked correctly - v6.7.5 (PR #328): removed process_exception when adding async - broke again (#329) - v6.7.10 (PR #348): attempted fix but didn't restore process_exception - still broken - v6.7.11 (this fix): restores process_exception - fixes #329 Fixes #329

webjunkie · 2025-10-27T07:54:31Z

This PR claimed process_exception was "non-functional without MiddlewareMixin" and removed it, expecting the context manager to handle exception capture. That assessment was incorrect.

Django's process_exception works on all middleware styles and is required to capture view exceptions. Django's BaseHandler intercepts view exceptions before they reach middleware's context manager, providing them only via the process_exception hook. Without it, view exceptions are silently lost.

Properly fixed in #350 with process_exception restored and tests that correctly demonstrate Django's exception interception behavior.

Fixes SynchronousOnlyOperation when accessing request.user in ASGI deployments. Django's request.user is a lazy object that triggers DB access when touched. In async context, this raises SynchronousOnlyOperation. The middleware now uses request.auser() in async paths to avoid blocking calls. Changes: - Add aextract_tags() and aextract_request_user() methods - Update __acall__() to use async versions - Add test verifying user extraction works in async context - Follow Django's naming convention for async methods (auser, asave, etc.) The issue was introduced in v6.7.5 (PR #328) but only became apparent after v6.7.10 (PR #348) made ASGI functional enough for users to discover it. Fixes #355

webjunkie requested a review from Copilot October 24, 2025 12:19

webjunkie marked this pull request as ready for review October 24, 2025 12:19

Copilot AI reviewed Oct 24, 2025

View reviewed changes

greptile-apps bot reviewed Oct 24, 2025

View reviewed changes

posthog/test/integrations/test_middleware.py Show resolved Hide resolved

chatgpt-codex-connector bot reviewed Oct 24, 2025

View reviewed changes

posthog/integrations/django.py Outdated Show resolved Hide resolved

webjunkie force-pushed the fix/django-hybrid-middleware branch from 36275f0 to 8c2d4e7 Compare October 24, 2025 12:36

webjunkie requested a review from Copilot October 24, 2025 12:54

Copilot AI reviewed Oct 24, 2025

View reviewed changes

sortafreel approved these changes Oct 24, 2025

View reviewed changes

skoob13 approved these changes Oct 24, 2025

View reviewed changes

chore: bump version to 6.7.10

d114496

webjunkie linked an issue Oct 24, 2025 that may be closed by this pull request

Django middleware is not capturing exceptions, again (regression of #286) #329

Closed

webjunkie merged commit 6af129f into master Oct 24, 2025
10 checks passed

webjunkie deleted the fix/django-hybrid-middleware branch October 24, 2025 13:50

webjunkie mentioned this pull request Oct 24, 2025

fix(django): restore process_exception to capture view exceptions #350

Merged

webjunkie mentioned this pull request Oct 28, 2025

fix(django): handle request.user in async middleware context #358

Open

fix(django): make middleware truly hybrid-compatible with sync and async Django stacks #348

fix(django): make middleware truly hybrid-compatible with sync and async Django stacks #348

Uh oh!

Conversation

webjunkie commented Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Context

Changes

Middleware Implementation

Code Quality Improvements

Technical Details

Test Results

Acceptance Criteria

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

webjunkie commented Oct 24, 2025

Uh oh!

chatgpt-codex-connector bot commented Oct 24, 2025

Uh oh!

skoob13 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

webjunkie commented Oct 27, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

webjunkie commented Oct 24, 2025 •

edited

Loading