fix: Clean up and decouple interledge-packet::ParseError in other crates #722
Conversation
whalelephant
force-pushed
the
packet-errors
branch
from
011deda
to
bd3a168
Compare
There was a problem hiding this comment.
Overall I think this looks very good; you updated it during my scan through, so some things might already be good to resolve.
There are some string nits; perhaps the biggest issue is with the changed messages. Are you sure those can change? I think I reverted some changes as well in the bytes 0.5 upgrade, so I'd be inclined to keep the external behaviour (the messages) as the same by default when refactoring.
Either way, I think at least the interledger-packet::Error commits need a ! to signify a breaking change (since those at minimum are exported).
EDIT: thought about it a bit, expanded the I'd be inclined to keep the external behaviour (the messages) as the same by default when refactoring.
whalelephant
force-pushed
the
packet-errors
branch
4 times, most recently
from
3246d40
to
54738fc
Compare
| #[error("UTF-8 Error: {0}")] | ||
| Utf8Err(#[from] Utf8Error), | ||
| #[error("UTF-8 Conversion Error: {0}")] | ||
| FromUtf8Err(#[from] FromUtf8Error), | ||
| #[error("Chrono Error: {0}")] | ||
| ChronoErr(#[from] chrono::ParseError), |
There was a problem hiding this comment.
Just a note for the follow-ups: I dont think there's any point in carrying this around either. If the timestamp isn't good it doesn't really matter how bad is it. I think chrono::ParseError is quite uninformative as well.
| pub(crate) fn try_from_without_expiry(prepare: &Prepare) -> Result<Self, CcpPacketError> { | ||
| let destination = prepare.destination(); | ||
| if destination != *CCP_CONTROL_DESTINATION { | ||
| return Err(CcpPacketError::UnexpectedDestination(destination)); | ||
| } | ||
|
|
||
| if prepare.execution_condition() != PEER_PROTOCOL_CONDITION { |
There was a problem hiding this comment.
Interesting aside note: This check is duplicated along with it's log stmt... Probably not great to log and return an error but ok to clean this up later.
|
The latest |
|
Rerunning as the other one failed to start as well. |
|
Github actions is having issues, we'll need to restart later. |
Introduce TrailingBytesError, DataTypeError, PacketTypeError Signed-off-by: bwty <whalelephant@users.noreply.github.com>
Signed-off-by: bwty <whalelephant@users.noreply.github.com> Signed-off-by: bwty <whalelephant@users.noreply.github.com>
Signed-off-by: bwty <whalelephant@users.noreply.github.com>
Signed-off-by: bwty <whalelephant@users.noreply.github.com>
Signed-off-by: bwty <whalelephant@users.noreply.github.com>
whalelephant
force-pushed
the
packet-errors
branch
from
ca4123c
to
e664d03
Compare
cc #708
This attempts to decouple the error type
ParseErrorin interledger-packet with other crates.The idea is to have more explicit errors that are more efficient.