Skip to content

Commit 5daf230

Browse files
fix: PR #42 review feedback (CLI timeout, search asserts, error codes, exceptions)
1 parent c2f1616 commit 5daf230

7 files changed

Lines changed: 44 additions & 14 deletions

File tree

api/export_api.py

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,14 @@
3636
# Tests monkeypatch this path; keep in sync with utils.export_state_store.
3737
_STATE_FILE = EXPORT_STATE_FILE
3838

39+
_EXPORT_ERRORS = (
40+
json.JSONDecodeError,
41+
KeyError,
42+
ValueError,
43+
OSError,
44+
FileNotFoundError,
45+
)
46+
3947

4048
def _state_lock() -> Any:
4149
return export_state_lock(_STATE_FILE)
@@ -162,7 +170,7 @@ def bulk_export() -> FlaskReturn:
162170
)
163171
new_sessions_map[sid] = sess_info.get("modified", 0)
164172
count += 1
165-
except Exception as e:
173+
except _EXPORT_ERRORS as e:
166174
current_app.logger.warning(
167175
"Failed to export %s: %s", sid[:10], e
168176
)
@@ -224,7 +232,7 @@ def bulk_export() -> FlaskReturn:
224232
)
225233
new_sessions_map[sid] = sess_info.get("modified", 0)
226234
count += 1
227-
except Exception as e:
235+
except _EXPORT_ERRORS as e:
228236
current_app.logger.warning(
229237
"Failed to export %s: %s", sid[:10], e
230238
)
@@ -287,7 +295,7 @@ def export_session(project_name: str, session_id: str) -> FlaskReturn:
287295
fmt = request.args.get("format", "md")
288296
try:
289297
session = parse_session(filepath)
290-
except Exception:
298+
except _EXPORT_ERRORS:
291299
current_app.logger.exception(
292300
"Failed to parse session %s for export", session_id
293301
)
@@ -307,7 +315,7 @@ def export_session(project_name: str, session_id: str) -> FlaskReturn:
307315

308316
try:
309317
stats = compute_stats(session)
310-
except Exception:
318+
except _EXPORT_ERRORS:
311319
current_app.logger.exception(
312320
"Failed to compute stats for export %s", session_id
313321
)

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
@@ -13,6 +14,14 @@
1314

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

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

1726
@sessions_bp.route("/api/sessions/<path:project_name>/<session_id>")
1827
def get_session(project_name: str, session_id: str) -> FlaskReturn:
@@ -39,7 +48,7 @@ def get_session(project_name: str, session_id: str) -> FlaskReturn:
3948
404,
4049
)
4150
return json_response(session)
42-
except Exception:
51+
except _PARSE_ERRORS:
4352
current_app.logger.exception("Failed to parse session %s", session_id)
4453
return error_response(
4554
ErrorCode.PARSE_ERROR,
@@ -65,7 +74,7 @@ def get_session_stats(project_name: str, session_id: str) -> FlaskReturn:
6574

6675
try:
6776
session = parse_session(filepath)
68-
except Exception:
77+
except _PARSE_ERRORS:
6978
current_app.logger.exception("Failed to parse session %s", session_id)
7079
return error_response(
7180
ErrorCode.PARSE_ERROR,
@@ -84,7 +93,7 @@ def get_session_stats(project_name: str, session_id: str) -> FlaskReturn:
8493
try:
8594
stats = compute_stats(session)
8695
return json_response(stats)
87-
except Exception:
96+
except _PARSE_ERRORS:
8897
current_app.logger.exception("Failed to compute stats for %s", session_id)
8998
return error_response(
9099
ErrorCode.INTERNAL_ERROR,

pyproject.toml

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

1111
[tool.coverage.run]
12-
# Exclude untested export/stats modules until Week 3+ adds coverage (see issue #4 follow-up).
1312
omit = [
1413
"tests/*",
15-
"utils/md_exporter.py",
16-
"utils/session_stats.py",
17-
"utils/json_exporter.py",
1814
]
1915

2016
[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)