Skip to content

Commit

Permalink
Add generic error message for unhandled 500s (#751)
Browse files Browse the repository at this point in the history
Co-authored-by: Li Wan <49334982+wanliAlex@users.noreply.github.com>
Co-authored-by: Farshid Zavareh <farshid@marqo.ai>
  • Loading branch information
3 people authored Feb 6, 2024
1 parent edcb9fd commit 74bf999
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 4 deletions.
6 changes: 2 additions & 4 deletions src/marqo/tensor_search/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,6 @@ def marqo_base_exception_handler(request: Request, exc: base_exceptions.MarqoErr
# Base exceptions
(base_exceptions.InternalError, api_exceptions.InternalError),
(base_exceptions.InvalidArgumentError, api_exceptions.InvalidArgError),

# If no mapping is found, raise a generic API error (500)
(base_exceptions.MarqoError, api_exceptions.MarqoWebError),
]

converted_error = None
Expand All @@ -95,8 +92,9 @@ def marqo_base_exception_handler(request: Request, exc: base_exceptions.MarqoErr
break

# Completely unhandled exception (500)
# This should abstract away internal error.
if not converted_error:
converted_error = api_exceptions.MarqoWebError(exc.message)
converted_error = api_exceptions.MarqoWebError("Marqo encountered an unexpected internal error.")

return marqo_api_exception_handler(request, converted_error)

Expand Down
10 changes: 10 additions & 0 deletions tests/tensor_search/test_api_exception_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,3 +89,13 @@ def test_base_exception_handler_vespa_errors(self, mock_api_exception_handler):
with self.subTest("Vespa error: Vespa Error"):
marqo_base_exception_handler(self.normal_request, vespa_exceptions.VespaError(self.generic_error_message))
assert isinstance(mock_api_exception_handler.call_args_list[-1][0][1], api_exceptions.MarqoWebError)

@mock.patch("marqo.tensor_search.api.marqo_api_exception_handler")
def test_base_exception_handler_unhandled_error(self, mock_api_exception_handler):
# Ensure that an unhandled error is converted to a MarqoWebError and the original message is not propagated
marqo_base_exception_handler(self.normal_request, base_exceptions.MarqoError("This should not be propagated."))
converted_error = mock_api_exception_handler.call_args_list[-1][0][1]

self.assertIsInstance(converted_error, api_exceptions.MarqoWebError)
self.assertNotIn("This should not be propagated.", converted_error.message)
self.assertIn("unexpected internal error", converted_error.message)

0 comments on commit 74bf999

Please sign in to comment.