Skip to content

Commit 0c67e59

Browse files
feat: structured {error, code} bodies across all api blueprints (#146)
* feat: structured {error, code} bodies across all API blueprints * fix: reject falsey non-dict bodies on validate-path * Address brad feedback * ci: exclude flaky composers-10 export bench from regression gate
1 parent bdd85fa commit 0c67e59

17 files changed

Lines changed: 636 additions & 246 deletions

api/composers.py

Lines changed: 31 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313

1414
from flask import Blueprint, Response
1515

16-
from api.flask_config import json_response
16+
from api.flask_config import api_error, json_response
1717

1818
from utils.workspace_path import resolve_workspace_path
1919
from utils.path_helpers import to_epoch_ms
@@ -28,6 +28,25 @@ def _read_json_file(path: str) -> Any:
2828
return json.load(f)
2929

3030

31+
def _parse_workspace_composer_rows(data: Any) -> list[Any]:
32+
"""Validate ``composer.composerData`` envelope and return ``allComposers``."""
33+
if not isinstance(data, dict):
34+
raise SchemaError(
35+
"WorkspaceComposers",
36+
"composer.composerData",
37+
hint=f"expected object, got {type(data).__name__}",
38+
)
39+
if "allComposers" not in data:
40+
raise SchemaError("WorkspaceComposers", "allComposers")
41+
all_composers = data.get("allComposers")
42+
if not isinstance(all_composers, list):
43+
raise SchemaError(
44+
"WorkspaceComposers",
45+
"allComposers",
46+
hint=f"expected list, got {type(all_composers).__name__}",
47+
)
48+
return all_composers
49+
3150
@bp.route("/api/composers")
3251
def list_composers() -> tuple[Response, int] | Response:
3352
"""List all composers across workspace databases (GET /api/composers).
@@ -67,22 +86,7 @@ def list_composers() -> tuple[Response, int] | Response:
6786
).fetchone()
6887

6988
if row and row[0]:
70-
data = json.loads(row[0])
71-
if not isinstance(data, dict):
72-
raise SchemaError(
73-
"WorkspaceComposers",
74-
"composer.composerData",
75-
hint=f"expected object, got {type(data).__name__}",
76-
)
77-
if "allComposers" not in data:
78-
raise SchemaError("WorkspaceComposers", "allComposers")
79-
all_composers = data.get("allComposers")
80-
if not isinstance(all_composers, list):
81-
raise SchemaError(
82-
"WorkspaceComposers",
83-
"allComposers",
84-
hint=f"expected list, got {type(all_composers).__name__}",
85-
)
89+
all_composers = _parse_workspace_composer_rows(json.loads(row[0]))
8690
for c in all_composers:
8791
try:
8892
local = WorkspaceLocalComposer.from_dict(c)
@@ -126,7 +130,7 @@ def list_composers() -> tuple[Response, int] | Response:
126130

127131
except Exception:
128132
_logger.exception("Failed to get composers")
129-
return json_response({"error": "Failed to get composers"}, 500)
133+
return api_error("Failed to get composers", "composers_list_failed", 500)
130134

131135

132136
@bp.route("/api/composers/<composer_id>")
@@ -139,9 +143,11 @@ def get_composer(composer_id: str) -> tuple[Response, int] | Response:
139143
Returns:
140144
Composer JSON from per-workspace storage or global fallback. Per-workspace
141145
schema drift is logged and skipped before global fallback is attempted.
142-
404 when the composer is absent from both stores (``{"error": "Composer not found"}``)
143-
or when the global row fails validation (``{"error": "Composer schema drift"}``).
144-
500 on unexpected failure.
146+
404 when the composer is absent from both stores
147+
(``{"error": "Composer not found", "code": "composer_not_found"}``)
148+
or when the global row fails validation
149+
(``{"error": "Composer schema drift", "code": "composer_schema_drift"}``).
150+
500 on unexpected failure (``{"error", "code"}`` via :func:`api_error`).
145151
"""
146152
try:
147153
workspace_path = resolve_workspace_path()
@@ -163,25 +169,7 @@ def get_composer(composer_id: str) -> tuple[Response, int] | Response:
163169
).fetchone()
164170

165171
if row and row[0]:
166-
data = json.loads(row[0])
167-
# Mirror the envelope guards list_composers() applies at line 60–74
168-
# so a drifted local row (data not a dict, or allComposers missing
169-
# / non-list) surfaces as a logged SchemaError, not a 500.
170-
if not isinstance(data, dict):
171-
raise SchemaError(
172-
"WorkspaceComposers",
173-
"composer.composerData",
174-
hint=f"expected object, got {type(data).__name__}",
175-
)
176-
if "allComposers" not in data:
177-
raise SchemaError("WorkspaceComposers", "allComposers")
178-
all_composers = data.get("allComposers")
179-
if not isinstance(all_composers, list):
180-
raise SchemaError(
181-
"WorkspaceComposers",
182-
"allComposers",
183-
hint=f"expected list, got {type(all_composers).__name__}",
184-
)
172+
all_composers = _parse_workspace_composer_rows(json.loads(row[0]))
185173
for c in all_composers:
186174
if isinstance(c, dict) and c.get("composerId") == composer_id:
187175
try:
@@ -240,14 +228,14 @@ def get_composer(composer_id: str) -> tuple[Response, int] | Response:
240228
e,
241229
type(e).__name__,
242230
)
243-
return json_response({"error": "Composer schema drift"}, 404)
231+
return api_error("Composer schema drift", "composer_schema_drift", 404)
244232
payload = composer.cursor_storage_payload()
245233
payload["conversation"] = payload.get("conversation") or []
246234
return json_response(payload)
247235
except (OSError, sqlite3.Error, json.JSONDecodeError, ValueError):
248236
pass
249237

250-
return json_response({"error": "Composer not found"}, 404)
238+
return api_error("Composer not found", "composer_not_found", 404)
251239
except Exception:
252240
_logger.exception("Failed to get composer")
253-
return json_response({"error": "Failed to get composer"}, 500)
241+
return api_error("Failed to get composer", "composer_get_failed", 500)

api/config_api.py

Lines changed: 66 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,54 @@
1313

1414
from flask import Blueprint, Response, request
1515

16-
from api.flask_config import json_response
16+
from api.flask_config import api_error, json_response
1717

1818
from utils.path_validation import WorkspacePathError, validate_workspace_path
1919
from utils.workspace_path import set_workspace_path_override
2020

2121
bp = Blueprint("config_api", __name__)
2222
_logger = logging.getLogger(__name__)
2323

24+
_WORKSPACE_PATH_ERROR_CODES: dict[str, str] = {
25+
"path is required": "path_required",
26+
"path does not exist": "path_not_found",
27+
"path is not a directory": "path_not_directory",
28+
(
29+
"path does not look like a Cursor workspaceStorage directory "
30+
"(no immediate subdirectory contains state.vscdb)"
31+
): "path_not_workspace_storage",
32+
}
33+
34+
35+
def _workspace_path_error_code(message: str) -> str:
36+
return _WORKSPACE_PATH_ERROR_CODES.get(message, "invalid_workspace_path")
37+
38+
39+
def _validate_path_error(
40+
message: str,
41+
code: str,
42+
) -> Response | tuple[Response, int]:
43+
return json_response(
44+
{"valid": False, "error": message, "code": code, "workspaceCount": 0},
45+
)
46+
47+
48+
def _parse_workspace_path_from_body(body: object) -> object | None:
49+
"""Return the raw ``path`` field when *body* is a JSON object; else ``None``."""
50+
if not isinstance(body, dict):
51+
return None
52+
path: object = body.get("path", "")
53+
return path
54+
55+
56+
def _canonicalize_workspace_path(raw: object) -> tuple[str | None, str | None]:
57+
"""Return ``(canonical, None)`` or ``(None, error_message)`` on validation failure."""
58+
try:
59+
# validate_workspace_path raises WorkspacePathError for non-str/empty input.
60+
return validate_workspace_path(raw), None # type: ignore[arg-type]
61+
except WorkspacePathError as e:
62+
return None, str(e)
63+
2464

2565
@bp.route("/api/detect-environment")
2666
def detect_environment() -> Response:
@@ -76,22 +116,21 @@ def validate_path() -> tuple[Response, int] | Response:
76116
JSON with ``valid``, ``workspaceCount``, and canonical ``path`` on success.
77117
``valid`` is ``false`` when the path fails validation or contains no
78118
workspace folders with ``state.vscdb``. Invalid JSON body returns
79-
``{"valid": false, "error": "invalid JSON body", "workspaceCount": 0}``.
80-
Path validation errors return ``{"valid": false, "error": "...", "workspaceCount": 0}``.
81-
500 with ``{"valid": false, "error": "Failed to validate path"}`` on
82-
unexpected failure.
119+
``{"valid": false, "error": "invalid JSON body", "code": "invalid_json_body", "workspaceCount": 0}``.
120+
Path validation errors return
121+
``{"valid": false, "error": "...", "code": "...", "workspaceCount": 0}``.
122+
500 with
123+
``{"valid": false, "error": "Failed to validate path", "code": "validate_path_failed", "workspaceCount": 0}``
124+
on unexpected failure.
83125
"""
84126
try:
85-
body = request.get_json(silent=True) or {}
86-
if not isinstance(body, dict):
87-
return json_response(
88-
{"valid": False, "error": "invalid JSON body", "workspaceCount": 0}
89-
)
90-
raw = body.get("path", "")
91-
try:
92-
canonical = validate_workspace_path(raw)
93-
except WorkspacePathError as e:
94-
return json_response({"valid": False, "error": str(e), "workspaceCount": 0})
127+
raw = _parse_workspace_path_from_body(request.get_json(silent=True))
128+
if raw is None:
129+
return _validate_path_error("invalid JSON body", "invalid_json_body")
130+
canonical, message = _canonicalize_workspace_path(raw)
131+
if message is not None:
132+
return _validate_path_error(message, _workspace_path_error_code(message))
133+
assert canonical is not None # paired with successful validation above
95134

96135
workspace_count = 0
97136
for name in os.listdir(canonical):
@@ -116,7 +155,10 @@ def validate_path() -> tuple[Response, int] | Response:
116155
type(e).__name__,
117156
exc_info=True,
118157
)
119-
return json_response({"valid": False, "error": "Failed to validate path"}, 500)
158+
return json_response(
159+
{"valid": False, "error": "Failed to validate path", "code": "validate_path_failed", "workspaceCount": 0},
160+
500,
161+
)
120162

121163

122164
@bp.route("/api/set-workspace", methods=["POST"])
@@ -138,24 +180,24 @@ def set_workspace() -> tuple[Response, int] | Response:
138180
# the outer Exception handler then mis-reports as a 500 server error
139181
# instead of a 400 client error. (CodeRabbit on PR #16.)
140182
body = request.get_json(silent=True)
141-
if not isinstance(body, dict):
142-
return json_response({"error": "request body must be a JSON object"}, 400)
143-
raw = body.get("path", "")
183+
raw = _parse_workspace_path_from_body(body)
184+
if raw is None:
185+
return api_error("request body must be a JSON object", "invalid_json_body", 400)
144186
# Validate the supplied path BEFORE storing the override (issue #15).
145187
# validate_workspace_path collapses `..` traversal AND resolves symlinks
146188
# via realpath, then enforces that the canonical target is an existing
147189
# directory containing Cursor workspace markers. Returns the canonical
148190
# path so we store that, not whatever the caller sent.
149191
try:
150-
canonical = validate_workspace_path(raw)
151-
except WorkspacePathError as e:
152-
return json_response({"error": str(e)}, 400)
192+
canonical, message = _canonicalize_workspace_path(raw)
153193
except Exception: # noqa: BLE001 — only here as a fallback
154-
return json_response({"error": "Failed to validate workspace path"}, 500)
194+
return api_error("Failed to validate workspace path", "validate_workspace_path_failed", 500)
195+
if message is not None:
196+
return api_error(message, _workspace_path_error_code(message), 400)
155197
try:
156198
set_workspace_path_override(canonical)
157199
except Exception: # noqa: BLE001 — keep the response shape structured JSON
158-
return json_response({"error": "Failed to set workspace path"}, 500)
200+
return api_error("Failed to set workspace path", "set_workspace_path_failed", 500)
159201
return json_response({"success": True, "path": canonical})
160202

161203

0 commit comments

Comments
 (0)