view: make OwnedView::to_owned_message infallible#268
Merged
Conversation
v0.8.1 was released from the v0.8.x branch (cut from the v0.8.0 tag, so the unreleased breaking changes on main stay out of it). Bring main's changelog up to date: add .changes/0.8.1.md with the released section, fold it into CHANGELOG.md via changie merge, and remove the now-shipped unknown-field accounting fragment from .changes/unreleased so the 0.9.0 release notes do not report the fix a second time.
|
All contributors have signed the CLA ✍️ ✅ |
Every OwnedView constructor wire-decodes its view (decode, decode_with_options, from_owned), and a view produced by wire decoding always converts — since 0.8.1, decode-time unknown-field accounting charges exactly what conversion re-materializes and conversion replays under the recorded budget and group-nesting depth. The Result on OwnedView::to_owned_message therefore only ever encoded an unreachable error path, which downstream RPC glue was papering over with internal unwraps. Drop it: the method (and the generated FooOwnedView wrapper method) now returns the owned message directly, with #[must_use] preserving the discarded-result lint the implicit Result used to provide. The MessageView trait method deliberately stays fallible: hand-written impls and push_raw-built views can legitimately fail, and plain view types cannot carry wire-decode provenance in their own type. The trait doc now cross-references the infallible OwnedView sibling. A contract violation — a buggy hand-written MessageView impl wrapped in OwnedView, or a from_parts view without wire-decode provenance — panics with a descriptive message via a non-generic #[cold] path (format machinery emitted once, not per monomorphization), pinned by a should_panic test. from_parts' safety contract is strengthened accordingly: the view must have been produced by wire-decoding the buffer, not merely borrow from it. The examples/bsr-quickstart generated code intentionally lags: it is produced by the published BSR plugin (pinned), already does not compile on main against the current runtime, and is regenerated when the next plugin ships.
729baf6 to
c541146
Compare
azdagron
approved these changes
Jul 1, 2026
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
For 0.9.0:
OwnedView::to_owned_messageand the generatedFooOwnedView::to_owned_messagebecome infallible, returning the owned message directly. Requested off the back of the connect-rust workstream, which currently unwraps internally so RPC handlers don't see a fallible conversion — this fixes it at the right layer.Why this is sound
Every
OwnedViewconstructor wire-decodes its view (decode,decode_with_options,from_owned),Clonepreserves provenance (refcount bump + view copy including the conversion budget), noDeserialize/Frompath constructs one otherwise, andunsafe from_parts' safety contract is strengthened to require wire-decode provenance. Combined with the 0.8.1 guarantee (decode charges exactly what conversion re-materializes, and replay runs under the recorded budget and nesting depth), theResultonly ever encoded an unreachable error path. A contract violation — a buggy hand-writtenMessageViewimpl, or afrom_partsbreach — panics with a descriptive message via a non-generic#[cold]path, pinned by ashould_panictest.The
MessageViewtrait method deliberately staysResult: hand-written impls andpush_raw-built views can legitimately fail, and a plain view type cannot carry provenance. The trait doc now cross-references the infallible sibling. The lazy family (genuinely fallible, deferred validation) is untouched.Structure
Two commits: (1) changelog bookkeeping recording the shipped v0.8.1 maintenance release on main (adds
.changes/0.8.1.md, removes the shipped #266 fragment from unreleased so 0.9.0 notes don't double-report it — also makes this PR's "since 0.8.1" citation resolvable in main's changelog); (2) the breaking change itself, with regenerated WKT + descriptor code and aBreaking changesfragment carrying explicit migration guidance (including thefrom_partsaudit note).Notes for review
cargo semver-checks -p buffa --baseline-version 0.8.1: the one failure (message_field_always_presentgeneric-param change) is pre-existing on main; the return-type change here isn't in its lint set but is a break by inspection — hence the fragment.examples/bsr-quickstartgains one more expected mismatch: it's generated by the published BSR plugin (pinned), already doesn't compile on main against the current runtime, and is workspace-excluded/non-CI. It regenerates when the 0.9.0 plugin ships — left untouched deliberately.OwnedViewviafrom_parts(their dispatch glue satisfies its contract exactly); a provenance-gated trait variant is the library-side alternative if we want it later.Verified: full workspace tests, clippy (all-targets, all-features), rustdoc
-D warningson published crates, conformance 14/14, and a consumer-crate check driving the wrapper, the genericOwnedViewover a 1000-record unknown-field payload, Clone provenance, the documented panic text from a downstream contract-breaking impl, the negative compile probe (trait staysResult), and the#[must_use]warning.