From e88c40864ff16af51a82cd75af3f72907a6aa2d0 Mon Sep 17 00:00:00 2001 From: Andy Chosak Date: Wed, 22 Jul 2026 14:39:38 -0400 Subject: [PATCH 1/2] Fix API response when size limit is exceeded The current code has a bug - it should have raised a 404 error but instead returns the Http404 Python class, which triggers a 500 error to the user. This commit changes the logic to be consistent with other API validation errors, using the DRF ValidationError class. These trigger a 400 HTTP Bad Request response to the user. --- complaint_search/decorators.py | 3 +++ complaint_search/export.py | 14 ++++++++++---- complaint_search/tests/test_export.py | 14 +++++++++++--- 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/complaint_search/decorators.py b/complaint_search/decorators.py index 3334746..6e492a5 100644 --- a/complaint_search/decorators.py +++ b/complaint_search/decorators.py @@ -2,6 +2,7 @@ from opensearchpy import TransportError from rest_framework import status +from rest_framework.exceptions import APIException from rest_framework.response import Response @@ -12,6 +13,8 @@ def catch_es_error(function): def wrap(request, *args, **kwargs): try: return function(request, *args, **kwargs) + except APIException: + raise except TransportError as te: log.error(te) diff --git a/complaint_search/export.py b/complaint_search/export.py index 07e246f..c676a34 100644 --- a/complaint_search/export.py +++ b/complaint_search/export.py @@ -4,13 +4,13 @@ from io import StringIO from django.http import StreamingHttpResponse -from django.http.response import Http404 + +from rest_framework.exceptions import ValidationError from complaint_search.defaults import MAX_DOWNLOAD_SIZE class OpenSearchExporter(object): - # export_csv - Stream an OpenSearch response as a CSV file # # Parameters: @@ -66,8 +66,14 @@ def stream(): # - total_count (int) # The total number of records to be output def export_json(self, scanResponse, total_count): - if not total_count or total_count > MAX_DOWNLOAD_SIZE: - return Http404 + if total_count and total_count > MAX_DOWNLOAD_SIZE: + raise ValidationError( + { + "size": [ + f"Result set of {total_count} exceeds the export limit of {MAX_DOWNLOAD_SIZE}" + ] + } + ) def stream(): count = 0 diff --git a/complaint_search/tests/test_export.py b/complaint_search/tests/test_export.py index 8b2fb16..3a4fc94 100644 --- a/complaint_search/tests/test_export.py +++ b/complaint_search/tests/test_export.py @@ -4,10 +4,10 @@ from collections import OrderedDict from django.http import StreamingHttpResponse -from django.http.response import Http404 from django.test import TestCase from parameterized import parameterized +from rest_framework.exceptions import ValidationError from complaint_search.defaults import MAX_DOWNLOAD_SIZE from complaint_search.export import OpenSearchExporter @@ -82,8 +82,16 @@ def test_json_export_limit(self): length = MAX_DOWNLOAD_SIZE + 1 es_exporter = OpenSearchExporter() gen = es_generator(length) - es_exporter.export_json(gen, length) - self.assertRaises(Http404) + with self.assertRaises(ValidationError) as context: + es_exporter.export_json(gen, length) + self.assertEqual(context.exception.get_codes(), {"size": ["invalid"]}) + + def test_json_export_empty(self): + es_exporter = OpenSearchExporter() + res = es_exporter.export_json(es_generator(0), 0) + self.assertIsInstance(res, StreamingHttpResponse) + content = io.BytesIO(b"".join(res.streaming_content)).read() + self.assertEqual(content, b"[]") class TestCSVExportWithUnicodeCharacters(TestCase): From 6bb5fa00345352e119d24944a7b6e7507cc30b7d Mon Sep 17 00:00:00 2001 From: Andy Chosak Date: Wed, 22 Jul 2026 16:18:26 -0400 Subject: [PATCH 2/2] Improve error logging Provide additional exception details when logging errors. --- complaint_search/decorators.py | 6 +-- complaint_search/tests/test_decorators.py | 55 +++++++++++++++++++++++ 2 files changed, 58 insertions(+), 3 deletions(-) create mode 100644 complaint_search/tests/test_decorators.py diff --git a/complaint_search/decorators.py b/complaint_search/decorators.py index 6e492a5..4c0768a 100644 --- a/complaint_search/decorators.py +++ b/complaint_search/decorators.py @@ -16,13 +16,13 @@ def wrap(request, *args, **kwargs): except APIException: raise except TransportError as te: - log.error(te) + log.error("OpenSearch %s on %s: %s", type(te).__name__, request.path, te) status_code = 424 # HTTP_424_FAILED_DEPENDENCY res = {"error": "There was an error calling OpenSearch"} return Response(res, status=status_code) - except Exception as e: - log.error(e) + except Exception: + log.exception("Unhandled error on %s", request.path) status_code = status.HTTP_500_INTERNAL_SERVER_ERROR res = {"error": "There was a problem retrieving your request"} diff --git a/complaint_search/tests/test_decorators.py b/complaint_search/tests/test_decorators.py new file mode 100644 index 0000000..124b7a7 --- /dev/null +++ b/complaint_search/tests/test_decorators.py @@ -0,0 +1,55 @@ +from django.test import RequestFactory, TestCase + +from opensearchpy import ConnectionTimeout, TransportError +from rest_framework import status +from rest_framework.exceptions import ValidationError + +from complaint_search.decorators import catch_es_error + + +class CatchESErrorTest(TestCase): + def setUp(self): + self.request = RequestFactory().get("/") + + def test_api_exception_is_not_swallowed(self): + @catch_es_error + def view(request): + raise ValidationError({"size": ["too big"]}) + + with self.assertRaises(ValidationError): + view(self.request) + + def test_transport_error_returns_424(self): + @catch_es_error + def view(request): + raise TransportError(503, "unavailable", {}) + + with self.assertLogs("complaint_search.decorators", "ERROR") as logs: + response = view(self.request) + + self.assertEqual(response.status_code, 424) + self.assertIn("OpenSearch TransportError on /", logs.output[0]) + + def test_connection_timeout_is_logged_by_class(self): + @catch_es_error + def view(request): + raise ConnectionTimeout( + "TIMEOUT", "read timed out", Exception("read timed out") + ) + + with self.assertLogs("complaint_search.decorators", "ERROR") as logs: + response = view(self.request) + + self.assertEqual(response.status_code, 424) + self.assertIn("OpenSearch ConnectionTimeout on /", logs.output[0]) + + def test_other_errors_return_500_with_traceback(self): + @catch_es_error + def view(request): + raise ValueError + + with self.assertLogs("complaint_search.decorators", "ERROR") as logs: + response = view(self.request) + + self.assertEqual(response.status_code, status.HTTP_500_INTERNAL_SERVER_ERROR) + self.assertIn("Traceback", logs.output[0])