Skip to content

Commit 78e3559

Browse files
digithreeclaude
andcommitted
Fix error detection to check error value, not just key presence
- Change error detection from checking key existence to checking value - Only treat as error if error key has non-None value - Add test for success case with 'error': None in response - Fixes false positive error detection when API returns success The Pocket API returns {'error': None, 'list': {...}} for successful responses, so we need to check the error value, not just key presence. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 877b5ee commit 78e3559

File tree

2 files changed

+38
-3
lines changed

2 files changed

+38
-3
lines changed

pocket_to_sqlite/utils.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -126,9 +126,9 @@ def __iter__(self):
126126
page = response.json()
127127
logging.debug(f"API response keys: {list(page.keys())}")
128128

129-
# Check for API errors
130-
if "error" in page:
131-
error_msg = page.get('error', 'Unknown error')
129+
# Check for API errors (error key present AND has a non-None value)
130+
error_msg = page.get('error')
131+
if error_msg is not None:
132132
logging.error(f"API returned error: {page}")
133133

134134
# Handle payload too large by reducing page size

tests/test_save_pocket.py

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,41 @@ def test_fetch_items_handles_payload_too_large():
185185
assert fetcher.page_size == 50 # 100 // 2
186186

187187

188+
def test_fetch_items_handles_error_none_success():
189+
"""Test that FetchItems handles responses with error: None as success."""
190+
from unittest.mock import Mock, patch
191+
192+
# Mock auth
193+
auth = {
194+
"pocket_consumer_key": "test_key",
195+
"pocket_access_token": "test_token"
196+
}
197+
198+
# Mock response with error: None (this is actually success)
199+
mock_response_success = Mock()
200+
mock_response_success.status_code = 200
201+
mock_response_success.json.return_value = {
202+
"error": None,
203+
"list": {"1": {"item_id": "1", "title": "Test"}},
204+
"since": 123
205+
}
206+
mock_response_success.raise_for_status.return_value = None
207+
208+
# Mock empty response to end iteration
209+
mock_response_empty = Mock()
210+
mock_response_empty.status_code = 200
211+
mock_response_empty.json.return_value = {"error": None, "list": {}, "since": 124}
212+
mock_response_empty.raise_for_status.return_value = None
213+
214+
with patch('requests.post', side_effect=[mock_response_success, mock_response_empty]):
215+
fetcher = utils.FetchItems(auth, page_size=50)
216+
items = list(fetcher)
217+
218+
# Should successfully fetch items despite error key being present
219+
assert len(items) == 1
220+
assert items[0]["item_id"] == "1"
221+
222+
188223
def test_ensure_fts_with_no_items_table():
189224
"""Test that ensure_fts handles case when items table doesn't exist."""
190225
db = sqlite_utils.Database(":memory:")

0 commit comments

Comments
 (0)