Skip to content

Commit d5d13a7

Browse files
committed
fix: prevent hang when store-configured query returns zero rows
When a query configured with Store returns an empty result set, the server correctly responds with state=succeeded and result_uri=None. Previously, the driver misinterpreted this as a non-store query and called __request_results(), which returned null results that were silently dropped — leaving the cursor's queue.get() blocking forever. Two fixes applied: 1. Primary fix: In the SUCCEEDED+STATE_UPDATED path, check query.store before falling through to __request_results(). When store is configured but result_uri is falsy, immediately complete with an empty ExecutionResult. 2. Defensive fix: In the execution_result handler, when results is falsy or not a dict, deliver an empty ExecutionResult instead of silently returning. This ensures the cursor is always unblocked. Also adds comprehensive tests covering empty store results, normal store results, non-store queries, null execution results, and empty dict execution results.
1 parent 65a8711 commit d5d13a7

File tree

4 files changed

+244
-3
lines changed

4 files changed

+244
-3
lines changed

pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[project]
22
name = "wherobots-python-dbapi"
3-
version = "0.25.0"
3+
version = "0.25.1"
44
description = "Python DB-API driver for Wherobots DB"
55
authors = [{ name = "Maxime Petazzoni", email = "max@wherobots.com" }]
66
requires-python = ">=3.10, <4"

tests/test_empty_store_results.py

Lines changed: 229 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,229 @@
1+
"""Tests for the empty store results fix.
2+
3+
These tests verify that:
4+
1. A store-configured query with empty results (result_uri=null) completes
5+
promptly instead of hanging.
6+
2. An execution_result event with results=null unblocks the cursor instead
7+
of hanging forever.
8+
"""
9+
10+
import json
11+
import queue
12+
import threading
13+
from unittest.mock import MagicMock, patch
14+
15+
import pytest
16+
17+
from wherobots.db.connection import Connection, Query
18+
from wherobots.db.cursor import Cursor
19+
from wherobots.db.models import ExecutionResult, Store, StoreResult
20+
from wherobots.db.types import EventKind, ExecutionState, StorageFormat
21+
22+
23+
class TestEmptyStoreResults:
24+
"""Tests for the primary fix: store configured, result_uri is null."""
25+
26+
def _make_connection_and_cursor(self):
27+
"""Create a Connection with a mocked WebSocket and return (connection, cursor)."""
28+
mock_ws = MagicMock()
29+
# Prevent the background thread from running the main loop
30+
mock_ws.protocol.state = 4 # CLOSED state, so __main_loop exits immediately
31+
conn = Connection(mock_ws)
32+
cursor = conn.cursor()
33+
return conn, cursor
34+
35+
def test_store_configured_empty_results_completes(self):
36+
"""When a store-configured query succeeds with result_uri=null,
37+
the cursor should receive an empty ExecutionResult (not hang)."""
38+
result_queue = queue.Queue()
39+
40+
def handler(result):
41+
result_queue.put(result)
42+
43+
store = Store.for_download(format=StorageFormat.GEOJSON)
44+
query = Query(
45+
sql="SELECT * FROM t WHERE 1=0",
46+
execution_id="exec-1",
47+
state=ExecutionState.RUNNING,
48+
handler=handler,
49+
store=store,
50+
)
51+
52+
conn, _ = self._make_connection_and_cursor()
53+
conn._Connection__queries["exec-1"] = query
54+
55+
# Simulate receiving a state_updated message with result_uri=null
56+
message = {
57+
"kind": "state_updated",
58+
"execution_id": "exec-1",
59+
"state": "succeeded",
60+
"result_uri": None,
61+
"size": None,
62+
}
63+
conn._Connection__ws.recv.return_value = json.dumps(message)
64+
conn._Connection__listen()
65+
66+
# The handler should have been called
67+
result = result_queue.get(timeout=1)
68+
assert isinstance(result, ExecutionResult)
69+
assert result.results is None
70+
assert result.error is None
71+
assert result.store_result is None
72+
assert query.state == ExecutionState.COMPLETED
73+
74+
def test_store_configured_with_result_uri_produces_store_result(self):
75+
"""When a store-configured query succeeds with result_uri set,
76+
the cursor should receive an ExecutionResult with store_result."""
77+
result_queue = queue.Queue()
78+
79+
def handler(result):
80+
result_queue.put(result)
81+
82+
store = Store.for_download(format=StorageFormat.GEOJSON)
83+
query = Query(
84+
sql="SELECT * FROM t",
85+
execution_id="exec-2",
86+
state=ExecutionState.RUNNING,
87+
handler=handler,
88+
store=store,
89+
)
90+
91+
conn, _ = self._make_connection_and_cursor()
92+
conn._Connection__queries["exec-2"] = query
93+
94+
message = {
95+
"kind": "state_updated",
96+
"execution_id": "exec-2",
97+
"state": "succeeded",
98+
"result_uri": "https://presigned-url.example.com/results.geojson",
99+
"size": 12345,
100+
}
101+
conn._Connection__ws.recv.return_value = json.dumps(message)
102+
conn._Connection__listen()
103+
104+
result = result_queue.get(timeout=1)
105+
assert isinstance(result, ExecutionResult)
106+
assert result.results is None
107+
assert result.error is None
108+
assert result.store_result is not None
109+
assert result.store_result.result_uri == "https://presigned-url.example.com/results.geojson"
110+
assert result.store_result.size == 12345
111+
assert query.state == ExecutionState.COMPLETED
112+
113+
def test_no_store_configured_calls_request_results(self):
114+
"""When no store is configured and result_uri is null,
115+
__request_results should be called (existing behavior)."""
116+
result_queue = queue.Queue()
117+
118+
def handler(result):
119+
result_queue.put(result)
120+
121+
query = Query(
122+
sql="SELECT 1",
123+
execution_id="exec-3",
124+
state=ExecutionState.RUNNING,
125+
handler=handler,
126+
store=None, # No store configured
127+
)
128+
129+
conn, _ = self._make_connection_and_cursor()
130+
conn._Connection__queries["exec-3"] = query
131+
132+
message = {
133+
"kind": "state_updated",
134+
"execution_id": "exec-3",
135+
"state": "succeeded",
136+
"result_uri": None,
137+
"size": None,
138+
}
139+
conn._Connection__ws.recv.return_value = json.dumps(message)
140+
141+
with patch.object(conn, "_Connection__request_results") as mock_request:
142+
conn._Connection__listen()
143+
mock_request.assert_called_once_with("exec-3")
144+
145+
# Handler should NOT have been called (waiting for execution_result)
146+
assert result_queue.empty()
147+
148+
149+
class TestDefensiveNullResults:
150+
"""Tests for the defensive fix: execution_result with results=null."""
151+
152+
def _make_connection_and_cursor(self):
153+
mock_ws = MagicMock()
154+
mock_ws.protocol.state = 4
155+
conn = Connection(mock_ws)
156+
cursor = conn.cursor()
157+
return conn, cursor
158+
159+
def test_null_results_in_execution_result_unblocks_cursor(self):
160+
"""When execution_result arrives with results=null,
161+
the cursor should be unblocked with an empty ExecutionResult."""
162+
result_queue = queue.Queue()
163+
164+
def handler(result):
165+
result_queue.put(result)
166+
167+
query = Query(
168+
sql="SELECT 1",
169+
execution_id="exec-4",
170+
state=ExecutionState.RESULTS_REQUESTED,
171+
handler=handler,
172+
store=None,
173+
)
174+
175+
conn, _ = self._make_connection_and_cursor()
176+
conn._Connection__queries["exec-4"] = query
177+
178+
# Simulate execution_result with results=null
179+
# (this is what the server sends for store-only executions
180+
# when retrieve_results is erroneously called)
181+
message = {
182+
"kind": "execution_result",
183+
"execution_id": "exec-4",
184+
"state": "succeeded",
185+
"results": None,
186+
}
187+
conn._Connection__ws.recv.return_value = json.dumps(message)
188+
conn._Connection__listen()
189+
190+
result = result_queue.get(timeout=1)
191+
assert isinstance(result, ExecutionResult)
192+
assert result.results is None
193+
assert result.error is None
194+
assert result.store_result is None
195+
assert query.state == ExecutionState.COMPLETED
196+
197+
def test_empty_dict_results_in_execution_result_unblocks_cursor(self):
198+
"""When execution_result arrives with results={} (empty dict),
199+
the cursor should be unblocked with an empty ExecutionResult."""
200+
result_queue = queue.Queue()
201+
202+
def handler(result):
203+
result_queue.put(result)
204+
205+
query = Query(
206+
sql="SELECT 1",
207+
execution_id="exec-5",
208+
state=ExecutionState.RESULTS_REQUESTED,
209+
handler=handler,
210+
store=None,
211+
)
212+
213+
conn, _ = self._make_connection_and_cursor()
214+
conn._Connection__queries["exec-5"] = query
215+
216+
message = {
217+
"kind": "execution_result",
218+
"execution_id": "exec-5",
219+
"state": "succeeded",
220+
"results": {},
221+
}
222+
conn._Connection__ws.recv.return_value = json.dumps(message)
223+
conn._Connection__listen()
224+
225+
result = result_queue.get(timeout=1)
226+
assert isinstance(result, ExecutionResult)
227+
assert result.results is None
228+
assert result.error is None
229+
assert query.state == ExecutionState.COMPLETED

uv.lock

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

wherobots/db/connection.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,16 @@ def __listen(self) -> None:
192192
query.handler(ExecutionResult(store_result=store_result))
193193
return
194194

195+
if query.store is not None:
196+
# Store was configured but produced no results (empty result set)
197+
logging.info(
198+
"Query %s completed with store configured but no results to store.",
199+
execution_id,
200+
)
201+
query.state = ExecutionState.COMPLETED
202+
query.handler(ExecutionResult())
203+
return
204+
195205
# No store configured, request results normally
196206
self.__request_results(execution_id)
197207
return
@@ -200,6 +210,8 @@ def __listen(self) -> None:
200210
results = message.get("results")
201211
if not results or not isinstance(results, dict):
202212
logging.warning("Got no results back from %s.", execution_id)
213+
query.state = ExecutionState.COMPLETED
214+
query.handler(ExecutionResult())
203215
return
204216

205217
query.state = ExecutionState.COMPLETED

0 commit comments

Comments
 (0)