Skip to content

Commit b111439

Browse files
committed
review: wrap cursorDiskKV reads in _safe_fetchall (#31, round 4)
CodeRabbit flagged that the five ``global_db.execute(...).fetchall()`` calls in assemble_workspace_tabs could raise sqlite3.Error (e.g. a missing or corrupt cursorDiskKV table) and abort the whole tabs endpoint. Matches the resilience pattern already applied to _infer_workspace_name_from_context in round 3. Add a local ``_safe_fetchall(query, params=()) -> list`` helper inside assemble_workspace_tabs that catches sqlite3.Error and returns []. All five call sites — bubbleId LIKE, codeBlockDiff LIKE, two messageRequestContext LIKEs, and the composerData fetch — route through it. The route now returns ``{"tabs": []}, 200`` instead of 500-ing when the schema is corrupt. Regression: tests/test_workspace_tabs_sql_errors.py seeds a real globalStorage/state.vscdb without cursorDiskKV → asserts the response is ({"tabs": []}, 200) and no sqlite3.Error propagates. 196 tests pass (was 195; +1 new).
1 parent de43d22 commit b111439

2 files changed

Lines changed: 71 additions & 6 deletions

File tree

services/workspace_tabs.py

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import json
44
import os
55
import re
6+
import sqlite3
67
from datetime import datetime
78

89
from utils.path_helpers import (
@@ -83,8 +84,14 @@ def assemble_workspace_tabs(
8384

8485
workspace_display_name = _get_workspace_display_name(workspace_path, workspace_id)
8586

87+
def _safe_fetchall(query: str, params: tuple = ()) -> list:
88+
try:
89+
return global_db.execute(query, params).fetchall()
90+
except sqlite3.Error:
91+
return []
92+
8693
# Load bubbles
87-
for row in global_db.execute("SELECT key, value FROM cursorDiskKV WHERE key LIKE 'bubbleId:%'"):
94+
for row in _safe_fetchall("SELECT key, value FROM cursorDiskKV WHERE key LIKE 'bubbleId:%'"):
8895
parts = row["key"].split(":")
8996
if len(parts) >= 3:
9097
bid = parts[2]
@@ -96,7 +103,7 @@ def assemble_workspace_tabs(
96103
pass
97104

98105
# Load codeBlockDiffs
99-
for row in global_db.execute("SELECT key, value FROM cursorDiskKV WHERE key LIKE 'codeBlockDiff:%'"):
106+
for row in _safe_fetchall("SELECT key, value FROM cursorDiskKV WHERE key LIKE 'codeBlockDiff:%'"):
100107
chat_id = _extract_chat_id_from_code_block_diff_key(row["key"])
101108
if not chat_id:
102109
continue
@@ -110,7 +117,7 @@ def assemble_workspace_tabs(
110117
pass
111118

112119
# Load messageRequestContext
113-
for row in global_db.execute("SELECT key, value FROM cursorDiskKV WHERE key LIKE 'messageRequestContext:%'"):
120+
for row in _safe_fetchall("SELECT key, value FROM cursorDiskKV WHERE key LIKE 'messageRequestContext:%'"):
114121
parts = row["key"].split(":")
115122
if len(parts) >= 3:
116123
chat_id = parts[1]
@@ -126,7 +133,7 @@ def assemble_workspace_tabs(
126133

127134
# Build projectLayoutsMap
128135
project_layouts_map: dict[str, list] = {}
129-
for row in global_db.execute("SELECT key, value FROM cursorDiskKV WHERE key LIKE 'messageRequestContext:%'"):
136+
for row in _safe_fetchall("SELECT key, value FROM cursorDiskKV WHERE key LIKE 'messageRequestContext:%'"):
130137
parts = row["key"].split(":")
131138
if len(parts) >= 2:
132139
cid = parts[1]
@@ -147,11 +154,11 @@ def assemble_workspace_tabs(
147154
pass
148155

149156
# Get composer data entries with conversations
150-
composer_rows = global_db.execute(
157+
composer_rows = _safe_fetchall(
151158
"SELECT key, value FROM cursorDiskKV WHERE key LIKE 'composerData:%'"
152159
" AND value LIKE '%fullConversationHeadersOnly%'"
153160
" AND value NOT LIKE '%fullConversationHeadersOnly\":[]%'"
154-
).fetchall()
161+
)
155162

156163
invalid_workspace_aliases = _infer_invalid_workspace_aliases(
157164
composer_rows=composer_rows,
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
from __future__ import annotations
2+
3+
import json
4+
import os
5+
import sqlite3
6+
import sys
7+
import tempfile
8+
import unittest
9+
10+
from flask import Flask
11+
12+
REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
13+
if REPO_ROOT not in sys.path:
14+
sys.path.insert(0, REPO_ROOT)
15+
16+
from services.workspace_tabs import assemble_workspace_tabs
17+
18+
19+
def _seed_workspace_with_corrupt_global_db(parent: str) -> str:
20+
ws_root = os.path.join(parent, "workspaceStorage")
21+
global_root = os.path.join(parent, "globalStorage")
22+
os.makedirs(ws_root, exist_ok=True)
23+
os.makedirs(global_root, exist_ok=True)
24+
ws_dir = os.path.join(ws_root, "ws-a")
25+
os.makedirs(ws_dir, exist_ok=True)
26+
with open(os.path.join(ws_dir, "workspace.json"), "w") as f:
27+
json.dump({"folder": "/tmp/proj"}, f)
28+
sqlite3.connect(os.path.join(ws_dir, "state.vscdb")).close()
29+
30+
# Global DB exists but cursorDiskKV is missing → every LIKE query
31+
# in assemble_workspace_tabs raises sqlite3.OperationalError.
32+
conn = sqlite3.connect(os.path.join(global_root, "state.vscdb"))
33+
conn.execute("CREATE TABLE other (x INTEGER)")
34+
conn.commit()
35+
conn.close()
36+
return ws_root
37+
38+
39+
class TestCursorDiskKvCorruptDoesNot500(unittest.TestCase):
40+
def test_missing_cursordiskkv_returns_empty_tabs_not_error(self) -> None:
41+
app = Flask(__name__)
42+
app.config["TESTING"] = True
43+
app.config["EXCLUSION_RULES"] = []
44+
45+
with tempfile.TemporaryDirectory() as tmp:
46+
ws_root = _seed_workspace_with_corrupt_global_db(tmp)
47+
with app.test_request_context("/api/workspaces/global/tabs"):
48+
try:
49+
payload, status = assemble_workspace_tabs("global", ws_root, rules=[])
50+
except sqlite3.Error:
51+
self.fail("sqlite3.Error should be caught, not propagated")
52+
53+
self.assertEqual(status, 200)
54+
self.assertEqual(payload, {"tabs": []})
55+
56+
57+
if __name__ == "__main__":
58+
unittest.main()

0 commit comments

Comments
 (0)