complete the migration from thiserror to gix-error - #2847
Sebastian Thiel (Byron) wants to merge 53 commits into
Conversation
CI exposed three remaining migration adaptations: gix-shallow doctests could not convert Exn through Box<dyn Error>, the ein init journey snapshots still expected the removed enum message, and lint rejected two mechanical expressions. Convert Exn explicitly in the doctest, update both init snapshots, and apply the two lint-preserving rewrites. Observed in test-doc, test-journey, and lint on PR #2847.
|
Did the comparison. Three things, plus one offer. 1. 2. 3. I've written a guard for that: Also: in On the comparison: I audited every There's more from the sweep — a per-type verdict on all 42 types #2716 left concrete, an erasure order for the E0119 chains, and a list of dropped |
|
Thanks Amey Pawar (@ameyypawar), while noting that I find no pleasure in reading these AI generated blobs of text. My main gripe is that it's a bot speaking through you, so unless you say you produced this text by hand or think you could produce it, disclosure is the way to go. I recommend adding a few lines of yourself on top giving me your verdict, no matter what it is (i.e. something like "this looks reasonable to me, and I spot-checked one of these claims"), followed, by a separator to clearly mark the AI blob. Thanks again. |
b7df324 to
92a3276
Compare
CI exposed three remaining migration adaptations: gix-shallow doctests could not convert Exn through Box<dyn Error>, the ein init journey snapshots still expected the removed enum message, and lint rejected two mechanical expressions. Convert Exn explicitly in the doctest, update both init snapshots, and apply the two lint-preserving rewrites. Observed in test-doc, test-journey, and lint on PR #2847.
CI exposed three remaining migration adaptations: gix-shallow doctests could not convert Exn through Box<dyn Error>, the ein init journey snapshots still expected the removed enum message, and lint rejected two mechanical expressions. Convert Exn explicitly in the doctest, update both init snapshots, and apply the two lint-preserving rewrites. Observed in test-doc, test-journey, and lint on PR #2847.
92a3276 to
240b509
Compare
CI exposed three remaining migration adaptations: gix-shallow doctests could not convert Exn through Box<dyn Error>, the ein init journey snapshots still expected the removed enum message, and lint rejected two mechanical expressions. Convert Exn explicitly in the doctest, update both init snapshots, and apply the two lint-preserving rewrites. Observed in test-doc, test-journey, and lint on PR #2847.
240b509 to
6b1745e
Compare
|
Re-checked after your push. Four things I'd still look at. The message loss bothers me most; on the security tests I'd rather have your call than mine. The sweep and this write-up are both AI-produced — I took help of AI tools throughout. I checked the two source arms and the Messages dropped: 53 sites, 50 distinct. Worst: 17 assertions weakened. Two matter: the three erased-API asserts in
Dead branches: Also: ~31 double-wraps left after the 19 you removed — a floor, counted from monomorphised instantiations rather than grep, so I can pull the list if useful. And |
CI exposed three remaining migration adaptations: gix-shallow doctests could not convert Exn through Box<dyn Error>, the ein init journey snapshots still expected the removed enum message, and lint rejected two mechanical expressions. Convert Exn explicitly in the doctest, update both init snapshots, and apply the two lint-preserving rewrites. Observed in test-doc, test-journey, and lint on PR #2847.
6b1745e to
153109c
Compare
CI exposed three remaining migration adaptations: gix-shallow doctests could not convert Exn through Box<dyn Error>, the ein init journey snapshots still expected the removed enum message, and lint rejected two mechanical expressions. Convert Exn explicitly in the doctest, update both init snapshots, and apply the two lint-preserving rewrites. Observed in test-doc, test-journey, and lint on PR #2847.
153109c to
1ab89b7
Compare
1ab89b7 to
e9f189c
Compare
CI exposed three remaining migration adaptations: gix-shallow doctests could not convert Exn through Box<dyn Error>, the ein init journey snapshots still expected the removed enum message, and lint rejected two mechanical expressions. Convert Exn explicitly in the doctest, update both init snapshots, and apply the two lint-preserving rewrites. Observed in test-doc, test-journey, and lint on PR #2847.
|
Thanks Amey Pawar (@ameyypawar). This is an interesting experiment as you essentially take the role of a reviewer, while my agent double-checks and fixes. And all that without any human review, so I am already very curious on how the actual review can be done efficiently. |
CI exposed three remaining migration adaptations: gix-shallow doctests could not convert Exn through Box<dyn Error>, the ein init journey snapshots still expected the removed enum message, and lint rejected two mechanical expressions. Convert Exn explicitly in the doctest, update both init snapshots, and apply the two lint-preserving rewrites. Observed in test-doc, test-journey, and lint on PR #2847.
e9f189c to
2d9f5d9
Compare
CI exposed three remaining migration adaptations: gix-shallow doctests could not convert Exn through Box<dyn Error>, the ein init journey snapshots still expected the removed enum message, and lint rejected two mechanical expressions. Convert Exn explicitly in the doctest, update both init snapshots, and apply the two lint-preserving rewrites. Observed in test-doc, test-journey, and lint on PR #2847.
2d9f5d9 to
5647e7f
Compare
CI exposed three remaining migration adaptations: gix-shallow doctests could not convert Exn through Box<dyn Error>, the ein init journey snapshots still expected the removed enum message, and lint rejected two mechanical expressions. Convert Exn explicitly in the doctest, update both init snapshots, and apply the two lint-preserving rewrites. Observed in test-doc, test-journey, and lint on PR #2847.
5647e7f to
04a637e
Compare
CI exposed three remaining migration adaptations: gix-shallow doctests could not convert Exn through Box<dyn Error>, the ein init journey snapshots still expected the removed enum message, and lint rejected two mechanical expressions. Convert Exn explicitly in the doctest, update both init snapshots, and apply the two lint-preserving rewrites. Observed in test-doc, test-journey, and lint on PR #2847.
04a637e to
2224378
Compare
CI exposed three remaining migration adaptations: gix-shallow doctests could not convert Exn through Box<dyn Error>, the ein init journey snapshots still expected the removed enum message, and lint rejected two mechanical expressions. Convert Exn explicitly in the doctest, update both init snapshots, and apply the two lint-preserving rewrites. Observed in test-doc, test-journey, and lint on PR #2847.
<!-- Byron --> rubberstamp
<!-- Byron --> rubberstamp
<!-- Byron --> rubberstamp
`DeltaBaseUnresolved` retained the missing object ID but exposed no semantic source, so conversion to `gix::Error` lost its missing-object meaning. Expose a `NotFoundError` source while retaining the custom error and its ID. Both header and entry decoding now share the same classification. Validation: reproduced the missing classification before the fix; all 19 pack file tests pass in tree and `auto-chain-error` modes. Formatting passes.
<!-- Byron --> rubberstamp, but looked at it more to understand why it's more code. Answer: downstream relies on better error classification. However, I think this can also be reduced a bit.
Custom object database errors forwarded to their inner error's `source()`, skipping the error that supplied an I/O or semantic classification. Some leaf variants exposed no classification at all. Retain the immediate causes for lookup, write, prefix, alternate and integrity errors. Store realpath exceptions as `gix_error::Error` so their complete tree remains available. Expose missing delta bases, malformed object sizes and alternate cycles, invalid alternate paths and retryable verification failures through classified sources. Validation: the new regressions fail before the fix. All object database tests and doctests pass in tree and `auto-chain-error` modes. Focused Clippy and formatting pass.
<!-- Byron --> rubberstamp But looked into it more to see why it has so much more code. The simple answer is that it basically doesn't use `gix-error`, but writes error types manually. This needs rework.
Missing reference and object errors ended their `source()` chains, leaving `gix_error::Error::is_not_found()` unable to recognize them after erasure, including when exposed through `gix::Error`. Expose static classification causes while keeping the concrete variants and existing sources. Also classify malformed reference data, symbolic cycles, and invalid reflog input. Preserve tag decoding failures with corruption context instead of reporting an existing malformed tag as a missing object. Regression coverage exercises lookup, peeling, transactions, packed-reference iteration, and reflogs after erasure, including retained I/O causes and retry policy. All 194 `gix-ref` tests pass with both tree and `auto-chain-error` storage; formatting and focused Clippy checks pass.
<!-- Byron --> rubberstamp, but it needs some more work to not repeat `can_retry()`
Transport errors duplicated the shared I/O retry policy and stamped a `RetryableError` only during selected conversions. The same failure could therefore change classification depending on how it reached `gix::Error`. Use native sources and the shared retry predicates throughout transport and protocol handling. Network callers use `can_retry_lenient()`, including its out-of-memory policy, while conversion no longer adds retry markers. Expose validation for unsafe SSH/path arguments and corruption for malformed packet responses without losing the original custom error types. Validation: regression reproduced before the fix; blocking and async transport and protocol tests pass, including localhost connection refusal. Library tests also pass with `gix-error/auto-chain-error` and the curl backend. `gix` checks with blocking networking enabled; formatting passes. Strict Clippy encounters existing test warnings and `drop_non_drop` in fetch tracing.
<!-- Byron --> rubberstamp
<!-- Byron --> rubberstamp <!-- agent --> Keep the original `TryReserveError` inside the `OutOfMemory` I/O error so structured classification can distinguish allocator failure without treating malformed stream metadata as corruption.
<!-- Byron -->
rubberstamp
<!-- agent -->
The crate-by-crate migration retained operation-specific error aliases to
limit downstream churn. With the migration complete, those names only hide
the shared error types and keep otherwise empty API namespaces alive.
Use the underlying `gix_error` types directly throughout the workspace,
including indirect aliases, renamed exports, test helpers, and the URL fuzz
target. Remove namespaces and files that only held forwarding aliases, and
update documentation and migration guidance to use the canonical types.
Adjust the source locations recorded in error snapshots after deleting the
alias declarations.
Keep `gix::{Error, Exn}` and `gix::error` as the central facade, along with
unrenamed canonical re-exports, required associated types, concrete errors,
and aliases that add structure. Preserve each `Exn` parameter, conditional
error alternative, error message, and source chain. Include all downstream
adaptations in this breaking change so the stack remains buildable.
<!-- Byron --> rubberstamp <!-- agent --> Replace classification-only downcasts with semantic predicates for retryability, missing resources, invalid input, corruption, and resource exhaustion. Use borrowed probable-cause inspection where callers only need the underlying error, removing temporary ownership conversions.
<!-- Byron --> rubberstamp <!-- agent --> The central `gix::Error` already implements the conversions used by `?`. Remove redundant `map_err(Exn::into_error)` and equivalent `Error::from` calls, including conversions inside context closures, while retaining the context itself and explicit conversions at returned-result boundaries.
<!-- Byron --> rubberstamp <!-- agent --> `ResultExt` already accepts exceptions directly. Remove five erasures in pack generation and merge paths that immediately add context, and avoid converting the committer exception to `Error` before adding clone context. Preserve each context message and final erasure where the callback needs it.
<!-- Byron --> rubberstamp <!-- agent --> Inspect retry policies directly on `Exn` in `gix-index`, `gix-transport`, and `gix-worktree-stream` tests. Remove conversions to `gix_error::Error` that were only needed to inspect these exceptions.
`std::io::Error::source()` skips its custom payload, hiding classification markers, custom error types, and branches of a nested `gix_error::Error`. Retain the payload while walking native sources so borrowed retry policies, exception traversal, and both porcelain error modes see the complete cause. Document how custom errors expose their immediate cause and classification markers. Cover every classification, nested branches, and concrete payload downcasts, and update diagnostic snapshots for the retained I/O payload. Validated with `cargo test -p gix-error` and focused Clippy in both tree and `auto-chain-error` modes, plus 980 affected-crate unit and integration tests.
…nversion Expose the shared `ValidationError` marker from reference, tag, submodule, and path-component errors without changing their variants or input details. This lets callers distinguish invalid names from absent references through `gix::Error`, including optional reference lookups. Cover all four validation error types before and after conversion and add porcelain reference lookup regressions. The 329 validation tests pass in both error modes, as do the reference API tests and focused Clippy.
Expose a `ValidationError` source from command-line parser errors so their classification survives raising and conversion to `gix_error::Error`. Keep the original parser variant available for callers that need details. Regression cases cover missing quotes, dangling escapes, and assignment-only input. All command tests and doctests pass in both error modes; focused Clippy also passes.
Forwarding to the inner I/O error's `source()` hides the I/O error itself, preventing callers from inspecting its kind through a custom persistence error. Return the immediate cause so generic error classification can find it, while preserving the handle needed to recover from failed persistence. Extend both writable-file and marker recovery tests to check the source. All tempfile tests and doctests pass.
Error inspection rebuilt the entire error graph before returning its first item, so even a root match visited unrelated sources and allocated storage. Use a shared iterator that expands each node only when another item is needed, preserving breadth-first order, concrete types, and caller locations in both error representations. Expose `classify(&error)` for custom borrowed errors, and share classification predicates with `Error` and `Exn`. The shared traversal and predicate definitions remove more code than the borrowed API adds. Keep `probable_cause()` unchanged. Validated default and `auto-chain-error` tests, including source-call counters, custom I/O payloads, nested branches, and doctests. Focused Clippy passes in both modes.
Merge-base traversal replaced object-store and decoding errors with a static message. Missing objects, retryable I/O failures, and custom backend errors therefore lost their causes and classifications at the revision boundary. Return `Exn<Message>` and raise graph insertion context around the original failure. Remove the forwarding `Error` alias and `Simple` type, and adapt the porcelain API to return `gix::Error`. The regression reproduces the lost backend cause before the fix and verifies its concrete I/O kind, missing-resource classification, and retry policy after conversion. All 113 revision tests and the porcelain revision tests pass; `gix` and `gix-merge` compile with the changed signature.
Curl replaces upload and download callback failures with generic transfer errors. Keep the original I/O error alongside curl's diagnostic so custom retry policies and other classifications remain inspectable after conversion to `gix::Error`. Clear the saved callback failure between transfers. Also retain the integer parser's cause when a virtual-host port is invalid. Both conversions previously discarded their sources in `map_err()`. All 58 `gix-transport` tests with `http-client-curl` pass, including network-free regressions for aborted uploads with custom error payloads and failed download pipe writes.
Writing to a byte slice may succeed with a short write. Delta application ignored that byte count, so oversized copy and insert instructions silently truncated their output instead of reporting corrupt data. Split off an output slice of exactly the required size before copying. This also removes two mappings of I/O failures that slice writes cannot produce. A regression covers both copy and insert instructions; all `gix-pack` tests pass.
Fetch ref updates discarded commit decoding and traversal setup errors, treating any such failure as permission to force the update. A malformed local or remote commit could therefore overwrite a ref without a force refspec. Traversal errors were also ignored when looking for the ancestor. Propagate those failures with their original causes and context. Check object kinds explicitly to retain the existing behavior for non-commit targets without mistaking corruption for an object-kind mismatch. All 26 `gix` library tests with `blocking-network-client` pass. Regressions cover malformed commits on both sides, unchanged refs after a failed check, and valid updates involving non-commit targets.
Parsers and adapters discarded encoding, integer, date, signature, and object-access failures when replacing them with context. Preserve their concrete causes so classification and downcasting keep working after conversion to `gix::Error` or an I/O error. Return `Exn` from fallible path, command-line, gitdir, and pack-entry conversions where necessary, and adapt their consumers in the same change. Packed-ref and reflog errors retain their parser sources and input details; reflog recovery reports the actual recovery failure. Loose-object verification now propagates lookup and enumeration failures instead of treating every lookup error as retryable or silently skipping failed enumeration. Remove unnecessary UTF-8 conversions for ASCII suffixes and check span bounds before narrowing. Parsers that only return `()` explicitly destructure it. No production `map_err()` closure still discards a wildcard-bound error. Also preserve causes in formatting-only CLI and commit-graph adapters, where stringification previously lost checksum corruption classifications. Regression coverage includes malformed refs and reflogs, loose-object verification, Windows encoding failures, filesystem stack reuse, and checksum classification. Affected tests and doctests, workspace all-target checks, async checks, Windows cross-checks, and focused Clippy pass. Two macOS discovery tests could not mount disk images in this environment.
Callers can enrich errors with named values they already possess without introducing a custom payload type. `Metadata` keeps a message and an ordered dictionary of typed scalar values, including lossless bytes and native paths. `Error::metadata()` and `Exn::metadata()` iterate separate contexts through existing error traversal, preserving original causes and classifications. Recovery continues to use classifications and concrete domain errors. Document metadata keys on each function that directly returns them. Validation: `cargo test -p gix-error` with default and `auto-chain-error` features; `cargo clippy -p gix-error --all-targets --all-features`.
…y signals Return canonical `Exn` errors from reference operations and `gix::Error` at porcelain boundaries. Preserve native parser, filesystem, lock and custom name-conversion sources instead of rewrapping them in operation-specific enums. Use documented `Metadata` dictionaries for diagnostic paths, reference names, input bytes and positions. Keep concrete signals for absent references, malformed loose references, stale expected values, existing references and missing committer identity. These support GitButler-style recovery without string matching; stale reference values still require reconciliation before retrying. Fetch only treats an absent referent as unborn, propagating malformed referents and read failures. Remove empty error namespaces and duplicate conversions along with their workspace callers. Preserve unterminated packed input and count peeled lines when reporting iterator positions. Validation: `gix-ref` tests with SHA-1 and SHA-256 fixtures; `gix` reference, revision and fetch tests; `gix-discover`, blocking `gix-protocol`, and `gix-tix` tests; workspace all-target checks and Clippy; focused documentation builds. CI `test-fast (windows-latest)` exposed a stale expectation in `loose_iter_with_broken_refs`: `ReferenceCreation` now formats its native path with escaping, so a Windows separator appears as two backslashes. Update the Windows-only expected message to match, retaining the escaped diagnostic. Validation: the focused `gix-ref` iterator regression passes locally; the Windows execution is covered by CI.
Return canonical `Exn` errors for loose and dynamic object lookup, alternate resolution, prefix lookup and integrity verification. Preserve original I/O, decoder, allocation, persistence and custom reader sources instead of forwarding them through operation-specific error enums. Use documented scalar `Metadata` contexts for native paths, object IDs, sizes, pack counts and recursion limits. Keep `alternate::Cycle` with its discovered directory chain, and preserve explicit retryability for interrupted verification or concurrent disk changes. An absent delta base remains not found, while a recursion limit alone implies neither absence nor corruption. Empty loose files are now classified as corruption. Remove empty error namespaces and redundant conversions in porcelain and CLI callers. Keep the genuine I/O boundary for store initialization and pack loading, using the existing adapter to retain both the I/O kind and the complete cause. Replace wrapper-construction tests with actual custom-reader, malformed-object, missing-delta and depth-limit failures. Validate metadata and classifications after conversion, including native path values and retained cycle details. Validation: `gix-odb` tests and doctests with SHA-1 fixtures, and SHA-256 with parallel access; workspace all-target Clippy; warning-denying `gix-odb` docs.
Tasks
gixto reduce all the overhead.not_found()support forgix-referrors.can_retry()duplication ingix-transportEverything below this line was generated by Codex GPT-5.
Created by Codex on behalf of Byron. Byron will review before this is ready to merge.
Reported issue
Refs #2351
Summary
thiserroruse from workspace crates and exposes top-level failures throughgix::Errorgix-erroranyhowin the binaries and forwards thegix-error/anyhowfeature throughgixValidation
cargo check --workspace --all-targetscargo test -p gix-errorcargo test -p gix-error --features auto-chain-error --test auto-chain-errorcargo test -p gix --test gix revision::spec::