Skip to content

Commit fbec114

Browse files
fix: PR #42 review feedback (CLI timeout, search asserts, error codes, exceptions)
1 parent 6bb2245 commit fbec114

8 files changed

Lines changed: 51 additions & 19 deletions

File tree

api/error_codes.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@ def error_response(
2424
status: int,
2525
**extra: object,
2626
) -> tuple[Response, int]:
27-
body: dict[str, object] = {"error": message, "code": str(code)}
28-
body.update(extra)
27+
body: dict[str, object] = {"error": message, "code": code}
28+
reserved = frozenset({"error", "code"})
29+
for key, value in extra.items():
30+
if key not in reserved:
31+
body[key] = value
2932
return jsonify(body), status

api/export_api.py

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,14 @@
3333
# Tests monkeypatch this path; keep in sync with utils.export_state_store.
3434
_STATE_FILE = EXPORT_STATE_FILE
3535

36+
_EXPORT_ERRORS = (
37+
json.JSONDecodeError,
38+
KeyError,
39+
ValueError,
40+
OSError,
41+
FileNotFoundError,
42+
)
43+
3644

3745
def _state_lock():
3846
return export_state_lock(_STATE_FILE)
@@ -159,7 +167,7 @@ def bulk_export():
159167
)
160168
new_sessions_map[sid] = sess_info.get("modified", 0)
161169
count += 1
162-
except Exception as e:
170+
except _EXPORT_ERRORS as e:
163171
current_app.logger.warning(
164172
"Failed to export %s: %s", sid[:10], e
165173
)
@@ -221,7 +229,7 @@ def bulk_export():
221229
)
222230
new_sessions_map[sid] = sess_info.get("modified", 0)
223231
count += 1
224-
except Exception as e:
232+
except _EXPORT_ERRORS as e:
225233
current_app.logger.warning(
226234
"Failed to export %s: %s", sid[:10], e
227235
)
@@ -232,9 +240,6 @@ def bulk_export():
232240
)
233241
zf.writestr("manifest.jsonl", manifest_str)
234242

235-
if count > 0:
236-
_write_state(new_sessions_map, count)
237-
238243
if count == 0:
239244
return error_response(
240245
ErrorCode.EXPORT_NOTHING_TO_EXPORT,
@@ -243,6 +248,8 @@ def bulk_export():
243248
since=since,
244249
)
245250

251+
_write_state(new_sessions_map, count)
252+
246253
buf.seek(0)
247254
date_tag = datetime.now().strftime("%Y-%m-%d")
248255
if since == "last":
@@ -286,7 +293,7 @@ def export_session(project_name, session_id):
286293
fmt = request.args.get("format", "md")
287294
try:
288295
session = parse_session(filepath)
289-
except Exception:
296+
except _EXPORT_ERRORS:
290297
current_app.logger.exception(
291298
"Failed to parse session %s for export", session_id
292299
)
@@ -306,7 +313,7 @@ def export_session(project_name, session_id):
306313

307314
try:
308315
stats = compute_stats(session)
309-
except Exception:
316+
except _EXPORT_ERRORS:
310317
current_app.logger.exception(
311318
"Failed to compute stats for export %s", session_id
312319
)

api/sessions.py

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
"""Session detail and stats endpoints."""
22

3+
import json
34
import os
45

56
from flask import Blueprint, current_app, jsonify
@@ -12,6 +13,14 @@
1213

1314
sessions_bp = Blueprint("sessions", __name__)
1415

16+
_PARSE_ERRORS = (
17+
json.JSONDecodeError,
18+
KeyError,
19+
ValueError,
20+
OSError,
21+
FileNotFoundError,
22+
)
23+
1524

1625
@sessions_bp.route("/api/sessions/<path:project_name>/<session_id>")
1726
def get_session(project_name, session_id):
@@ -38,7 +47,7 @@ def get_session(project_name, session_id):
3847
404,
3948
)
4049
return jsonify(session)
41-
except Exception:
50+
except _PARSE_ERRORS:
4251
current_app.logger.exception("Failed to parse session %s", session_id)
4352
return error_response(
4453
ErrorCode.PARSE_ERROR,
@@ -64,7 +73,7 @@ def get_session_stats(project_name, session_id):
6473

6574
try:
6675
session = parse_session(filepath)
67-
except Exception:
76+
except _PARSE_ERRORS:
6877
current_app.logger.exception("Failed to parse session %s", session_id)
6978
return error_response(
7079
ErrorCode.PARSE_ERROR,
@@ -75,7 +84,7 @@ def get_session_stats(project_name, session_id):
7584
try:
7685
stats = compute_stats(session)
7786
return jsonify(stats)
78-
except Exception:
87+
except _PARSE_ERRORS:
7988
current_app.logger.exception("Failed to compute stats for %s", session_id)
8089
return error_response(
8190
ErrorCode.INTERNAL_ERROR,

pyproject.toml

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,8 @@ addopts = "--cov=api --cov=utils --cov-report=term-missing --cov-report=xml:cove
33
testpaths = ["tests"]
44

55
[tool.coverage.run]
6-
# Exclude untested export/stats modules until Week 3+ adds coverage (see issue #4 follow-up).
76
omit = [
87
"tests/*",
9-
"utils/md_exporter.py",
10-
"utils/session_stats.py",
11-
"utils/json_exporter.py",
128
]
139

1410
[tool.coverage.report]

static/js/shared/markdown.test.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,4 +44,13 @@ describe('renderMarkdown', () => {
4444
const html = renderMarkdown('`code`');
4545
expect(html).toBe('<code>code</code>');
4646
});
47+
48+
it('falls back to escaped output when DOMPurify is unavailable', () => {
49+
delete globalThis.DOMPurify;
50+
const html = renderMarkdown('Hello **world**');
51+
expect(html).toBeDefined();
52+
expect(html).toContain('Hello');
53+
expect(html).toContain('**world**');
54+
expect(html).not.toMatch(/<script/i);
55+
});
4756
});

tests/test_cli_e2e.py

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,12 @@ def _cli_env() -> dict[str, str]:
2020
return env
2121

2222

23-
def _run_cli(argv: list[str], *, env: dict | None = None) -> subprocess.CompletedProcess:
23+
def _run_cli(
24+
argv: list[str],
25+
*,
26+
env: dict | None = None,
27+
timeout: float = 60.0,
28+
) -> subprocess.CompletedProcess:
2429
cmd = [sys.executable, str(EXPORT_SCRIPT), *argv]
2530
merged = _cli_env()
2631
if env:
@@ -33,6 +38,7 @@ def _run_cli(argv: list[str], *, env: dict | None = None) -> subprocess.Complete
3338
env=merged,
3439
encoding="utf-8",
3540
errors="replace",
41+
timeout=timeout,
3642
)
3743

3844

@@ -53,7 +59,8 @@ def test_cli_list_exits_zero(tmp_path):
5359
assert "test-project" in proc.stdout.lower()
5460

5561

56-
def test_cli_list_unknown_project_exits_zero_with_message(tmp_path):
62+
def test_cli_list_nonmatching_project_filter_prints_no_projects(tmp_path):
63+
"""--project filters by substring; zero matches prints 'No projects found.'"""
5764
base = _seed_base_dir(tmp_path)
5865
proc = _run_cli(["list", "--base-dir", str(base), "--project", "does-not-exist"])
5966
assert proc.returncode == 0

tests/test_error_codes.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ def test_error_codes_on_endpoints(client, method, path, kwargs, status, code):
5252
fn = getattr(client, method)
5353
resp = fn(path, **kwargs)
5454
assert resp.status_code == status
55-
assert_error_response(resp, expected_code=str(code))
55+
assert_error_response(resp, expected_code=code)
5656

5757

5858
def test_bulk_export_empty_includes_export_nothing_code(client_empty):

tests/test_search.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ def test_limit_default(client_single):
5252
def test_limit_whitespace_defaults(client_single):
5353
resp = client_single.get("/api/search?q=Hello&limit=%20%20%20")
5454
assert resp.status_code == 200
55+
_assert_search_hits(resp.get_json(), max_items=50)
5556

5657

5758
def test_limit_zero(client_single):

0 commit comments

Comments
 (0)