Summary
Error handling and logging across the Beacon workspace is uneven and limited. We want to systematically raise every crate to a consistent standard so failures are diagnosable.
Current state
- Only 2 of ~16 crates define structured error types (
beacon-object-storage::StorageError, beacon-arrow-odv::OdvError); the rest lean on bare anyhow, ad-hoc String errors, DataFusion's error type, or silently-swallowed Options.
- Heavy
unwrap()/expect()/panic!() in non-test code (e.g. beacon-datafusion-ext ~70 expect, beacon-arrow-netcdf ~54, beacon-core/runtime.rs 33 unwrap), several reachable at runtime via malformed input/files.
- Logging is inconsistent:
tracing is wired up in beacon-api, but many crates barely log and a few still use println!/eprintln!/dbg!.
Standard to apply per crate
- Library crates: a dedicated
thiserror Error enum + Result<T> alias (per-crate error.rs).
- Binaries/orchestrators (
beacon-api, beacon-core): keep anyhow with .context().
- DataFusion trait paths (
TableProvider/ExecutionPlan impls): keep returning datafusion::error::Result (converting cascades and fights trait signatures); improve internal messages/context only.
- Convert
unwrap/expect/panic! that are reachable at runtime (I/O, parsing, fallible conversions, user/file input) into propagated errors; leave provably-infallible ones with a justifying .expect("…") message.
- Add structured
tracing at the right levels; remove println!/dbg! from library code.
Plan
Execute crate-by-crate in dependency order (leaf → top), starting with beacon-common as the reviewable template.
Summary
Error handling and logging across the Beacon workspace is uneven and limited. We want to systematically raise every crate to a consistent standard so failures are diagnosable.
Current state
beacon-object-storage::StorageError,beacon-arrow-odv::OdvError); the rest lean on bareanyhow, ad-hocStringerrors, DataFusion's error type, or silently-swallowedOptions.unwrap()/expect()/panic!()in non-test code (e.g.beacon-datafusion-ext~70expect,beacon-arrow-netcdf~54,beacon-core/runtime.rs33unwrap), several reachable at runtime via malformed input/files.tracingis wired up inbeacon-api, but many crates barely log and a few still useprintln!/eprintln!/dbg!.Standard to apply per crate
thiserrorErrorenum +Result<T>alias (per-crateerror.rs).beacon-api,beacon-core): keepanyhowwith.context().TableProvider/ExecutionPlanimpls): keep returningdatafusion::error::Result(converting cascades and fights trait signatures); improve internal messages/context only.unwrap/expect/panic!that are reachable at runtime (I/O, parsing, fallible conversions, user/file input) into propagated errors; leave provably-infallible ones with a justifying.expect("…")message.tracingat the right levels; removeprintln!/dbg!from library code.Plan
Execute crate-by-crate in dependency order (leaf → top), starting with
beacon-commonas the reviewable template.StorageError— audit/extend)OdvError— audit/extend)