Overuse of interledger_packet::errors::ParseError

[koivunej]

The most popular case is ParseError::InvalidPacket which requires allocation of a String, which is most often a static string but might be result of a format!.

Some variants like ParseError::WrongType are never constructed, instead ParseError::InvalidPacket is used:

• https://github.com/interledger-rs/interledger-rs/blob/8820e8de7f5f14644032346fce9e58ab4c20d1fd/crates/interledger-packet/src/packet.rs#L40
• https://github.com/interledger-rs/interledger-rs/blob/8820e8de7f5f14644032346fce9e58ab4c20d1fd/crates/interledger-packet/src/packet.rs#L54-L57
• recheck at https://github.com/interledger-rs/interledger-rs/blob/8820e8de7f5f14644032346fce9e58ab4c20d1fd/crates/interledger-packet/src/packet.rs#L539-L544

Going over through all calls to ParseError::InvalidPacket and replacing them with actual variants might be the way forward. Since ParseError is also used from other places (ccp, stream), and in relation to #704 it might be better to separate OER errors from the interledger-packet errors. This coupling currently forces the #704 case to be quite invisible since when decryption fails:

1. It is reported as a String https://github.com/interledger-rs/interledger-rs/blob/8820e8de7f5f14644032346fce9e58ab4c20d1fd/crates/interledger-stream/src/packet.rs#L143-L144
2. The string gets immediatedly dropped on the next callsite https://github.com/interledger-rs/interledger-rs/blob/8820e8de7f5f14644032346fce9e58ab4c20d1fd/crates/interledger-stream/src/server.rs#L285-L286

If instead interledger-stream would use smaller custom enum, like below, it would allow more clear separation for "bad packet" and "failed to decrypt".

enum InvalidPacket {
   // this assumes that interledger_packet::oer::* functionality would
   // report EncodingError instead of ParseError
   Encoding(interledger_packet::EncodingError),
   FailedToDecrypt,
}

Tagging this as breaking, as at least the interledger-packet apis are public. Inside {ccp,stream} the errors should not leak, and changes should only affect the crate-internal API.