Skip to content

Commit d42e6bf

Browse files
Scott KostolniScott Kostolni
authored andcommitted
feat(errors): add structured error handling
1 parent efece7d commit d42e6bf

File tree

13 files changed

+452
-107
lines changed

13 files changed

+452
-107
lines changed

README.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,14 @@ Notes: You can define multiple profiles (e.g., `[default]`, `[work]`) and select
9494
- `limitless export-markdown --limit 5` / `--date YYYY-MM-DD --combine` — print or write markdown exports.
9595
- `limitless export-csv --date 2025-11-01 --output /tmp/lifelogs.csv` — dump metadata (add `--include-markdown` to include body content).
9696

97+
## Error handling & exit codes
98+
99+
- Network failures (including timeouts) are retried when safe and surface as concise `ApiError` messages (`HTTP 429` with Retry-After hints, or `Request timed out …`).
100+
- Local persistence issues (e.g., unwritable data dir) become `StorageError`/`StateError` with the offending path in the context.
101+
- The CLI wraps those exceptions in friendly stderr output so you see a single `Error: …` line instead of a Python traceback; pass `-v/--verbose` to capture the stack trace in logs if needed.
102+
- Exit codes: `0` success, `1` operational/service errors, `2` validation/config problems (e.g., invalid timezone, missing `--date` when using `--combine`), `130` for `Ctrl+C` aborts.
103+
- Errors also drive JSON logging via `--verbose`, so CI logs include structured context without leaking secrets.
104+
97105
## Documentation
98106

99107
- Usage guide: docs/USAGE.md

docs/TASKS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ Status: living document; update as work progresses. Use TDD for every task.
1717
- Combined per‑date Markdown export (`export-markdown --date ... --write-dir ... --combine`)
1818
- Env integration in CLI: auto-load `.env` at startup
1919
- Structured JSON logging and log levels (`--verbose`)
20-
- Improved error messages for non-retryable 4xx/5xx
20+
- Centralized error handling: exception hierarchy, timeout-aware HTTP errors, and CLI exit codes with friendly stderr output
2121
- Timezone validation in CLI (IANA names) and documented behavior
2222
- Performance: tuned defaults; added `--batch-size`
2323
- User-scoped config file (TOML) with precedence: CLI flags > env > config; `--config`/`--profile`

docs/USAGE.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,13 @@ python -m limitless_tools.cli.main search --query "Weekly Meeting" --fuzzy --fuz
115115

116116
Regex searches accept `--regex` or the short form `-rg` and are case-insensitive; fuzzy search uses `rapidfuzz` when available (falls back to `difflib`) with `--fuzzy-threshold` defaulting to `80`.
117117

118+
### Error handling & exit codes
119+
120+
- The HTTP client retries `429/502/503/504` responses (respecting `Retry-After`) and wraps network failures/timeouts in `ApiError` so the CLI can surface a concise message such as `Error: Request timed out while fetching lifelogs.`
121+
- Storage/state operations raise `StorageError`/`StateError` with the problematic path in their context; services rewrap them as `ServiceError` for consistent CLI UX.
122+
- Exit codes: `0` success, `1` operational/service errors (HTTP/network/storage), `2` validation/config issues (e.g., invalid timezone, missing `--date` when using `--combine`), `130` when the user interrupts with `Ctrl+C`.
123+
- The CLI prints only a single `Error: …` line to stderr; pass `-v/--verbose` (or set `LIMITLESS_DEBUG=1`) to include full stack traces in the structured logs while keeping stdout clean for JSON piping.
124+
118125
### Configuration file (MVP)
119126

120127
You can store defaults in a user config TOML and select profiles via `--profile`:

limitless_tools/cli/main.py

Lines changed: 94 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
from limitless_tools.config.env import load_env
1414
from limitless_tools.config.logging import setup_logging
1515
from limitless_tools.config.paths import default_data_dir, expand_path
16+
from limitless_tools.errors import LimitlessError, ValidationError
1617
from limitless_tools.services.lifelog_service import LifelogService, SaveReport
1718

1819

@@ -159,48 +160,34 @@ def _normalize_data_dir(value: str | None, *, base_dir: str | None = None) -> st
159160
return default_data_dir()
160161

161162

162-
def main(argv: list[str] | None = None) -> int:
163-
# Ensure .env and related environment variables are loaded before parsing
164-
load_env()
165-
parser = _build_parser()
166-
args = parser.parse_args(args=argv)
167-
setup_logging(verbose=bool(getattr(args, "verbose", False)))
168-
169-
log = logging.getLogger("limitless_tools.cli")
170-
log.info("cli_start", extra={"event": "cli_start", "command": args.command})
171-
if getattr(args, "verbose", False):
172-
# Emit a debug message for tests/diagnostics (avoid reserved LogRecord keys)
173-
log.debug("parsed_args", extra={"cli_args": vars(args)})
174-
175-
# Load config and resolve profile
176-
# Allow env var overrides for config path and profile
177-
config_path_arg = args.config or os.getenv("LIMITLESS_CONFIG")
178-
resolved_config_path = expand_path(config_path_arg) or default_config_path()
179-
profile_name = args.profile or os.getenv("LIMITLESS_PROFILE") or "default"
180-
181-
cfg = load_config(resolved_config_path)
182-
prof = get_profile(cfg, profile_name)
183-
config_base_dir = str(Path(resolved_config_path).expanduser().parent)
184-
185-
# Precedence: CLI flags > environment variables > config > defaults
186-
argv_list = argv or []
187-
163+
def _coerce_timeout_value(value: object, log: logging.Logger) -> float | None:
164+
if isinstance(value, (int, float)):
165+
return float(value)
166+
if isinstance(value, str):
167+
stripped = value.strip()
168+
if not stripped:
169+
return None
170+
try:
171+
return float(stripped)
172+
except ValueError:
173+
log.debug("Invalid http_timeout config value: %s", value)
174+
return None
175+
176+
177+
def _execute_command(
178+
*,
179+
args: argparse.Namespace,
180+
argv_list: list[str],
181+
prof: dict,
182+
config_base_dir: str,
183+
parser: argparse.ArgumentParser,
184+
log: logging.Logger,
185+
resolved_config_path: str,
186+
profile_name: str,
187+
) -> int:
188188
def _provided(opt: str) -> bool:
189189
return opt in argv_list
190190

191-
def _coerce_timeout_value(value: object) -> float | None:
192-
if isinstance(value, (int, float)):
193-
return float(value)
194-
if isinstance(value, str):
195-
stripped = value.strip()
196-
if not stripped:
197-
return None
198-
try:
199-
return float(stripped)
200-
except ValueError:
201-
log.debug("Invalid http_timeout config value: %s", value)
202-
return None
203-
204191
# data_dir precedence
205192
data_dir_from_config = False
206193
if not _provided("--data-dir") and not os.getenv("LIMITLESS_DATA_DIR"):
@@ -223,7 +210,7 @@ def _coerce_timeout_value(value: object) -> float | None:
223210
resolved_api_url = os.getenv("LIMITLESS_API_URL") or (prof.get("api_url") if isinstance(prof.get("api_url"), str) else None)
224211
resolved_http_timeout: float | None = None
225212
if not os.getenv("LIMITLESS_HTTP_TIMEOUT"):
226-
resolved_http_timeout = _coerce_timeout_value(prof.get("http_timeout"))
213+
resolved_http_timeout = _coerce_timeout_value(prof.get("http_timeout"), log)
227214

228215
args.data_dir = _normalize_data_dir(
229216
getattr(args, "data_dir", None),
@@ -273,11 +260,12 @@ def _coerce_timeout_value(value: object) -> float | None:
273260
if args.timezone:
274261
try:
275262
_ = ZoneInfo(args.timezone)
276-
except Exception:
277-
sys.stderr.write(
278-
f"Invalid timezone: {args.timezone}. Use an IANA name like 'America/Los_Angeles' or 'UTC'.\n"
279-
)
280-
return 2
263+
except Exception as exc:
264+
raise ValidationError(
265+
f"Invalid timezone: {args.timezone}. Use an IANA name like 'America/Los_Angeles' or 'UTC'.",
266+
cause=exc,
267+
context={"timezone": args.timezone},
268+
) from exc
281269
service = LifelogService(
282270
api_key=resolved_api_key,
283271
api_url=resolved_api_url,
@@ -367,8 +355,10 @@ def _coerce_timeout_value(value: object) -> float | None:
367355
)
368356
eff_write_dir = args.write_dir or cfg_output_dir
369357
if not args.date or not eff_write_dir:
370-
sys.stderr.write("--combine requires --date and a write directory (provide --write-dir or set output_dir in config)\n")
371-
return 2
358+
raise ValidationError(
359+
"--combine requires --date and a write directory (provide --write-dir or set output_dir in config)",
360+
context={"command": "export-markdown"},
361+
)
372362
text = service.export_markdown_by_date(date=args.date, frontmatter=args.frontmatter)
373363
from pathlib import Path as _Path
374364
outdir = _Path(eff_write_dir)
@@ -464,5 +454,62 @@ def _coerce_timeout_value(value: object) -> float | None:
464454
return 2
465455

466456

457+
def main(argv: list[str] | None = None) -> int:
458+
# Ensure .env and related environment variables are loaded before parsing
459+
load_env()
460+
parser = _build_parser()
461+
args = parser.parse_args(args=argv)
462+
setup_logging(verbose=bool(getattr(args, "verbose", False)))
463+
464+
log = logging.getLogger("limitless_tools.cli")
465+
log.info("cli_start", extra={"event": "cli_start", "command": args.command})
466+
if getattr(args, "verbose", False):
467+
# Emit a debug message for tests/diagnostics (avoid reserved LogRecord keys)
468+
log.debug("parsed_args", extra={"cli_args": vars(args)})
469+
470+
# Load config and resolve profile
471+
# Allow env var overrides for config path and profile
472+
config_path_arg = args.config or os.getenv("LIMITLESS_CONFIG")
473+
resolved_config_path = expand_path(config_path_arg) or default_config_path()
474+
profile_name = args.profile or os.getenv("LIMITLESS_PROFILE") or "default"
475+
476+
cfg = load_config(resolved_config_path)
477+
prof = get_profile(cfg, profile_name)
478+
config_base_dir = str(Path(resolved_config_path).expanduser().parent)
479+
480+
argv_list = argv or []
481+
verbose = bool(getattr(args, "verbose", False))
482+
483+
try:
484+
return _execute_command(
485+
args=args,
486+
argv_list=argv_list,
487+
prof=prof,
488+
config_base_dir=config_base_dir,
489+
parser=parser,
490+
log=log,
491+
resolved_config_path=resolved_config_path,
492+
profile_name=profile_name,
493+
)
494+
except ValidationError as exc:
495+
_stderr_line(f"Error: {exc}")
496+
return 2
497+
except LimitlessError as exc:
498+
log_args = {"context": getattr(exc, "context", {})}
499+
if verbose:
500+
log.exception("limitless_error", extra=log_args)
501+
else:
502+
log.error("limitless_error", extra={**log_args, "error": str(exc)})
503+
_stderr_line(f"Error: {exc}")
504+
return 1
505+
except KeyboardInterrupt:
506+
_stderr_line("Aborted by user.")
507+
return 130
508+
except Exception: # pragma: no cover - final safeguard
509+
log.exception("unhandled_exception")
510+
_stderr_line("Unexpected error occurred. Re-run with --verbose for stack trace.")
511+
return 1
512+
513+
467514
if __name__ == "__main__": # pragma: no cover
468515
raise SystemExit(main())

limitless_tools/errors.py

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
"""Shared exception hierarchy for the Limitless Tools CLI."""
2+
3+
from __future__ import annotations
4+
5+
from dataclasses import dataclass, field
6+
from typing import Any
7+
8+
9+
@dataclass(slots=True)
10+
class LimitlessError(Exception):
11+
"""Base class for all domain-specific errors."""
12+
13+
message: str
14+
cause: Exception | None = None
15+
context: dict[str, Any] = field(default_factory=dict)
16+
17+
def __post_init__(self) -> None:
18+
if self.cause is not None and not isinstance(self.cause, BaseException):
19+
raise TypeError("cause must be an exception instance")
20+
21+
def __str__(self) -> str: # pragma: no cover - trivial method
22+
return self.message
23+
24+
25+
@dataclass(slots=True)
26+
class ConfigurationError(LimitlessError):
27+
"""Raised when configuration or environment setup is invalid."""
28+
29+
30+
@dataclass(slots=True)
31+
class ValidationError(LimitlessError):
32+
"""Raised when user input fails validation (e.g., invalid CLI args)."""
33+
34+
35+
@dataclass(slots=True)
36+
class ApiError(LimitlessError):
37+
"""Raised when the remote API responds with an error or network failure."""
38+
39+
status_code: int | None = None
40+
41+
42+
@dataclass(slots=True)
43+
class StorageError(LimitlessError):
44+
"""Raised when local storage operations fail."""
45+
46+
47+
@dataclass(slots=True)
48+
class StateError(LimitlessError):
49+
"""Raised for sync state read/write issues."""
50+
51+
52+
@dataclass(slots=True)
53+
class OutputError(LimitlessError):
54+
"""Raised when export/write targets cannot be written."""
55+
56+
57+
@dataclass(slots=True)
58+
class ServiceError(LimitlessError):
59+
"""Raised by services when orchestration fails."""
60+

limitless_tools/http/client.py

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
from collections.abc import Callable
66
from typing import Any
77

8+
from limitless_tools.errors import ApiError, ConfigurationError
9+
810
try:
911
import requests
1012
except ImportError: # pragma: no cover - requests should be present in dev
@@ -82,7 +84,7 @@ def _enforce_base_url_allowlist(self) -> None:
8284
extra_hosts = {h.strip() for h in extra.split(",") if h.strip()}
8385
allowed = default_allowed | extra_hosts
8486
if host not in allowed:
85-
raise ValueError(f"Base URL host not allowed: {host}")
87+
raise ConfigurationError(f"Base URL host not allowed: {host}")
8688

8789
def get_lifelogs(
8890
self,
@@ -152,7 +154,28 @@ def get_lifelogs(
152154
req_kwargs["timeout"] = self.timeout
153155
except (AttributeError, ValueError, TypeError) as exc:
154156
log.debug("Session.get signature missing timeout: %s", exc)
155-
resp = self.session.get(url, headers=self._headers(), params=params, **req_kwargs) # type: ignore[union-attr]
157+
try:
158+
resp = self.session.get( # type: ignore[union-attr]
159+
url,
160+
headers=self._headers(),
161+
params=params,
162+
**req_kwargs,
163+
)
164+
except Exception as exc:
165+
if attempt < self.max_retries:
166+
attempt += 1
167+
delay = self.backoff_factor * (2 ** (attempt - 1))
168+
self.sleep_fn(delay)
169+
continue
170+
msg = self._network_error_message(exc)
171+
raise ApiError(
172+
msg,
173+
cause=exc,
174+
context={
175+
"url": url,
176+
"params": {k: params.get(k) for k in ("cursor", "limit", "date") if params.get(k)},
177+
},
178+
) from exc
156179
if getattr(resp, "ok", False):
157180
break
158181
status = getattr(resp, "status_code", None)
@@ -185,7 +208,11 @@ def get_lifelogs(
185208
continue
186209
# Build informative error message for non-retryable errors
187210
detail = self._error_detail(resp)
188-
raise RuntimeError(f"HTTP {status} error fetching lifelogs: {detail}")
211+
raise ApiError(
212+
f"HTTP {status} error fetching lifelogs: {detail}",
213+
status_code=status,
214+
context={"url": url, "params": {"cursor": params.get("cursor")}},
215+
)
189216

190217
body = resp.json()
191218
page_items: list[dict[str, Any]] = body.get("data", {}).get("lifelogs", []) or []
@@ -236,3 +263,18 @@ def _error_detail(self, resp: Any) -> str:
236263
if isinstance(text, str) and text.strip():
237264
return text.strip()
238265
return "Unknown error"
266+
267+
def _network_error_message(self, exc: Exception) -> str:
268+
text = str(exc).strip().lower()
269+
if self._looks_like_timeout(exc, text):
270+
return "Request timed out while fetching lifelogs."
271+
return "Network error while fetching lifelogs."
272+
273+
@staticmethod
274+
def _looks_like_timeout(exc: Exception, text: str) -> bool:
275+
if isinstance(exc, TimeoutError): # noqa: F821 - built-in in >=3.10
276+
return True
277+
if "timeout" in text:
278+
return True
279+
timeout_cls = getattr(requests, "Timeout", None) if requests is not None else None
280+
return timeout_cls is not None and isinstance(exc, timeout_cls)

0 commit comments

Comments
 (0)