Skip to content

Commit e9e16e2

Browse files
author
Wouter Mellema
committed
Address PR #12 round 2 review: cache known fields, fix flaky test, robust error handling
- Cache _get_known_fields() result per-class to avoid recomputation - Use responses.add_callback() in concurrent pagination test for deterministic page matching - Fix RateLimitError.__str__ to use 'is not None' for retry_after=0 - Guard _handle_response() against non-dict JSON error bodies
1 parent f527f62 commit e9e16e2

4 files changed

Lines changed: 38 additions & 24 deletions

File tree

betterstack/uptime/api.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,10 @@ def _handle_response(self, response: requests.Response) -> None:
116116
status_code = response.status_code
117117
try:
118118
error_body = response.json()
119-
message = error_body.get("error", response.reason)
119+
if isinstance(error_body, dict):
120+
message = error_body.get("error", response.reason)
121+
else:
122+
message = response.reason or f"HTTP {status_code}"
120123
except (ValueError, KeyError):
121124
error_body = None
122125
message = response.reason or f"HTTP {status_code}"

betterstack/uptime/base.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,9 @@ def _get_known_fields(cls) -> set[str]:
9393
for f in dataclass_fields(cls):
9494
if not f.name.startswith("_") and f.name != "id":
9595
known.add(f.name)
96+
97+
# Cache per-class to avoid recomputation
98+
cls._known_fields = known
9699
return known
97100

98101
def _set_attribute(self, name: str, value: Any) -> None:

betterstack/uptime/exceptions.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ def __init__(
6060

6161
def __str__(self) -> str:
6262
base = super().__str__()
63-
if self.retry_after:
63+
if self.retry_after is not None:
6464
return f"{base} (retry after {self.retry_after}s)"
6565
return base
6666

tests/test_paginatedapi.py

Lines changed: 30 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
"""Tests for PaginatedAPI class."""
22

3+
import json
34
import unittest
45

56
import responses
@@ -128,11 +129,8 @@ def test_get_single_object(self):
128129
@responses.activate
129130
def test_get_concurrent_multiple_pages(self):
130131
"""Test concurrent fetching of multiple pages."""
131-
# First page response with pagination info showing 3 total pages
132-
responses.add(
133-
responses.GET,
134-
f"{TEST_BASE_URL}test_json",
135-
json={
132+
page_responses = {
133+
None: {
136134
"data": [{"id": "1", "name": "item1"}],
137135
"pagination": {
138136
"first": f"{TEST_BASE_URL}test_json?page=1",
@@ -141,14 +139,16 @@ def test_get_concurrent_multiple_pages(self):
141139
"next": f"{TEST_BASE_URL}test_json?page=2",
142140
},
143141
},
144-
status=200,
145-
)
146-
147-
# Second page response
148-
responses.add(
149-
responses.GET,
150-
f"{TEST_BASE_URL}test_json",
151-
json={
142+
"1": {
143+
"data": [{"id": "1", "name": "item1"}],
144+
"pagination": {
145+
"first": f"{TEST_BASE_URL}test_json?page=1",
146+
"last": f"{TEST_BASE_URL}test_json?page=3",
147+
"prev": None,
148+
"next": f"{TEST_BASE_URL}test_json?page=2",
149+
},
150+
},
151+
"2": {
152152
"data": [{"id": "2", "name": "item2"}],
153153
"pagination": {
154154
"first": f"{TEST_BASE_URL}test_json?page=1",
@@ -157,14 +157,7 @@ def test_get_concurrent_multiple_pages(self):
157157
"next": f"{TEST_BASE_URL}test_json?page=3",
158158
},
159159
},
160-
status=200,
161-
)
162-
163-
# Third page response
164-
responses.add(
165-
responses.GET,
166-
f"{TEST_BASE_URL}test_json",
167-
json={
160+
"3": {
168161
"data": [{"id": "3", "name": "item3"}],
169162
"pagination": {
170163
"first": f"{TEST_BASE_URL}test_json?page=1",
@@ -173,7 +166,22 @@ def test_get_concurrent_multiple_pages(self):
173166
"next": None,
174167
},
175168
},
176-
status=200,
169+
}
170+
171+
def page_callback(request):
172+
from urllib.parse import parse_qs, urlparse
173+
174+
parsed = urlparse(request.url)
175+
params = parse_qs(parsed.query)
176+
page = params.get("page", [None])[0]
177+
body = page_responses.get(page, page_responses[None])
178+
return (200, {}, json.dumps(body))
179+
180+
responses.add_callback(
181+
responses.GET,
182+
f"{TEST_BASE_URL}test_json",
183+
callback=page_callback,
184+
content_type="application/json",
177185
)
178186

179187
resp = list(self.api.get("test_json"))

0 commit comments

Comments
 (0)