Skip to content

Commit c25fab8

Browse files
🐛(backend) increase default timeout and nextcloud timeout (#214)
2 parents 5baa02e + 2477d72 commit c25fab8

File tree

4 files changed

+62
-8
lines changed

4 files changed

+62
-8
lines changed

backend/app/clients/base.py

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,11 @@ class BaseAPIClient:
1515

1616
service_name: str
1717

18-
def __init__(self, http_client: httpx.AsyncClient, base_url: str, token: str) -> None:
18+
def __init__(self, http_client: httpx.AsyncClient, base_url: str, token: str, timeout: float | None = None) -> None:
1919
self.client = http_client
2020
self.base_url = base_url.rstrip("/")
2121
self.token = token
22+
self.timeout = timeout
2223

2324
def _build_url(self, path: str) -> str:
2425
"""Build full URL from base and path."""
@@ -38,11 +39,13 @@ async def _get_resource_with_headers[T](
3839
"""Get resource and return both data and response headers."""
3940
try:
4041
url = self._build_url(path)
41-
response = await self.client.get(
42-
url,
43-
params=params or {},
44-
headers=self._auth_headers(),
45-
)
42+
kwargs: dict[str, Any] = {
43+
"params": params or {},
44+
"headers": self._auth_headers(),
45+
}
46+
if self.timeout is not None:
47+
kwargs["timeout"] = self.timeout
48+
response = await self.client.get(url, **kwargs)
4649

4750
if response.status_code != 200:
4851
raise ExternalServiceError(
@@ -54,6 +57,9 @@ async def _get_resource_with_headers[T](
5457
validated = TypeAdapter(model_type).validate_python(data)
5558
return validated, dict(response.headers)
5659

60+
except httpx.TimeoutException:
61+
logger.exception(f"Timeout calling {self.service_name} API")
62+
raise
5763
except httpx.HTTPError as e:
5864
logger.exception(f"HTTP error calling {self.service_name} API")
5965
raise ExternalServiceError(self.service_name, f"HTTP error: {e}") from e

backend/app/core/http_clients.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
logger = logging.getLogger(__name__)
1010

11-
DEFAULT_TIMEOUT = 2.0
11+
DEFAULT_TIMEOUT = 5.0
1212
DEFAULT_MAX_RETRIES = 2
1313

1414

backend/app/routes/ocs.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ async def get_ocs_client(request: Request, http_client: HTTPClient) -> OCSClient
2121

2222
token = await get_token(request, settings.OCS_AUDIENCE)
2323

24-
return OCSClient(http_client, settings.OCS_URL, token)
24+
return OCSClient(http_client, settings.OCS_URL, token, timeout=10.0)
2525

2626

2727
@router.get("/activities", response_model=FileActivityResponse)

backend/tests/clients/test_ocs.py

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,21 @@ def test_init(self, mock_http_client: AsyncMock, client: OCSClient) -> None:
4747
assert client.client is mock_http_client
4848
assert client.base_url == "https://nextcloud.example.com"
4949
assert client.token == "test-token"
50+
assert client.timeout is None
51+
52+
def test_init_with_custom_timeout(self, mock_http_client: AsyncMock) -> None:
53+
"""Test OCSClient initialization with custom timeout."""
54+
client = OCSClient(
55+
http_client=mock_http_client,
56+
base_url="https://nextcloud.example.com",
57+
token="test-token",
58+
timeout=10.0,
59+
)
60+
assert client.timeout == 10.0
61+
62+
def test_init_timeout_is_none_by_default(self, client: OCSClient) -> None:
63+
"""Test that timeout defaults to None."""
64+
assert client.timeout is None
5065

5166
def test_init_strips_trailing_slash(self, mock_http_client: AsyncMock) -> None:
5267
"""Test that trailing slash is stripped from base_url."""
@@ -183,6 +198,39 @@ async def test_search_files_no_results(self, client: OCSClient, mock_http_client
183198

184199
assert result == []
185200

201+
async def test_default_timeout_uses_client_default(self, client: OCSClient, mock_http_client: AsyncMock) -> None:
202+
"""Test that default timeout (None) does not pass timeout kwarg, preserving client default."""
203+
mock_response = create_mock_response(status_code=200, json_data={"ocs": {"data": {"entries": []}}})
204+
mock_http_client.get.return_value = mock_response
205+
206+
await client.search_files(term="test")
207+
208+
call_kwargs = mock_http_client.get.call_args[1]
209+
assert "timeout" not in call_kwargs
210+
211+
async def test_custom_timeout_passes_value(self, mock_http_client: AsyncMock) -> None:
212+
"""Test that custom timeout passes timeout value to client.get()."""
213+
client = OCSClient(
214+
http_client=mock_http_client,
215+
base_url="https://nextcloud.example.com",
216+
token="test-token",
217+
timeout=10.0,
218+
)
219+
mock_response = create_mock_response(status_code=200, json_data={"ocs": {"data": {"entries": []}}})
220+
mock_http_client.get.return_value = mock_response
221+
222+
await client.search_files(term="test")
223+
224+
call_kwargs = mock_http_client.get.call_args[1]
225+
assert call_kwargs["timeout"] == 10.0
226+
227+
async def test_timeout_exception_is_reraised(self, client: OCSClient, mock_http_client: AsyncMock) -> None:
228+
"""Test that httpx.TimeoutException is re-raised, not wrapped as ExternalServiceError."""
229+
mock_http_client.get.side_effect = httpx.ReadTimeout("read timed out")
230+
231+
with pytest.raises(httpx.TimeoutException):
232+
await client.search_files(term="test")
233+
186234
async def test_get_file_activities_single_file(self, client: OCSClient, mock_http_client: AsyncMock) -> None:
187235
"""Test file activities with a single file per activity."""
188236
mock_response = create_mock_response(

0 commit comments

Comments
 (0)