lunatic-rs icon indicating copy to clipboard operation
lunatic-rs copied to clipboard

Abstract process panics when using different Serializer in `RequestHandler`

Open tqwewe opened this issue 2 years ago • 4 comments

Given the following code:

use lunatic::{
    process::{AbstractProcess, ProcessRef, Request, RequestHandler, StartProcess},
    serializer::Json,
};

struct MyProcess;

impl AbstractProcess for MyProcess {
    type State = Self;
    type Arg = ();

    fn init(_: ProcessRef<Self>, _arg: ()) -> Self::State {
        MyProcess
    }
}

impl RequestHandler<i32, Json> for MyProcess {
    type Response = i32;
    fn handle(_state: &mut Self::State, _request: i32) -> Self::Response {
        1
    }
}

fn main() {
    let my_process = MyProcess::start((), None); // Ok
    my_process.request(1); // Panic
}

Lunatic panics with the following:

thread '' panicked at 'called Result::unwrap() on an Err value: DeserializationFailed(Bincode(Custom("invalid value: integer 1699881595, expected variant index 0 <= i < 3")))', ~/.cargo/registry/src/github.com-1ecc6299db9ec823/lunatic-0.10.0-alpha.1/src/mailbox.rs:162:47

It seems like Bincode is still being used somewhere, even though my request is only implemented for json. Request::<_, Json>::requst(&my_process, 1); also doesn't seem to work.

Is this a bug in Lunatic? Or is there something I'm doing wrong here?

tqwewe avatar Aug 15 '22 03:08 tqwewe

This is a bug. I fixed it with this commit: https://github.com/lunatic-solutions/lunatic-rs/commit/7355868e548bfcb1f6bb27266273bc42f5327e8e

To give you a bit more context. The AbstractProcess always uses bincode for some internal encodings (pointer of handler functions). And I mistakenly used the wrong serializer here (S instead of Bincode). However, the messages should always use the specified serializer.

bkolobara avatar Aug 15 '22 11:08 bkolobara

I also pushed out a release of the rust library containing the fix: v0.10.6. So you can directly depend on it, instead of the old lunatic-0.10.0-alpha.1 one.

bkolobara avatar Aug 15 '22 11:08 bkolobara

This seemed to solve the code example I posted, but if you change type Arg = () into type Arg = serde_json::Value then it fails with:

thread '' panicked at 'called Result::unwrap() on an Err value: DeserializationFailed(Bincode(DeserializeAnyNotSupported))', ~/.cargo/registry/src/github.com-1ecc6299db9ec823/lunatic-0.10.6/src/mailbox.rs:42:41

Updated code snippet
use lunatic::{
    process::{AbstractProcess, ProcessRef, Request, RequestHandler, StartProcess},
    serializer::Json,
};
use serde_json::json;

struct MyProcess;

impl AbstractProcess for MyProcess {
    type State = Self;
    type Arg = serde_json::Value;

    fn init(_: ProcessRef<Self>, _arg: serde_json::Value) -> Self::State {
        MyProcess
    }
}

impl RequestHandler<i32, Json> for MyProcess {
    type Response = i32;
    fn handle(_state: &mut Self::State, _request: i32) -> Self::Response {
        1
    }
}

fn main() {
    let my_process = MyProcess::start(json!(10.0), None); // Ok
    my_process.request(1); // Panic
}

tqwewe avatar Aug 15 '22 11:08 tqwewe

I never added support for a custom serializer for init arguments. Currently it always uses Bincode, but we could allow switching it out with something like this:

impl AbstractProcess<Json> for MyProcess {}

I don't think type arguments are allowed in Rust to have default value (Bincode)?:

impl AbstractProcess<Json> for MyProcess {
    type State = Self;
    type Arg = serde_json::Value;
    type Serializer = Json;
}

We would just need to propagate the serializer type to all the start<T> and starter<T> types, so that you don't use Bincode by default when spawning the process. Feel free to submit a PR for it!

bkolobara avatar Aug 15 '22 12:08 bkolobara

I'm going to close this for now since it's quite a large change and main branch has become quite out of sync. If we need to come back to this, we can reopen this issue.

tqwewe avatar Oct 21 '22 06:10 tqwewe