LabDataStorageEvaluation/docs/rules/code-error-handling.md

# Code / Error handling - never swallow, always surface, fail loud

**Purpose + scope.** Governs every `try:` / `except:` / `raise` site in
Python code in this repository (data generation, format converters, benchmark
runner, extrapolation, reporting, helper tools). Binding for all contributors.

The pipeline's core failure principle: **a wrong artifact is worse than no
artifact.** Every task's output feeds downstream tasks; silently degraded
data poisons every result built on it. When in doubt, abort with a clear
message and a non-zero exit code.

---

## 1. The rule

1. **Never `except: pass`.** A bare swallow is always a bug. Forbidden
   absolutely.
2. **Never `except Exception: logger.warning("...") + continue`** for
   anything that was not an EXPECTED, documented, transient condition. If
   you swallow and continue, you must be able to state why the program can
   correctly proceed.
3. **Always `logger.error(..., exc_info=True)` in `except` blocks.**
   `exc_info=True` captures the traceback into the log record (see
   [obs-python-logging.md](obs-python-logging.md)). Without it the original
   exception is lost.
4. **Re-raise unless you have a defined recovery path.** If the caller
   cannot continue meaningfully without the failed operation, re-raise
   after logging.
5. **Surface to the right channel:**
   - **CLI script (every pipeline task):** print the error to stderr and
     exit non-zero. A task that failed MUST NOT write its
     `./out/.done/<task>.ok` marker (see
     [build-pipeline-tasks.md](build-pipeline-tasks.md)).
   - **Validation gate (row counts, checksums, size bounds):** a failed
     gate is an abort, not a warning. Example: task 03 aborts when the
     generated tree is outside 120-200 MB; converters abort when row counts
     do not match `MANIFEST.csv`.
   - **Library / helper function:** re-raise; let the caller decide.
6. **Defensive "this cannot happen" branches must surface, not silently
   `return`.** If you reach a defensive branch, log at ERROR with the
   branch name and the inputs that got you there, then raise.

## 2. The exception ladder - the only three sanctioned swallow patterns

Code may swallow ONLY in these three documented patterns. Anything else
re-raises after logging.

### Pattern A: documented, recoverable transient (retry)

```python
for attempt in range(MAX_RETRIES):
	try:
		return do_thing()
	except (psycopg.OperationalError, TimeoutError) as exc:
		logger.warning("attempt %d/%d failed: %s", attempt + 1, MAX_RETRIES, exc)
		time.sleep(backoff(attempt))
# All retries exhausted - surface.
logger.error("do_thing failed after %d retries", MAX_RETRIES)
raise
```

Justified: the per-attempt warning is acceptable; the final failure
re-raises.

### Pattern B: optional best-effort cleanup

```python
def _close_conn(conn) -> None:
	try:
		conn.close()
	except Exception:
		# Cleanup is best-effort; the operation already finished, log and continue.
		logger.warning("closing connection failed", exc_info=True)
```

Justified: the comment names why continuing is correct; the traceback is
still captured.

### Pattern C: benchmark-cell isolation - record and continue the matrix

```python
try:
	result = run_cell(fmt, query, timeout_s=30 * 60)
except CellTimeout:
	logger.error("cell %s/%s timed out", fmt, query)
	write_result_row(fmt, query, status="TIMEOUT")   # recorded, not hidden
except Exception:
	logger.error("cell %s/%s failed", fmt, query, exc_info=True)
	write_result_row(fmt, query, status="ERROR")
```

Justified: spec 09 requires the benchmark matrix to survive a failing cell
(RDF Q5 is expected to time out). The failure is RECORDED as data
(`TIMEOUT` / `ERROR` in `results_raw.csv`), never silently dropped, and the
run continues with the next cell. This pattern applies ONLY to the benchmark
runner's per-cell boundary - a failure in the runner itself (setup, result
writing) still aborts.

## 3. Loading into SQLite / PostgreSQL - all-or-nothing per table

A converter that half-loads a table leaves a database that LOOKS complete
and silently skews every benchmark run against it.

1. Wrap each table's bulk load in a transaction; on any error roll back
   and abort the converter with a non-zero exit.
2. After loading, validate row counts against `MANIFEST.csv` and fail loud
   on any mismatch (see
   [test-pipeline-validation.md](test-pipeline-validation.md)).
3. Never write the task's `.done` marker after a partial load.

## 4. Anti-patterns

- **`except: pass`** - forbidden absolutely.
- **`except Exception: logger.warning("could not X")` + continue** -
  forbidden unless the function documents that "could not X" is a
  recoverable expected outcome. If it is, phrase it "X skipped (expected
  condition Y)" so a reader does not grep "could not" hunting for bugs.
- **Catching `BaseException`** - forbidden (swallows `KeyboardInterrupt`,
  `SystemExit`). Use `Exception`.
- **`logger.error("X")` without `exc_info=True` in an `except` block** -
  forbidden. The traceback IS the diagnostic.
- **`logger.error(str(exc))`** - forbidden; redundant with `exc_info=True`
  and loses type and stack.
- **Returning `None` to signal failure** from a function whose contract is
  "return T" - forbidden. Either raise, or return `T | None` with
  documented semantics the caller checks.
- **`raise Exception("...")`** - forbidden. Use a specific class:
  `ValueError` / `RuntimeError` / `KeyError` for trivial cases, a custom
  domain exception subclass (e.g. `ManifestMismatchError`) for domain
  conditions.
- **`raise` chained without context** - inside an `except`, prefer
  `raise NewError(...) from exc` so the original cause stays visible in the
  chained traceback. A bare `raise` that re-raises the same exception is
  fine; `raise SomethingElse(...)` without `from exc` is forbidden.
- **Writing a `.done` marker in a `finally` block** - the marker means
  "completed successfully", never "attempted".
- **Clamping / repairing out-of-range generated values silently** - a
  physical-model bug must surface, not be masked by a `max(0, ...)` added
  "to be safe". Deliberate clamps defined by the spec (e.g. COF floor 0.12)
  are model logic, not error handling.

## 5. Defensive branches

```python
# Good:
def open_store(fmt: str) -> Store:
	if fmt == "sqlite":
		return SqliteStore()
	if fmt == "postgresql":
		return PgStore()
	# Defensive: a new format added without updating this factory lands here.
	logger.error("open_store: unknown format=%r", fmt)
	raise ValueError(f"unknown storage format: {fmt!r}")
```

The silent-`return None` form lets the unknown value propagate and breaks
downstream code with a confusing `AttributeError` on the wrong line.

## 6. Logging vs surfacing - the decision

For a caught exception, ask in order:

1. **Will the pipeline produce a wrong artifact if I continue?** Re-raise /
   abort.
2. **Is this expected and recoverable in the caller's contract?** Log at
   WARNING with `exc_info=True`, return a sentinel the caller checks.
3. **Is this an isolated benchmark cell (pattern C)?** Log at ERROR, record
   the status as data, continue the matrix.
4. **Is the process going down anyway?** Log at CRITICAL with
   `exc_info=True`, then `sys.exit(1)` or let it propagate.

## 7. Related documents

- [obs-python-logging.md](obs-python-logging.md) - logging API and severity
  levels.
- [test-pipeline-validation.md](test-pipeline-validation.md) - the
  validation gates that turn silent corruption into loud failures.
- [build-pipeline-tasks.md](build-pipeline-tasks.md) - `.done` marker
  semantics.
- [bench-methodology.md](bench-methodology.md) - the TIMEOUT contract for
  benchmark cells.