Unable to understand Rust's error handling

109 Views Asked by At

I have a Rocket.rs server and in one of its routes, I'm calling a function that finds documents of a collection and loops over it.

Find documents of a collection:

let participations: Cursor<Document> = collection.aggregate(pipeline, None).await.expect("Failed to fetch documents.");

Now after fetching the documents, it returns a Cursor and then I call advance() on it to loop over the cursor. The problem arises while using is_ok() instead of expect() on advance() result value.

When doing this my code works fine without any errors.

while participations.advance().await.expect("Failed to advance.") {
    let participation_doc = participations.deserialize_current();
}

But when I switch to this.

while participations.advance().await.is_ok() {
    let participation_doc = participations.deserialize_current();
}

My code fails with an error.

thread 'rocket-worker-thread' panicked at 'called `Option::unwrap()` on a `None` value'

After debugging I found out that this error is encountered when calling participations.deserialize_current();. It throws an error because advance() call was not successful. But why my loop was executed in the first place if the advance() call was not successful is_ok() should've returned false. And why does my code work fine when using expect(). If there's an error then expect() should make my code panic.

What I want is my code to not panic even if advance() fails that's why I was using is_ok(). Any help would be appreciated.

2

There are 2 best solutions below

2
Aurel Bílý On BEST ANSWER

As the comments have pointed out, you didn't specify which library this advance() call comes from, but I'm assuming it is Cursor::advance from MongoDB.

The signature of that method is:

pub async fn advance(&mut self) -> mongodb::error::Result<bool>;

So, it is an asynchronous method which will either give you a Boolean, or a MongoDB error. As the docs say:

The return value indicates whether new results were successfully returned (true) or if the cursor has been closed (false).

By awaiting, you get a value of mongodb::error::Result<bool>. Then, you do two different things with it in your code snippets:

  • result.expect("Failed to advance.") (similar to result.unwrap())

    Here, you are saying "assume there was no MongoDB error (or else panic), then check whether there are more values available".

  • result.is_ok()

    Here, you are saying "check whether there was a MongoDB error".

Notice that in the second case, you are not actually using the Boolean value. (Did the compiler not warn you about this by any chance?) Maybe the API could be designed a bit better, but the first variant is slightly better. If you also want to gracefully handle errors, you want to check both that there is no MongoDB error, and that there are more values available from the Cursor.

Something like this:

loop {
    match participations.advance().await {
        Ok(true) => {
            let participation_doc = participations.deserialize_current();
            todo!(); // do things with participation_doc ...
        }
        Ok(false) => break, // done with the collection
        Err(err) => todo!(), // handle err somehow
    }
}
0
Nabeel Naveed On

tldr: result usage here is for internal errors, the actual value i.e Result<Ok(),Error> is used to indicate the presence of a value.

The return type of advance returns a Result<bool> , the way they have designed this result is for it to error if the issue is internal with the connection to the db itself, they are using true ,false to indicate the absence of a value instead of handling the result you should handle the returned value. in your first line it will return a true or false value which while can understand, in your second your are checking for the result itself whether its Ok or an Err(err) , even when it returns Ok(false) your second code snippet will run fine which is wrong.