Skip to content

Commit 8b0ffcd

Browse files
committed
Revert "Merge PR sooperset#572: DRAFT - fix(decorators): Propagate API errors instead of returning empty list"
This reverts commit e10f234, reversing changes made to b5743cf.
1 parent 2652e1c commit 8b0ffcd

File tree

2 files changed

+23
-32
lines changed

2 files changed

+23
-32
lines changed

src/mcp_atlassian/utils/decorators.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,13 @@ def wrapper(self: Any, *args: Any, **kwargs: Any) -> Any:
8989
operation_name = getattr(func, "__name__", "API operation")
9090
logger.error(f"Error processing {operation_name} results: {str(e)}")
9191
return []
92+
except Exception as e: # noqa: BLE001 - Intentional fallback with logging
93+
operation_name = getattr(func, "__name__", "API operation")
94+
logger.error(f"Unexpected error during {operation_name}: {str(e)}")
95+
logger.debug(
96+
f"Full exception details for {operation_name}:", exc_info=True
97+
)
98+
return []
9299

93100
return wrapper
94101

tests/unit/confluence/test_search.py

Lines changed: 16 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44

55
import pytest
66
import requests
7-
from atlassian.errors import ApiValueError
87
from requests import HTTPError
98

109
from mcp_atlassian.confluence.search import SearchMixin
@@ -257,6 +256,18 @@ def test_search_with_config_spaces_filter(self, search_mixin):
257256
)
258257
assert len(result) == 1
259258

259+
def test_search_general_exception(self, search_mixin):
260+
"""Test handling of general exceptions during search."""
261+
# Mock a general exception
262+
search_mixin.confluence.cql.side_effect = Exception("General error")
263+
264+
# Act
265+
results = search_mixin.search("error query")
266+
267+
# Assert
268+
assert isinstance(results, list)
269+
assert len(results) == 0
270+
260271
def test_search_user_success(self, search_mixin):
261272
"""Test search_user with successful results."""
262273
# Prepare the mock response
@@ -358,6 +369,7 @@ def test_search_user_with_custom_limit(self, search_mixin):
358369
(requests.RequestException, ("Network error",), []),
359370
(ValueError, ("Value error",), []),
360371
(TypeError, ("Type error",), []),
372+
(Exception, ("General error",), []),
361373
(KeyError, ("Missing key",), []),
362374
],
363375
)
@@ -375,37 +387,6 @@ def test_search_user_exception_handling(
375387
assert isinstance(results, list)
376388
assert results == expected_result
377389

378-
def test_search_propagated_exception_handling(self, search_mixin):
379-
"""Test that general Exceptions are propagated from the search method."""
380-
# Mock a general exception
381-
search_mixin.confluence.cql.side_effect = Exception("General error")
382-
383-
# Act and Assert
384-
with pytest.raises(Exception, match="General error"):
385-
search_mixin.search("error query")
386-
387-
def test_search_user_propagated_exception_handling(self, search_mixin):
388-
"""Test that general Exceptions are propagated from the search_user method."""
389-
# Mock a general exception
390-
search_mixin.confluence.get.side_effect = Exception("General error")
391-
392-
# Act and Assert
393-
with pytest.raises(Exception, match="General error"):
394-
search_mixin.search_user('user.fullname ~ "Test"')
395-
396-
def test_api_value_error_is_raised(self, search_mixin):
397-
"""Test that ApiValueError from the underlying library is raised, not suppressed."""
398-
# Arrange
399-
# Mock the cql method to raise ApiValueError, simulating an invalid query
400-
search_mixin.confluence.cql.side_effect = ApiValueError(
401-
"The query cannot be parsed", response=MagicMock()
402-
)
403-
404-
# Act & Assert
405-
# The decorator should now let this exception propagate instead of returning []
406-
with pytest.raises(ApiValueError):
407-
search_mixin.search("this is an invalid query")
408-
409390
@pytest.mark.parametrize(
410391
"status_code,exception_type",
411392
[
@@ -460,12 +441,14 @@ def test_search_user_edge_cases(self, search_mixin, mock_response, expected_leng
460441
assert isinstance(results, list)
461442
assert len(results) == expected_length
462443

444+
# You can also parametrize the regular search method exception tests:
463445
@pytest.mark.parametrize(
464446
"exception_type,exception_args,expected_result",
465447
[
466448
(requests.RequestException, ("API error",), []),
467449
(ValueError, ("Value error",), []),
468450
(TypeError, ("Type error",), []),
451+
(Exception, ("General error",), []),
469452
(KeyError, ("Missing key",), []),
470453
],
471454
)
@@ -483,6 +466,7 @@ def test_search_exception_handling(
483466
assert isinstance(results, list)
484467
assert results == expected_result
485468

469+
# Parametrize CQL query tests:
486470
@pytest.mark.parametrize(
487471
"query,limit,expected_params",
488472
[

0 commit comments

Comments
 (0)