Stuck in Option<Option<>> tarpit and can't get out

Hi everyone.

I've a small nom parser to parse redis RESP strings and a struct to put the values in.

Parser:

map(
                tuple((tag_no_case("$2\r\nPX\r\n"), parse_resp_string)),
                |(_expire_option, milliseconds)| {
                    milliseconds
                        .parse::<usize>()
                        .map_or_else(
                            |_| Err(RedisError::ParseIntError),
                            |milliseconds| Ok(SetCommandExpireOption::PX(milliseconds)),
                        )
                        .ok()
                },
            ),

And that's supposed to go into the struct

#[derive(Clone, Debug)]
pub struct SetCommandParameters {
    pub key: String,
    pub value: String,
    pub option: Option<SetCommandSetOption>,
    pub get: Option<bool>,
    pub **expire**: Option<SetCommandExpireOption>,
}

#[derive(Debug, Clone, Copy)]
pub enum SetCommandExpireOption {
    EX(usize),
    PX(usize),
    EXAT(usize),
    PXAT(usize),
    KEEPTTL
}

The problem is, what I get back from the parser is Option<Option<SetCommandExpireOption>> but I need a simple Option<SetCommandExpireOption>.

And not matter what I try I can't produce a simple Option. flatten() doesn't work since it only operates on iterators, so I'm truly stuck here.

How can I get an Option<> out of Option<Option<>> ?

Thank you in advance.

p.s. full source code here redis/src/parsers.rs at master · devfire/redis · GitHub for reference. I went down this journey because .expect() on int parse failures was closing the whole redis connection so I wanted to "handle" the error appropriately and deal with it.

match x {
  Some(Some(y)) => Some(y),
  _ => None
}

^-- untested ,but I think this does it

Hi!

In this case, I think it’s best for you to use another combinator: map_opt.

@@ -89,14 +90,10 @@ fn parse_set(input: &str) -> IResult<&str, RedisCommand> {
                     SetCommandExpireOption::EX(seconds.parse().expect("Seconds conversion failed"))
                 },
             ),
-            map(
+            map_opt(
                 tuple((tag_no_case("$2\r\nPX\r\n"), parse_resp_string)),
                 |(_expire_option, milliseconds)| {
-                    SetCommandExpireOption::PX(
-                        milliseconds
-                            .parse()
-                            .expect("Milliseconds conversion failed"),
-                    )
+                    milliseconds.parse().ok().map(SetCommandExpireOption::PX)
                 },
             ),
         ))),

This allows you to use a Option<SetCommandExpireOption> as the return type for the closure, while preserving SetCommandExpireOption (instead of Option<SetCommandExpireOption>) as the "desired" parser output. A small nuance.

2 Likes

Not sure what do you mean here - this is a method on Option<Option<T>>, without any additional requirements. Could you share the snippet with this error?

5 Likes

OK this definitely looks promising--thank you!

However, I lost the error return.

So, now when I send set foo bar px asdf it's happy to take it, even though asdf is an invalid expiry number, obviously.

How can I get my error back?

Yep, so there's no place to add .flatten() here, it fails on this snippet:

 map(
        tuple((tag_no_case("$2\r\nEX\r\n"), parse_resp_string)),
         |(_expire_option, seconds)| {
              seconds
                .parse::<usize>()
                 .map(|seconds| SetCommandExpireOption::EX(seconds))
                 .map_err(|_| RedisError::ParseIntError)
                 .ok()
                },
            ),

and when I try to append .flatten() at the end of .ok(), I get this:

error: Option<SetCommandExpireOption> is not an iterator
label: Option<SetCommandExpireOption> is not an iterator

note: the following trait bounds were not satisfied:
Option<SetCommandExpireOption>: Iterator
which is required by &mut Option<SetCommandExpireOption>: Iterator
label: Option<SetCommandExpireOption> is not an iterator

but the enum assignment fails with

mismatched types
expected enum `Option<SetCommandExpireOption>`
   found enum `Option<Option<SetCommandExpireOption>>`

So, that didn't work :frowning:

 map(
                tuple((tag_no_case("$2\r\nPX\r\n"), parse_resp_string)),
                |(_expire_option, milliseconds)| {
                    let msecs = milliseconds
                        .parse::<usize>()
                        .map_or_else(
                            |_| Err(RedisError::ParseIntError),
                            |milliseconds| Ok(SetCommandExpireOption::PX(milliseconds)),
                        )
                        .ok();

                    match msecs {
                        Some(y) => Some(y),
                        None => todo!(),
                    }

                    // seconds
                    //     .parse::<usize>().into_iter()
                    //     .filter_map(|seconds: usize|SetCommandExpireOption::EX(seconds))
                    // .map(|seconds| SetCommandExpireOption::EX(seconds))
                    // .or_else(|_| Err(RedisError::ParseIntError))
                },
            )

Nested Somes didn't work at all, it's an Option<Option<>> coming out of the parser, not the map. And the code above is simply right back where I started.

If you want to propagate the error, use map_res instead.

By the way, if you want to learn more about how to use nom, I recommend you to read its documentation. For this specific case, the document on error management is especially relevant: nom/doc/error_management.md at main · rust-bakery/nom · GitHub

It looks like you are calling flatten() on the inner Option<SetCommandExpireOption>, when it needs to be called on a value presumably outside the call to map. I think that would be here redis/src/parsers.rs at master · devfire/redis · GitHub

Unfortunately, that's not even an option. .flatten() is not even offered as a possibility on expire.

Yes, sorry. I did read that link nom bakery before but couldn't see how it would apply.

I refactored it with map_res like you suggested:

map_res(
                tuple((tag_no_case("$2\r\nEX\r\n"), parse_resp_string)),
                |(_expire_option, seconds)| {
                    seconds
                        .parse::<usize>()
                        .map(|seconds| SetCommandExpireOption::EX(seconds))
                        .or_else(|_| Err(RedisError::ParseIntError))
                },
            )

but it happily eats invalid input and returns OK.

I know I'm being dense, sorry but what am I missing here? I'm new to rust & nom so it's likely something obvious...

fn foo<T>(x: Option<Option<T>>) -> Option<T> {
    match x {
        Some(Some(y)) => Some(y),
        _ => None,
    }
}

Why doesn't it work? That compiles fine for me.

I see in your repository that this code ends up wrapped into an opt combinator. This combinator will turn recoverable errors into None.
However, map_res will wrap the error into a nom::Err::Error, a recoverable error.
If you need the parser to fail in an unrecoverable way, it’s required to put the error into a nom::Err::Failure.

There are several avenues to address this. Off the top of my head:

  • Use the cut combinator to turn the recoverable error into an unrecoverable error.
  • Write a custom combinator which returns Err::Failure when milliseconds.parse() fails.

(To learn more about that, you can read this document.)

To be fair, I don’t think nom is a particularly easy-to-use library, especially for newcomers. I understand it can be hard to wrap one’s head around it.
If you still can’t get something working after the pointers I gave above, send us a minimal example reproducing your problem in Rust playground, so that we can actually run it directly in the playground.
Also include the input you want to test against.
This way, it will be easier for people to modify the code and give a concrete suggestion working for you.

Incidentally, the idiomatic way of expressing or_else(|…| Err(…)) is map_err(|…| …).

4 Likes

What we need could be and_then of Option in std::option - Rust

Some(Some(1)).and_then(|x| x)

Maybe I'm missing the problem but would calling unwrap_or(None) work

like so?

    let x = Some(Some(123));
    let y = x.unwrap_or(None);

You need to look at the repository to get a full view of the problem, but it also involves nom and a few extra considerations.

1 Like

Correct it's not as simple as that unfortunately.

1 Like

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.