Skip to content

add missing impl Errors (breaking change) #130

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

add missing impl Errors (breaking change) #130

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions source/postcard-rpc/src/server/impls/test_channels.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use crate::{
Topic,
};
use core::fmt::Arguments;
use thiserror::Error;
use tokio::{select, sync::mpsc};

//////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -212,9 +213,10 @@ impl WireTx for ChannelWireTx {
}

/// A wire tx error
#[derive(Debug)]
#[derive(Debug, Error)]
pub enum ChannelWireTxError {
/// The receiver closed the channel
#[error("channel closed")]
ChannelClosed,
}

Expand Down Expand Up @@ -279,11 +281,13 @@ impl WireRx for ChannelWireRx {
}

/// A wire rx error
#[derive(Debug)]
#[derive(Debug, Error)]
pub enum ChannelWireRxError {
/// The sender closed the channel
#[error("channel closed")]
ChannelClosed,
/// The sender sent a too-large message
#[error("message too large")]
MessageTooLarge,
}

Expand Down
27 changes: 18 additions & 9 deletions source/postcard-rpc/src/server/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,13 @@ pub mod impls;

use core::{fmt::Arguments, ops::DerefMut};

use postcard_schema::Schema;
use serde::Serialize;

use crate::{
header::{VarHeader, VarKey, VarKeyKind, VarSeq},
DeviceMap, Key, TopicDirection,
};
use postcard_schema::Schema;
use serde::Serialize;
use thiserror::Error;

//////////////////////////////////////////////////////////////////////////////
// TX
Expand Down Expand Up @@ -77,21 +77,24 @@ pub trait WireTx {
}

/// The base [`WireTx`] Error Kind
#[derive(Debug, Clone, Copy)]
#[derive(Debug, Clone, Copy, Error)]
#[non_exhaustive]
pub enum WireTxErrorKind {
/// The connection has been closed, and is unlikely to succeed until
/// the connection is re-established. This will cause the Server run
/// loop to terminate.
#[error("connection closed")]
ConnectionClosed,
/// Other unspecified errors
#[error("other")]
Other,
/// Timeout (WireTx impl specific) reached
#[error("timeout reached")]
Timeout,
}

/// A conversion trait to convert a user error into a base Kind type
pub trait AsWireTxErrorKind {
pub trait AsWireTxErrorKind: core::error::Error {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This and line 150 makes this a breaking change according to cargo semver-checks:

james@magician ➜  postcard-rpc git:(add-missing-Error-impls) cargo semver-checks
    Building postcard-rpc v0.11.12 (current)
       Built [   9.633s] (current)
     Parsing postcard-rpc v0.11.12 (current)
      Parsed [   0.016s] (current)
    Building postcard-rpc v0.11.12 (baseline)
       Built [   9.127s] (baseline)
     Parsing postcard-rpc v0.11.12 (baseline)
      Parsed [   0.015s] (baseline)
    Checking postcard-rpc v0.11.12 -> v0.11.12 (no change)
     Checked [   0.020s] 153 checks: 152 pass, 1 fail, 0 warn, 11 skip

--- failure trait_added_supertrait: non-sealed trait added new supertraits ---

Description:
A non-sealed trait added one or more supertraits, which breaks downstream implementations of the trait
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#generic-bounds-tighten
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.41.0/src/lints/trait_added_supertrait.ron

Failed in:
  trait postcard_rpc::server::AsWireRxErrorKind gained Error in file /Users/james/personal/postcard-rpc/source/postcard-rpc/src/server/mod.rs:150
  trait postcard_rpc::server::AsWireTxErrorKind gained Error in file /Users/james/personal/postcard-rpc/source/postcard-rpc/src/server/mod.rs:97

     Summary semver requires new major version: 1 major and 0 minor checks failed
    Finished [  22.978s] postcard-rpc

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are these traits supposed to be implemented by anyone outside of postcard-rpc? if not then it wouldn't be breaking, right?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are public, so they can be implemented by anyone: https://docs.rs/postcard-rpc/latest/postcard_rpc/server/index.html#traits

/// Convert the error type into a base type
fn as_kind(&self) -> WireTxErrorKind;
}
Expand Down Expand Up @@ -127,21 +130,24 @@ pub trait WireRx {
}

/// The base [`WireRx`] Error Kind
#[derive(Debug, Clone, Copy)]
#[derive(Debug, Clone, Copy, Error)]
#[non_exhaustive]
pub enum WireRxErrorKind {
/// The connection has been closed, and is unlikely to succeed until
/// the connection is re-established. This will cause the Server run
/// loop to terminate.
#[error("connection closed")]
ConnectionClosed,
/// The received message was too large for the server to handle
#[error("the received message was too large for the server to handle")]
ReceivedMessageTooLarge,
/// Other message kinds
#[error("other")]
Other,
}

/// A conversion trait to convert a user error into a base Kind type
pub trait AsWireRxErrorKind {
pub trait AsWireRxErrorKind: core::error::Error {
/// Convert the error type into a base type
fn as_kind(&self) -> WireRxErrorKind;
}
Expand Down Expand Up @@ -384,15 +390,18 @@ where
}

/// A type representing the different errors [`Server::run()`] may return
#[derive(Debug, Error)]
pub enum ServerError<Tx, Rx>
where
Tx: WireTx,
Rx: WireRx,
{
/// A fatal error occurred with the [`WireTx::send()`] implementation
TxFatal(Tx::Error),
#[error("A fatal error occurred while transmitting")]
TxFatal(#[source] Tx::Error),
/// A fatal error occurred with the [`WireRx::receive()`] implementation
RxFatal(Rx::Error),
#[error("A fatal error occurred while receiving")]
RxFatal(#[source] Rx::Error),
}

impl<Tx, Rx, Buf, D> Server<Tx, Rx, Buf, D>
Expand Down
Loading