diff --git a/api/error_codes.py b/api/error_codes.py index 84da608..f0dee83 100644 --- a/api/error_codes.py +++ b/api/error_codes.py @@ -1,21 +1,12 @@ -"""Stable machine-readable error codes for API JSON error responses.""" +"""HTTP error envelope helpers; :class:`ErrorCode` lives in :mod:`models.error_codes`.""" from __future__ import annotations -from enum import StrEnum - from flask import Response, jsonify +from models.error_codes import ErrorCode -class ErrorCode(StrEnum): - SEARCH_INVALID_LIMIT = "SEARCH_INVALID_LIMIT" - INVALID_PATH = "INVALID_PATH" - SESSION_NOT_FOUND = "SESSION_NOT_FOUND" - INVALID_REQUEST_BODY = "INVALID_REQUEST_BODY" - INVALID_SINCE_MODE = "INVALID_SINCE_MODE" - PARSE_ERROR = "PARSE_ERROR" - EXPORT_NOTHING_TO_EXPORT = "EXPORT_NOTHING_TO_EXPORT" - INTERNAL_ERROR = "INTERNAL_ERROR" +__all__ = ["ErrorCode", "error_response"] def error_response( diff --git a/api/export_api.py b/api/export_api.py index a0d9377..92e97b4 100644 --- a/api/export_api.py +++ b/api/export_api.py @@ -1,6 +1,7 @@ """Export endpoints -- bulk zip download and single-session md/json.""" import io +import json import os import zipfile from datetime import datetime @@ -12,7 +13,12 @@ from api.error_codes import ErrorCode, error_response from models.export import ExportStateDict from utils.exclusion_rules import is_session_excluded -from utils.export_engine import EXPORT_ERRORS as _EXPORT_ERRORS, ZipSink, run_bulk_export +from utils.export_engine import ( + EXPORT_ERRORS as _EXPORT_ERRORS, + ExportFailure, + ZipSink, + run_bulk_export, +) from utils.export_state_store import ( EXPORT_STATE_FILE, atomic_write_export_state, @@ -31,6 +37,10 @@ # Tests monkeypatch this path; keep in sync with utils.export_state_store. _STATE_FILE = EXPORT_STATE_FILE +_EXPORT_WARNINGS_ZIP_NAME = "export-warnings.json" +_EXPORT_WARNINGS_HEADER_MAX_ENTRIES = 20 +_EXPORT_WARNINGS_HEADER_MAX_BYTES = 8192 + def _state_lock() -> Any: return export_state_lock(_STATE_FILE) @@ -49,6 +59,42 @@ def _read_state() -> ExportStateDict: return _load_state_from_disk() +def _serialize_export_failures(failures: list[ExportFailure]) -> list[dict[str, object]]: + return [ + { + "session_id": item.session_id, + "code": str(item.code), + "message": item.message, + } + for item in failures + ] + + +def _export_warnings_header_payload( + failures: list[ExportFailure], +) -> dict[str, object]: + """Bounded summary for X-Export-Warnings; full list lives in export-warnings.json.""" + entries = _serialize_export_failures(failures) + total = len(entries) + sample = entries[:_EXPORT_WARNINGS_HEADER_MAX_ENTRIES] + truncated = total > len(sample) + payload: dict[str, object] = { + "total_failures": total, + "truncated": truncated, + "failures": sample, + } + while ( + len(json.dumps(payload, separators=(",", ":"))) > _EXPORT_WARNINGS_HEADER_MAX_BYTES + and len(sample) > 1 + ): + sample = sample[:-1] + truncated = True + payload = {"total_failures": total, "truncated": truncated, "failures": sample} + if len(json.dumps(payload, separators=(",", ":"))) > _EXPORT_WARNINGS_HEADER_MAX_BYTES: + payload = {"total_failures": total, "truncated": True, "failures": []} + return payload + + def _write_state(sessions_map: dict[str, float], count: int) -> None: """Persist merge of *sessions_map* and update last-export metadata (*count* = this run only).""" with _state_lock(): @@ -119,12 +165,27 @@ def _on_export_error(sid: str, exc: Exception) -> None: manifest_style="api", on_export_error=_on_export_error, ) + if result.failures and result.exported_session_count > 0: + full_warnings = _serialize_export_failures(result.failures) + zf.writestr( + _EXPORT_WARNINGS_ZIP_NAME, + json.dumps(full_warnings, separators=(",", ":")) + "\n", + ) count = result.exported_session_count new_sessions_map = result.new_sessions_map latest_day = result.latest_day + failure_payload = _serialize_export_failures(result.failures) if count == 0: + if result.failures: + return error_response( + ErrorCode.EXPORT_ALL_FAILED, + "All export candidates failed", + 422, + since=since, + failures=failure_payload, + ) return error_response( ErrorCode.EXPORT_NOTHING_TO_EXPORT, "Nothing to export", @@ -145,12 +206,18 @@ def _on_export_error(sid: str, exc: Exception) -> None: suffix = "-incremental" else: suffix = "" - return send_file( + resp = send_file( buf, mimetype="application/zip", as_attachment=True, download_name=f"claude-code-export{suffix}-{date_tag}.zip", # type: ignore[call-arg] ) + if result.failures: + resp.headers["X-Export-Warnings"] = json.dumps( + _export_warnings_header_payload(result.failures), + separators=(",", ":"), + ) + return resp @export_bp.route("/api/export/session//") diff --git a/docs/api-reference.md b/docs/api-reference.md index 46b1cf9..d979cf3 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -62,6 +62,7 @@ Extra fields may appear for specific codes (for example `since` on invalid bulk- | `INVALID_SINCE_MODE` | 400 | `POST /api/export` | `since` is not `all`, `last`, or `incremental` | | `PARSE_ERROR` | 500 | Session, stats, export session | JSONL file could not be parsed | | `EXPORT_NOTHING_TO_EXPORT` | 422 | `POST /api/export` | No sessions matched the requested slice | +| `EXPORT_ALL_FAILED` | 422 | `POST /api/export` | At least one session was attempted but every candidate failed | | `INTERNAL_ERROR` | 500 | `GET .../stats`, export session | Unexpected failure after parse (e.g. stats computation) | --- @@ -372,13 +373,23 @@ Filename pattern: Zip contains Markdown per session and optional `manifest.jsonl` metadata. +When some sessions fail but at least one succeeds, the response is still **`200`** with the ZIP body (successful sessions only). Skipped sessions are surfaced two ways: + +| Channel | When | Value | +|---------|------|--------| +| `X-Export-Warnings` header | Partial export (≥1 success, ≥1 failure) | JSON object: `{ "total_failures", "truncated", "failures" }` where `failures` is a capped sample | +| `export-warnings.json` in the ZIP | Same | Full array of `{ "session_id", "code", "message" }` | + +`message` is a stable generic string per `code` (no exception text or paths). `code` uses the same strings as the error catalog (`PARSE_ERROR`, `INTERNAL_ERROR`, etc.). + #### Errors | Status | `code` | When | Extra fields | |--------|--------|------|--------------| | 400 | `INVALID_REQUEST_BODY` | Body is not a JSON object | — | | 400 | `INVALID_SINCE_MODE` | Invalid `since` value | `since` echoes rejected value | -| 422 | `EXPORT_NOTHING_TO_EXPORT` | Zero sessions matched | `since` echoes request mode | +| 422 | `EXPORT_NOTHING_TO_EXPORT` | Zero sessions matched (none attempted) | `since` echoes request mode | +| 422 | `EXPORT_ALL_FAILED` | Candidates existed but every attempted session failed | `since`, `failures` — flat array of `{"session_id", "code", "message"}` objects (same item shape as the `failures` array inside the `X-Export-Warnings` header) | ```bash curl -X POST -H "Content-Type: application/json" \ diff --git a/models/error_codes.py b/models/error_codes.py new file mode 100644 index 0000000..1d81781 --- /dev/null +++ b/models/error_codes.py @@ -0,0 +1,17 @@ +"""Stable machine-readable error codes (shared by API and utils; no Flask dependency).""" + +from __future__ import annotations + +from enum import StrEnum + + +class ErrorCode(StrEnum): + SEARCH_INVALID_LIMIT = "SEARCH_INVALID_LIMIT" + INVALID_PATH = "INVALID_PATH" + SESSION_NOT_FOUND = "SESSION_NOT_FOUND" + INVALID_REQUEST_BODY = "INVALID_REQUEST_BODY" + INVALID_SINCE_MODE = "INVALID_SINCE_MODE" + PARSE_ERROR = "PARSE_ERROR" + EXPORT_NOTHING_TO_EXPORT = "EXPORT_NOTHING_TO_EXPORT" + EXPORT_ALL_FAILED = "EXPORT_ALL_FAILED" + INTERNAL_ERROR = "INTERNAL_ERROR" diff --git a/tests/test_cli_export_exit_codes.py b/tests/test_cli_export_exit_codes.py index 224021b..ae23ef1 100644 --- a/tests/test_cli_export_exit_codes.py +++ b/tests/test_cli_export_exit_codes.py @@ -13,8 +13,9 @@ sys.path.insert(0, str(REPO_ROOT)) import scripts.export as export +from models.error_codes import ErrorCode from tests.test_cli_e2e import _run_cli, _seed_base_dir -from utils.export_engine import BulkExportResult +from utils.export_engine import BulkExportResult, ExportFailure from utils.jsonl_parser import parse_session _SUMMARY_RE = re.compile( @@ -133,8 +134,17 @@ def _track_exit(result: BulkExportResult) -> None: def test_since_last_early_return_exits_one_on_failure(tmp_path, monkeypatch, capsys): - """Since-last early-return with failure_count>0 must produce real exit code 1.""" - fake_result = BulkExportResult(latest_day=None, failure_count=1) + """Since-last early-return with failures must produce real exit code 1.""" + fake_result = BulkExportResult( + latest_day=None, + failures=[ + ExportFailure( + session_id="session_fail", + message="Failed to parse session", + code=ErrorCode.PARSE_ERROR, + ) + ], + ) monkeypatch.setattr(export, "run_bulk_export", lambda **kwargs: fake_result) monkeypatch.setattr(export, "list_projects", lambda base: [{"name": "p", "path": "/p"}]) diff --git a/tests/test_export_api_bulk.py b/tests/test_export_api_bulk.py index 6cd5ef7..716087b 100644 --- a/tests/test_export_api_bulk.py +++ b/tests/test_export_api_bulk.py @@ -2,8 +2,10 @@ from __future__ import annotations +import io import json import sys +import zipfile from pathlib import Path import pytest @@ -13,7 +15,10 @@ from flask import Flask -from api.export_api import export_bp +from api.error_codes import ErrorCode +from api.export_api import _export_warnings_header_payload, export_bp +from utils.export_engine import ExportFailure +from utils.jsonl_parser import parse_session @pytest.fixture @@ -69,6 +74,132 @@ def test_bulk_export_empty_returns_422_json(isolated_state, tmp_path): assert body["since"] == "all" +def test_bulk_export_all_succeed_no_warnings_header(client): + resp = client.post("/api/export", json={"since": "all"}) + assert resp.status_code == 200 + assert resp.content_type.startswith("application/zip") + assert "X-Export-Warnings" not in resp.headers + zf = zipfile.ZipFile(io.BytesIO(resp.data)) + md_files = [name for name in zf.namelist() if name.endswith(".md")] + assert len(md_files) == 2 + + +def test_bulk_export_partial_fail_returns_warning_header(client, monkeypatch): + real_parse = parse_session + + def flaky_parse(path: str): + if path.endswith("session_def456.jsonl"): + raise json.JSONDecodeError("bad", "doc", 0) + return real_parse(path) + + monkeypatch.setattr("utils.export_engine.parse_session", flaky_parse) + resp = client.post("/api/export", json={"since": "all"}) + assert resp.status_code == 200 + assert "X-Export-Warnings" in resp.headers + header = json.loads(resp.headers["X-Export-Warnings"]) + assert header["total_failures"] == 1 + assert header["truncated"] is False + assert len(header["failures"]) == 1 + assert header["failures"][0]["session_id"] == "session_def456" + assert header["failures"][0]["code"] == "PARSE_ERROR" + assert header["failures"][0]["message"] == "Failed to parse session" + zf = zipfile.ZipFile(io.BytesIO(resp.data)) + assert len([name for name in zf.namelist() if name.endswith(".md")]) == 1 + zip_warnings = json.loads(zf.read("export-warnings.json").decode("utf-8")) + assert len(zip_warnings) == 1 + assert zip_warnings[0]["session_id"] == "session_def456" + assert "bad" not in zip_warnings[0]["message"] + + +def test_bulk_export_all_fail_returns_422(client, monkeypatch): + def always_fail(path: str): + raise json.JSONDecodeError("bad", "doc", 0) + + monkeypatch.setattr("utils.export_engine.parse_session", always_fail) + resp = client.post("/api/export", json={"since": "all"}) + assert resp.status_code == 422 + body = resp.get_json() + assert body["code"] == "EXPORT_ALL_FAILED" + assert body["since"] == "all" + assert len(body["failures"]) == 2 + assert {item["code"] for item in body["failures"]} == {"PARSE_ERROR"} + assert all(item["message"] == "Failed to parse session" for item in body["failures"]) + + +def test_export_warnings_header_payload_truncates_at_entry_limit(): + failures = [ + ExportFailure( + session_id=f"sess_{i:04d}", + message="Failed to parse session", + code=ErrorCode.PARSE_ERROR, + ) + for i in range(25) + ] + payload = _export_warnings_header_payload(failures) + assert payload["total_failures"] == 25 + assert payload["truncated"] is True + assert len(payload["failures"]) <= 20 + + +def test_export_warnings_header_payload_byte_overflow_fallback(monkeypatch): + monkeypatch.setattr("api.export_api._EXPORT_WARNINGS_HEADER_MAX_BYTES", 80) + failures = [ + ExportFailure( + session_id="x" * 200, + message="Failed to parse session", + code=ErrorCode.PARSE_ERROR, + ) + ] + payload = _export_warnings_header_payload(failures) + assert payload["truncated"] is True + assert payload["failures"] == [] + assert len(json.dumps(payload, separators=(",", ":"))) <= 80 + + +def test_bulk_export_partial_fail_incremental_excludes_failed_from_state( + client, monkeypatch, export_state_file +): + export_state_file.write_text( + json.dumps({"sessions": {}, "exportedCount": 0}), + encoding="utf-8", + ) + real_parse = parse_session + + def flaky_parse(path: str): + if path.endswith("session_def456.jsonl"): + raise json.JSONDecodeError("bad", "doc", 0) + return real_parse(path) + + monkeypatch.setattr("utils.export_engine.parse_session", flaky_parse) + resp = client.post("/api/export", json={"since": "incremental"}) + assert resp.status_code == 200 + + state = json.loads(export_state_file.read_text(encoding="utf-8")) + sessions = state.get("sessions", {}) + assert "session_abc123" in sessions + assert "session_def456" not in sessions + + +def test_bulk_export_partial_fail_excludes_failed_from_state( + client, monkeypatch, export_state_file +): + real_parse = parse_session + + def flaky_parse(path: str): + if path.endswith("session_def456.jsonl"): + raise json.JSONDecodeError("bad", "doc", 0) + return real_parse(path) + + monkeypatch.setattr("utils.export_engine.parse_session", flaky_parse) + resp = client.post("/api/export", json={"since": "all"}) + assert resp.status_code == 200 + + state = json.loads(export_state_file.read_text(encoding="utf-8")) + sessions = state.get("sessions", {}) + assert "session_abc123" in sessions + assert "session_def456" not in sessions + + def test_export_state_json_fields(isolated_state): isolated_state.write_text( json.dumps( diff --git a/utils/export_engine.py b/utils/export_engine.py index 92d9ed9..201ac91 100644 --- a/utils/export_engine.py +++ b/utils/export_engine.py @@ -9,6 +9,7 @@ from datetime import date, datetime, timezone from typing import Any, Callable, Literal, Protocol +from models.error_codes import ErrorCode from models.project import ProjectDict, SessionListItemDict from models.session import SessionDict, SessionMetadataDict from models.stats import SessionStatsDict @@ -59,6 +60,41 @@ def serialize_manifest_jsonl(manifest: list[dict[str, Any]]) -> str: return "\n".join(json.dumps(e, default=str) for e in manifest) + "\n" +@dataclass +class ExportFailure: + """One per-session bulk export failure for API warning/error payloads.""" + + session_id: str + message: str + code: ErrorCode + + +def failure_code_for_exception( + exc: Exception, + *, + phase: Literal["parse", "export"] = "parse", +) -> ErrorCode: + """Map an export exception to a stable :class:`ErrorCode`. + + Export-phase failures always map to ``INTERNAL_ERROR``; ``exc`` is not + inspected on that path (no per-type export codes yet). + """ + if phase == "export": + return ErrorCode.INTERNAL_ERROR + if isinstance(exc, EXPORT_ERRORS): + return ErrorCode.PARSE_ERROR + return ErrorCode.INTERNAL_ERROR + + +def failure_message_for_code(code: ErrorCode) -> str: + """Stable client-facing message; never embed ``str(exc)`` (issue #25).""" + if code == ErrorCode.PARSE_ERROR: + return "Failed to parse session" + if code == ErrorCode.INTERNAL_ERROR: + return "Failed to export session" + return "Export failed" + + @dataclass class BulkExportResult: """Outcome of a bulk export run.""" @@ -68,7 +104,7 @@ class BulkExportResult: manifest: list[dict[str, Any]] = field(default_factory=list) new_sessions_map: dict[str, float] = field(default_factory=dict) exported_session_count: int = 0 - failure_count: int = 0 + failures: list[ExportFailure] = field(default_factory=list) skipped_count: int = 0 skipped_mtime_unchanged_count: int = 0 total_candidates: int = 0 @@ -76,6 +112,11 @@ class BulkExportResult: latest_day_scan_total: int = 0 latest_day_match_count: int = 0 + @property + def failure_count(self) -> int: + """Number of per-session failures (derived from :attr:`failures`).""" + return len(self.failures) + class ExportSink(Protocol): """Receives exported session files and final manifest.""" @@ -262,8 +303,20 @@ def run_bulk_export( result = BulkExportResult() manifest: list[dict[str, Any]] = [] - def _record_failure(sid: str, exc: Exception) -> None: - result.failure_count += 1 + def _record_failure( + sid: str, + exc: Exception, + *, + phase: Literal["parse", "export"] = "parse", + ) -> None: + code = failure_code_for_exception(exc, phase=phase) + result.failures.append( + ExportFailure( + session_id=sid, + message=failure_message_for_code(code), + code=code, + ) + ) if on_export_error is not None: on_export_error(sid, exc) @@ -283,7 +336,7 @@ def _export_parsed( result.new_sessions_map[sid] = float(sess_info.get("modified", 0)) result.exported_session_count += 1 except Exception as exc: - _record_failure(sid, exc) + _record_failure(sid, exc, phase="export") if since == "last": latest_day, rows, scan_total = collect_sessions_for_latest_activity_day(