[jcspencer]

This PR builds on the changes proposed by @cquintana92 in #112 (fix: unknown request types should return failure)

This:

• Extends Request::Unknown to include an payload of type Unparsed.
• Add Session::unknown_message() method to Session trait to allow processing messages not known to the library
• Adds a roundtrip test case for an unknown message.

See #112 for more information on the original issue.

[wiktor-k]

Looks great IMO. We need a rebase on top of main for the CI to be green again 🍃

[jcspencer]

Hi @wiktor-k, should be ready for review now! Probably best reviewed in commit order (sorry for the force pushes!)

A bit of a summary of the updates:

• Compared to fix: unknown request types should return failure #112, this changes Request::Unknown to a tuple struct, since Session methods don't take standalone Requests anywhere else. This could also be split out into it's own struct to keep with convention (e.g. Request::Unknown(UnknownMessage)), but really up to you!
• I've landed on message_type as the name to use in the API here, since we're changing some of the APIs around here anyway
  • I didn't touch ProtoError::UnsupportedCommand, since it's used in quite a number of examples, and not breaking too much is likely desirable.
  • On that note, I've marked {Request, Response}:message_id as deprecated, since it's part of our public API - happy to leave those commits for another PR if you prefer!
• I've marked {Request, Response} as non-exhaustive; I don't think we'd have a reason to change Request again since it's a cover-all now, but we may add a custom response type Response soon, so better to get ahead of a breaking change again!

In the places I've updated stuff here, I've updated references from the RFC draft to the ratified spec; I've opened #116 as a bit of a tracking issue to do the rest of these.

Let me know if you have any issues, and feel free to cherry-pick off of this branch if there's later breaking commits you'd rather merge separately 😄

Thanks!

[wiktor-k]

Looks good. Had only minor remarks. Thank you! 🙏

[jcspencer]

Great, thanks @wiktor-k! Since there's a direct reference to a minor version bump, I'll let you decide how you want to approach the version bump (since I assume we'll bump soon once the rustcrypto packages are bumped too)?

I'll follow this up with some changes around the RFC soon

[wiktor-k]

Great, thanks @wiktor-k! Since there's a direct reference to a minor version bump, I'll let you decide how you want to approach the version bump (since I assume we'll bump soon once the rustcrypto packages are bumped too)?

We definitely need a bump due to changes in here and elsewhere. As for RustCrypto it'd be ideal if we could align on that but last time I checked there were still some rc crates and I don't want to wait too long 😅 (we're not going to ran out of version numbers so I don't think it'll be a problem)

I'll follow this up with some changes around the RFC soon

🥳