thiserror finalize: drop anyhow from production, add satkit::Error façade#86
Merged
Conversation
Migrate the last anyhow holdouts in utils/download.rs and utils/update_data.rs to thiserror enums, then update the five module Error types that wrapped them (spaceweather, jplephem, earth_orientation_params, earthgravity, solar_cycle_forecast) to consume Download(#[from] download::Error) directly instead of bridging through anyhow::Error. The Download variant is now typed end-to-end and the intermediate From<anyhow::Error> impls are gone. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… migrated Replace the last stringified / boxed-anyhow placeholder variants with typed #[from] sources, and migrate the two remaining anyhow-internal modules (sgp4 and frametransform) to per-module thiserror enums. After this commit, no production code references anyhow. - tle::Error: InvalidEpoch(String) -> InvalidEpoch(#[from] InstantError); KeplerConversion(String) -> Kepler(#[from] kepler::Error). The .map_err shims at the call sites collapse to plain `?`. - orbitprop::Error: Precompute(String) -> Jplephem(#[from] jplephem::Error) now that jplephem owns a typed Error. - sgp4: new typed Error wrapping SatRecInit(i32) and a boxed Source slot for SGP4Source impls (TLE is infallible; OMM forwards omm::Error). Public sgp4 / sgp4_full / SGP4Source::sgp4_init_args all return sgp4::Result. - frametransform: new typed Error covering UnsupportedFrame, IERS table parsing, and the propagating download / datadir / io / parse sources. - Doctest snippets switch to `Ok::<(), satkit::omm::Error>(())` so the rendered docs match the actual API and don't rely on anyhow at all. - Python bindings: explicit Result<SGP4State> closure annotation in the list path of pysgp4 and a one-line `?` cleanup in mod_utils after update_datafiles changed return type. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Production code no longer references anyhow; the only remaining uses are inside `#[cfg(test)] mod tests` blocks, which are compiled with [dev-dependencies]. Moving anyhow off the runtime dependency list keeps downstream crates from inheriting a transitive anyhow dep just to use satkit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per-module thiserror enums give downstream Rust users a precise, matchable error contract, but apps spanning many modules (e.g. a CLI that loads TLEs, propagates orbits, and writes ephemerides) end up needing to define an outer enum just to glue the various module errors together. This commit adds a thin satkit::Error / satkit::Result façade that has From impls for every public module-scoped Error in the crate. Apps that prefer a single result type can return Result<T, satkit::Error> from the top-level function and let `?` do the conversion at every module boundary without giving up the typed-error guarantees underneath. The façade is opt-in — module functions still return their narrow types. InstantError is now also re-exported from the crate root for symmetry with Instant. Closes #81 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Final PR in the
anyhow→thiserrormigration tracked in #81. Closes #81.Four commits in order:
f3c36b5— Phase 4: typed errors forutils::downloadandutils::update_datacf6a4c1— Retype placeholderString/anyhow::Errorvariants now that all dependencies are migrated56a7ca1— Moveanyhowto[dev-dependencies]692efb2— Add top-levelsatkit::ErrorfaçadeWhat changed
Phase 4 —
utils::download::Error+utils::update_data::Errordownload::Error::FeatureDisabled/FileNotFoundNoDownload { path }.spaceweather,solar_cycle_forecast,earth_orientation_params,jplephem,earthgravity) now consumeDownload(#[from] download::Error)directly. The bridgeFrom<anyhow::Error>impls are deleted.Retyping
tle::Error::InvalidEpoch(String)→#[from] InstantErrortle::Error::KeplerConversion(String)→Kepler(#[from] kepler::Error)orbitprop::Error::Precompute(String)→Jplephem(#[from] jplephem::Error)sgp4::Error(withSatRecInit(i32)+Source(Box<dyn Error + Send + Sync>)slot) — wasn't strictly in the brief but was needed to fully eliminate anyhow fromsgp4_impl.rsand theSGP4Sourcetrait.frametransform::Errorforto_gcrf/from_gcrfand IERSTable parsing — same reason.Ok::<(), satkit::omm::Error>(())style.anyhowmoveanyhowis now in[dev-dependencies]only. Production code references it nowhere.grep -rn "anyhow" src/→ 11 hits, all inside#[cfg(test)] mod testsblocks (test-only, kept for ergonomics).satkit::Errorfaçadesrc/error.rswith 19 variants (one per public module Error). All#[error(transparent)]+#[from].pub use error::{Error, Result};fromlib.rs. Also re-exportsInstantErrorfor symmetry withInstant.update_data::Errorvariant gated#[cfg(feature = "download")](matches the module's gating).Judgment calls worth a look
tle::Error::Sgp4(String)left asString—sgp4::Errordoes implstd::error::Errornow, so retyping is mechanically possible. Kept as String to limit surface change in this PR; trivial follow-up if you want it.sgp4::Error::SourceisBox<dyn Error + Send + Sync>rather than enum variants for OMM/TLE — addingFrom<omm::Error> for sgp4::Errorwould create an awkward inverse dependency (omm already depends on sgp4 for the trait). Boxed slot keeps the dependency arrow flowing one way.download::Errorhas two stub-related variants (FeatureDisabledandFileNotFoundNoDownload { path }) —download_if_not_existstub needs to convey "file missing AND no download feature available", which is semantically distinct from the other stubs that are pure feature-gate refusals.JplEphem,SpaceWeather,EarthOrientationParams,SolarCycleForecast,EarthGravity,LpEphem,Frametransform.anyhowimports kept (per the brief). 11use anyhow::*lines inside#[cfg(test)]blocks.Test plan
cargo build— cleancargo build --no-default-features— cleancargo build --features download— cleancargo test --release— 158 lib + 39 doc passcargo test --release --no-default-features— 157 lib + 38 doc passcargo test --release --features download— 159 lib + 41 doc passcargo check --manifest-path python/Cargo.toml— cleanCloses #81.
🤖 Generated with Claude Code