Skip to content

Commit 974ee42

Browse files
committed
test(django): replace flawed exception tests with correct ones
Previous tests didn't properly demonstrate the real Django flow where view exceptions are caught by Django and passed to process_exception. test_sync_middleware_exception_capture made get_response raise directly, which the context manager caught - doesn't test the real issue where Django catches view exceptions before they reach the middleware. test_process_exception_integration just called process_exception and asserted it was called - circular logic. New tests properly simulate Django's behavior: - test_view_exceptions_only_captured_via_process_exception: proves context manager can't see view exceptions, only process_exception captures them - test_process_exception_called_during_view_exception: simulates complete Django flow - test_process_exception_respects_capture_exceptions_false: verifies settings respected - test_process_exception_respects_request_filter: verifies request filter respected These tests would have caught the v6.7.5 regression where process_exception was removed.
1 parent 8d0d253 commit 974ee42

File tree

1 file changed

+113
-52
lines changed

1 file changed

+113
-52
lines changed

posthog/test/integrations/test_middleware.py

Lines changed: 113 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,86 @@ def extra_tags_func(request):
201201

202202
self.assertEqual(tags["$request_method"], "PATCH")
203203

204+
def test_process_exception_called_during_view_exception(self):
205+
"""
206+
Unit test verifying process_exception captures exceptions per Django's contract.
207+
208+
Since this is a library test (no Django runtime), we simulate how Django
209+
would invoke our middleware in production:
210+
1. Middleware.__call__ creates context with request tags
211+
2. View raises exception inside get_response
212+
3. Django's BaseHandler catches it, calls process_exception, returns error response
213+
4. Exception never propagates to middleware's context manager
214+
215+
We manually call process_exception to simulate Django's behavior - this is
216+
the only way to test the hook without a full Django integration test.
217+
"""
218+
mock_client = Mock()
219+
view_exception = ValueError("View raised this error")
220+
error_response = Mock(status_code=500)
221+
222+
def mock_get_response(request):
223+
# Simulate Django's exception handling: catches view exception,
224+
# calls process_exception hook if it exists, returns error response
225+
if hasattr(middleware, "process_exception"):
226+
middleware.process_exception(request, view_exception)
227+
return error_response
228+
229+
middleware = self.create_middleware(get_response=mock_get_response)
230+
middleware.client = mock_client
231+
232+
request = MockRequest(
233+
headers={"X-POSTHOG-DISTINCT-ID": "test-user"},
234+
method="POST",
235+
path="/api/endpoint",
236+
)
237+
response = middleware(request)
238+
239+
self.assertEqual(response.status_code, 500)
240+
mock_client.capture_exception.assert_called_once_with(view_exception)
241+
242+
def test_process_exception_respects_capture_exceptions_false(self):
243+
"""Verify process_exception respects capture_exceptions=False setting"""
244+
mock_client = Mock()
245+
view_exception = ValueError("Should not be captured")
246+
247+
def mock_get_response(request):
248+
if hasattr(middleware, "process_exception"):
249+
middleware.process_exception(request, view_exception)
250+
return Mock(status_code=500)
251+
252+
middleware = self.create_middleware(
253+
capture_exceptions=False, get_response=mock_get_response
254+
)
255+
middleware.client = mock_client
256+
257+
request = MockRequest()
258+
middleware(request)
259+
260+
mock_client.capture_exception.assert_not_called()
261+
262+
def test_process_exception_respects_request_filter(self):
263+
"""Verify process_exception respects request_filter setting"""
264+
mock_client = Mock()
265+
view_exception = ValueError("Should be filtered")
266+
267+
def mock_get_response(request):
268+
if hasattr(middleware, "process_exception"):
269+
middleware.process_exception(request, view_exception)
270+
return Mock(status_code=500)
271+
272+
middleware = self.create_middleware(
273+
request_filter=lambda req: False,
274+
capture_exceptions=True,
275+
get_response=mock_get_response,
276+
)
277+
middleware.client = mock_client
278+
279+
request = MockRequest()
280+
middleware(request)
281+
282+
mock_client.capture_exception.assert_not_called()
283+
204284

205285
class TestPosthogContextMiddlewareSync(unittest.TestCase):
206286
"""Test synchronous middleware behavior"""
@@ -250,71 +330,52 @@ def test_sync_middleware_with_filter(self):
250330
self.assertEqual(response, mock_response)
251331
get_response.assert_called_once_with(request)
252332

253-
def test_sync_middleware_exception_capture(self):
254-
"""Test that sync middleware captures exceptions during request processing"""
255-
mock_client = Mock()
256-
257-
# Make get_response raise an exception
258-
def raise_exception(request):
259-
raise ValueError("Test exception")
260-
261-
get_response = Mock(side_effect=raise_exception)
262-
263-
# Properly initialize middleware
264-
middleware = PosthogContextMiddleware(get_response)
265-
middleware.client = mock_client # Override with mock client
266-
267-
request = MockRequest()
268-
269-
# Should capture exception and re-raise
270-
with self.assertRaises(ValueError):
271-
middleware(request)
272-
273-
# Verify exception was captured by middleware
274-
mock_client.capture_exception.assert_called_once()
275-
captured_exception = mock_client.capture_exception.call_args[0][0]
276-
self.assertIsInstance(captured_exception, ValueError)
277-
self.assertEqual(str(captured_exception), "Test exception")
278-
279-
def test_process_exception_integration(self):
333+
def test_view_exceptions_only_captured_via_process_exception(self):
280334
"""
281-
Integration test simulating Django's actual exception handling flow.
335+
Demonstrates that process_exception is required to capture view exceptions.
282336
283-
When a view raises an exception:
284-
1. Middleware.__call__ creates context with request tags
285-
2. __call__ calls self.get_response(request)
286-
3. Inside Django's handler: catches exception, checks if middleware.process_exception
287-
exists (hasattr), calls it if present, returns error response
288-
4. Context manager exits
337+
In production Django, view exceptions don't propagate to middleware's context
338+
manager because Django's BaseHandler catches them first and converts them to
339+
error responses. Django provides the exception via process_exception hook instead.
340+
341+
This unit test proves:
342+
1. Context manager in __call__ never sees view exceptions (Django intercepts)
343+
2. Only process_exception can capture them
344+
3. Without process_exception, exceptions are silently lost (v6.7.5 regression)
289345
290-
This verifies exception capture works in the real Django flow.
346+
We manually call process_exception to verify the hook works - in production,
347+
Django's BaseHandler would call it when a view raises.
291348
"""
292349
mock_client = Mock()
350+
get_response = Mock(return_value=Mock(status_code=500))
293351

294-
get_response = Mock(return_value=Mock())
295352
middleware = PosthogContextMiddleware(get_response)
296353
middleware.client = mock_client
297354

298-
view_exception = ValueError("View error")
299-
error_response = Mock(status_code=500)
355+
def get_response_simulating_django(request):
356+
# Simulates Django behavior: view exception converted to error response,
357+
# never propagates to middleware's context manager
358+
return Mock(status_code=500)
300359

301-
def mock_get_response(request):
302-
# Simulate Django: check if process_exception exists, call it
303-
if hasattr(middleware, "process_exception"):
304-
middleware.process_exception(request, view_exception)
305-
return error_response
360+
middleware._sync_get_response = get_response_simulating_django
306361

307-
middleware.get_response = mock_get_response
362+
request = MockRequest()
308363

309-
request = MockRequest(
310-
headers={"X-POSTHOG-DISTINCT-ID": "user123"},
311-
method="POST",
312-
path="/api/test",
313-
)
314364
response = middleware(request)
315-
316-
self.assertEqual(response, error_response)
317-
mock_client.capture_exception.assert_called_once_with(view_exception)
365+
self.assertEqual(response.status_code, 500)
366+
367+
# Context manager didn't capture anything - exception was intercepted by Django
368+
mock_client.capture_exception.assert_not_called()
369+
370+
# Verify process_exception hook exists and captures exceptions when called
371+
if hasattr(middleware, "process_exception"):
372+
exception = ValueError("View error")
373+
middleware.process_exception(request, exception)
374+
mock_client.capture_exception.assert_called_once_with(exception)
375+
else:
376+
self.fail(
377+
"process_exception missing - view exceptions will not be captured!"
378+
)
318379

319380

320381
class TestPosthogContextMiddlewareAsync(unittest.TestCase):

0 commit comments

Comments
 (0)