aembke / redis-protocol.rs

A Rust implementation of RESP2 and RESP3
Apache License 2.0
40 stars 14 forks source link

Invalid frame kind on Frame::Error #6

Closed alex88 closed 5 years ago

alex88 commented 5 years ago

First of all thank you very much for this crate, it helped a lot!

Now, I've setup a tokio server that uses Framed to decode/encode using this crate, the encode function is:

impl Encoder for RedisCodec {
    type Item = Frame;
    type Error = io::Error;

    fn encode(&mut self, item: Self::Item, dst: &mut BytesMut) -> Result<(), Self::Error> {
        match encode_bytes(dst, &item) {
            Ok(_l) => return Ok(()),
            Err(e) => {
                println!("{:?} {:?}", e, item);
                return Err(io::Error::new(ErrorKind::Other, e.description()));
            }
        }
    }
}

trying to send this:

framed_socket.send(Frame::Error("foo".into())).await;

I get this:

RedisProtocolError { desc: "Invalid frame kind.", kind: EncodeError, context: None }

sending bulk strings works fine but I wanted to send an error when the command was unrecognized and I've tried even with different strings like ERR test to match what redis sends but with no luck, is there a way to understand better what the encoding error is?

Update: btw I'm using nightly-2019-09-23 (to be able to use tokio 0.2)

aembke commented 5 years ago

Yep that's my fault. I wrote this thinking it would only be used on Redis clients talking to an actual Redis server, and not as much for more general use implementing the protocol generically. When sending commands to a Redis server the protocol doesn't indicate a use case for sending Errors back to the server, so I didn't implement that code path. Right now you can only encode Bulkstrings, Null, and Arrays, and errors aren't supported.

https://github.com/aembke/redis-protocol.rs/blob/master/src/encode.rs#L79

I'll patch that soon and implement the branch for errors and other types and then this will work.

alex88 commented 5 years ago

Oh sorry, I've missed that, I'm a beginner at rust so a lot of parts aren't clear to me 😄

aembke commented 5 years ago

No worries, I totally forgot to document that anyways. I'll shoot to get a fix out over the weekend.

aembke commented 5 years ago

This is implemented in 1.0.0 on crates.io now. I've been meaning to add a few things and update this to 1.x, so you'll have to update your toml to that.

alex88 commented 5 years ago

Awesome thank you @aembke!