Browse Source

Fixes #23471: Don't report 404s to Sentry when replaced by an APPEND_SLASH redirect (#23650)

Arthur Hanson 4 hours ago
parent
commit
b39d665753

+ 11 - 0
netbox/netbox/middleware.py

@@ -40,6 +40,8 @@ class CommonMiddleware(DjangoCommonMiddleware):
     body (clients downgrade to GET on a 302) or raise a RuntimeError when
     DEBUG is enabled. Letting the original 404 propagate gives the caller a
     clear, actionable error instead.
+
+    Also reports 404 responses to Sentry after APPEND_SLASH handling.
     """
     UNSAFE_METHODS = frozenset(('DELETE', 'PATCH', 'POST', 'PUT'))
 
@@ -48,6 +50,15 @@ class CommonMiddleware(DjangoCommonMiddleware):
             return False
         return super().should_redirect_with_slash(request)
 
+    def process_response(self, request, response):
+        response = super().process_response(request, response)
+
+        if settings.SENTRY_ENABLED and response.status_code == 404 and getattr(request, '_sentry_report_404', False):
+            from sentry_sdk import capture_message
+            capture_message("Page not found", level="error")
+
+        return response
+
 
 class CoreMiddleware:
 

+ 48 - 1
netbox/netbox/tests/test_middleware.py

@@ -1,4 +1,5 @@
 import json
+import sys
 from contextlib import contextmanager
 from types import SimpleNamespace
 from unittest.mock import Mock, patch
@@ -10,7 +11,7 @@ from django.urls import reverse
 from rest_framework import status
 
 from netbox.middleware import CoreMiddleware, MaintenanceModeMiddleware
-from utilities.testing import TestCase
+from utilities.testing import TestCase, disable_warnings
 
 
 class CoreMiddlewareTestCase(TestCase):
@@ -156,3 +157,49 @@ class CoreMiddlewareTestCase(TestCase):
             response = self.process_internal_error(request, 'Simulated maintenance mode GraphQL error')
 
         self.assert_json_500_response(response)
+
+
+@override_settings(SENTRY_ENABLED=True)
+class SentryNotFoundReportingTestCase(TestCase):
+
+    def setUp(self):
+        super().setUp()
+        self.sentry_sdk = Mock()
+        patcher = patch.dict(sys.modules, {'sentry_sdk': self.sentry_sdk})
+        patcher.start()
+        self.addCleanup(patcher.stop)
+
+    def test_404_reported_to_sentry(self):
+        with disable_warnings('django.request'):
+            response = self.client.get('/nonexistent-path/')
+
+        self.assertEqual(response.status_code, 404)
+        self.sentry_sdk.capture_message.assert_called_once_with('Page not found', level='error')
+
+    def test_append_slash_redirect_not_reported_to_sentry(self):
+        url = reverse('api-status')
+
+        for method in ('get', 'head'):
+            with self.subTest(method=method):
+                response = getattr(self.client, method)(url.rstrip('/'))
+
+                self.assertEqual(response.status_code, 301)
+                self.assertEqual(response.headers['Location'], url)
+                self.sentry_sdk.capture_message.assert_not_called()
+
+    def test_unsafe_api_request_without_slash_reported_to_sentry(self):
+        with disable_warnings('django.request'):
+            response = self.client.post(
+                reverse('api-status').rstrip('/'), data={}, content_type='application/json'
+            )
+
+        self.assertEqual(response.status_code, 404)
+        self.sentry_sdk.capture_message.assert_called_once_with('Page not found', level='error')
+
+    @override_settings(SENTRY_ENABLED=False)
+    def test_404_not_reported_when_sentry_disabled(self):
+        with disable_warnings('django.request'):
+            response = self.client.get('/nonexistent-path/')
+
+        self.assertEqual(response.status_code, 404)
+        self.sentry_sdk.capture_message.assert_not_called()

+ 3 - 4
netbox/netbox/views/errors.py

@@ -31,11 +31,10 @@ class StaticMediaFailureView(View):
 
 def handler_404(request, exception):
     """
-    Wrap Django's default 404 handler to enable Sentry reporting.
+    Wrap Django's default 404 handler to enable Sentry reporting. The report is sent by CommonMiddleware
+    once the final response is known (it may be replaced with an APPEND_SLASH redirect).
     """
-    if settings.SENTRY_ENABLED:
-        from sentry_sdk import capture_message
-        capture_message("Page not found", level="error")
+    request._sentry_report_404 = True
 
     return page_not_found(request, exception)