Skip to content

Commit 82a1533

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 82a1533

File tree

1 file changed

+115
-52
lines changed

1 file changed

+115
-52
lines changed

posthog/test/integrations/test_middleware.py

Lines changed: 115 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,88 @@ 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+
246+
middleware = self.create_middleware(capture_exceptions=False)
247+
middleware.client = mock_client
248+
249+
view_exception = ValueError("Should not be captured")
250+
251+
def mock_get_response(request):
252+
if hasattr(middleware, "process_exception"):
253+
middleware.process_exception(request, view_exception)
254+
return Mock(status_code=500)
255+
256+
middleware._sync_get_response = mock_get_response
257+
258+
request = MockRequest()
259+
middleware(request)
260+
261+
mock_client.capture_exception.assert_not_called()
262+
263+
def test_process_exception_respects_request_filter(self):
264+
"""Verify process_exception respects request_filter setting"""
265+
mock_client = Mock()
266+
267+
middleware = self.create_middleware(
268+
request_filter=lambda req: False, capture_exceptions=True
269+
)
270+
middleware.client = mock_client
271+
272+
view_exception = ValueError("Should be filtered")
273+
274+
def mock_get_response(request):
275+
if hasattr(middleware, "process_exception"):
276+
middleware.process_exception(request, view_exception)
277+
return Mock(status_code=500)
278+
279+
middleware._sync_get_response = mock_get_response
280+
281+
request = MockRequest()
282+
middleware(request)
283+
284+
mock_client.capture_exception.assert_not_called()
285+
204286

205287
class TestPosthogContextMiddlewareSync(unittest.TestCase):
206288
"""Test synchronous middleware behavior"""
@@ -250,71 +332,52 @@ def test_sync_middleware_with_filter(self):
250332
self.assertEqual(response, mock_response)
251333
get_response.assert_called_once_with(request)
252334

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):
335+
def test_view_exceptions_only_captured_via_process_exception(self):
280336
"""
281-
Integration test simulating Django's actual exception handling flow.
337+
Demonstrates that process_exception is required to capture view exceptions.
282338
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
339+
In production Django, view exceptions don't propagate to middleware's context
340+
manager because Django's BaseHandler catches them first and converts them to
341+
error responses. Django provides the exception via process_exception hook instead.
289342
290-
This verifies exception capture works in the real Django flow.
343+
This unit test proves:
344+
1. Context manager in __call__ never sees view exceptions (Django intercepts)
345+
2. Only process_exception can capture them
346+
3. Without process_exception, exceptions are silently lost (v6.7.5 regression)
347+
348+
We manually call process_exception to verify the hook works - in production,
349+
Django's BaseHandler would call it when a view raises.
291350
"""
292351
mock_client = Mock()
352+
get_response = Mock(return_value=Mock(status_code=500))
293353

294-
get_response = Mock(return_value=Mock())
295354
middleware = PosthogContextMiddleware(get_response)
296355
middleware.client = mock_client
297356

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

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
362+
middleware._sync_get_response = get_response_simulating_django
306363

307-
middleware.get_response = mock_get_response
364+
request = MockRequest()
308365

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

319382

320383
class TestPosthogContextMiddlewareAsync(unittest.TestCase):

0 commit comments

Comments
 (0)