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). 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).
   • 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)

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

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

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).
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

# 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 - logging API and severity levels.
• test-pipeline-validation.md - the validation gates that turn silent corruption into loud failures.
• build-pipeline-tasks.md - .done marker semantics.
• bench-methodology.md - the TIMEOUT contract for benchmark cells.