Inconsistent Error Handling Behavior with ? operator in Marco #[tokio::main]

[TomMonkeyMan]

Version
Tokio version:

├── tokio v1.41.0
      └── tokio-macros v2.4.0 (proc-macro)

Rust version:

rustc 1.79.0 (129f3b996 2024-06-10)

Reqwest version:

reqwest v0.11.27

Platform

Darwin 22.5.0 Darwin Kernel Version 22.5.0: Thu Jun  8 22:22:22 PDT 2023; 
root:xnu-8796.121.3~7/RELEASE_X86_64 x86_64

Description

When using the #[tokio::main] macro to define an asynchronous main() function, I've encountered inconsistent behavior regarding error handling when returning std::io::Error as part of a Result that has Box<dyn Error> as its error type.
Specifically, in the main() function, it seems necessary to use the ? operator before returning it, otherwise it fails to compile. However, in other async functions, Box::new(std::io::Error) can be returned directly without the ? operator conversion.

For example, in the main() function, the following code is required:

Err(Box::new(std::io::Error::new(
    std::io::ErrorKind::Other,
    "Request failed",
)))?

While in other asynchronous functions, the following code works without the need for ?:

Err(Box::new(std::io::Error::new(
    std::io::ErrorKind::Other,
    "Request failed",
)))

Minimal, Complete, and Verifiable Example (MCVE)

To reproduce the issue, you can use the following minimal example:

Code1:

use reqwest::Client;
use std;

#[tokio::main]
async fn main() -> Result<(), Box<dyn std::error::Error>> {
    let client = Client::new();
    let test_url = "https://www.google.com";
    let request = client.get(test_url);
    let response = request.send().await?;

    if response.status().is_success() {
        println!("request succeeded");
        let content = response.text().await?;
        println!("request content {:#?}", content);
        Ok(())
    } else {
        let status = response.status();
        let error_text = response
            .text()
            .await
            .unwrap_or_else(|_| "Failed to read response text".to_string());
        eprintln!("Request failed with status: {} {:#?}", status, error_text);

        
        Err(Box::new(std::io::Error::new(
            std::io::ErrorKind::Other,
            "Request failed",
        )))?  // This line causes a compile error if `?` are not used (`Box::new()`)
    }
}

Code2:

use reqwest::Client;
use std;

pub async fn test() -> Result<(), Box<dyn std::error::Error>> {
    let client = Client::new();
    let test_url = "https://www.google.com";
    let request = client.get(test_url);
    let response = request.send().await?;

    if response.status().is_success() {
        println!("request succeeded");
        let content = response.text().await?;
        println!("request content {:#?}", content);
        Ok(())
    } else {
        let status = response.status();
        let error_text = response
            .text()
            .await
            .unwrap_or_else(|_| "Failed to read response text".to_string());
        eprintln!("Request failed with status: {} {:#?}", status, error_text);

        Err(Box::new(std::io::Error::new(
            std::io::ErrorKind::Other,
            "Request failed",
        )))
    }
}

#[tokio::main]
async fn main() -> Result<(), Box<dyn std::error::Error>> {
    test().await?;
    Ok(())
}

Notice that in Code1, within the main() function, the error must use the ? operator, while in Code2, the test function, the error can be returned directly without operator ?.

Expected Behavior
I expect both functions to require the same treatment for returning errors. Consistent error handling across all async functions, including main(), would make the code more intuitive and easier to maintain.

Possible Solutions
-Documentation: Clarify in the documentation whether this is expected behavior and why.
-Bug Fix: If it's a bug, fix the behavior so that all async functions, including main, handle return errors consistently.

This issue was also discussed in the community: https://users.rust-lang.org/t/why-must-i-convert-std-error-to-box-dyn-error-in-one-function-but-not-in-another/120141/9

Thank you for looking into this!

[Darksonn]

I guess the issue is that we're not properly informing the closure about the return type...

[TomMonkeyMan]

I guess the issue is that we're not properly informing the closure about the return type...

Thank you for the clarification! It makes sense regarding the closure and return type. I’ll take a closer look and experiment with it a bit more to deepen my understanding of tokio. Appreciate the insights—this is a great learning opportunity for me!

[Darksonn]

If we can fix this in a non-intrusive way, then I'm happy to see a PR for that. But changes to this part of the macro have proven tricky in the past.

[TomMonkeyMan]

Thanks for pointing that out! I fully understand that changes in this area can be tricky. If I come up with any ideas or plans for a potential PR, I’ll be sure to discuss them with you and the other maintainers before making any code changes or submitting anything. I really appreciate your guidance and openness to contributions!

[Darksonn]

You don't have to ask first before creating a PR. Just be prepared that it may (or may not!) be difficult to fix this. We have previously closed several PRs in this part of the codebase because they were incorrect and we couldn't figure out how to fix them.

[TomMonkeyMan]

Haha, thanks for the heads-up! I’ll keep that in mind and do my best.

[TomMonkeyMan]

Hi, apologies for the delayed response; I was a bit occupied recently. I did some digging into the root cause of this ? behavior by expanding the macro with cargo expand.

Code 1 expands into the following:

use reqwest::Client;
use std;

fn main() -> Result<(), Box<dyn std::error::Error>> {
    let body = async {
        let client = Client::new();
        let test_url = "https://www.google.com";
        let request = client.get(test_url);
        let response = request.send().await?;
        if response.status().is_success() {
            println!("request succeeded\n");
            let content = response.text().await?;
            println!("request content {0:#?}", content);
            Ok(())
        } else {
            let status = response.status();
            let error_text = response
                .text()
                .await
                .unwrap_or_else(|_| "Failed to read response text".to_string());
            println!("Request failed with status: {0} {1:#?}\n", status, error_text);
            Err(Box::new(std::io::Error::new(
                std::io::ErrorKind::Other,
                "Request failed",
            )))? // here we need a ?
        }
    };
    
    {
        return tokio::runtime::Builder::new_multi_thread()
            .enable_all()
            .build()
            .expect("Failed building the Runtime")
            .block_on(body);
    }
}

Code 2 expands as follows:

use reqwest::Client;
use std;

async fn test() -> Result<(), Box<dyn std::error::Error>> {
    let client = Client::new();
    let test_url = "https://www.google.com";
    let request = client.get(test_url);
    let response = request.send().await?;
    if response.status().is_success() {
        println!("request succeeded\n");
        let content = response.text().await?;
        println!("request content {0:#?}", content);
        Ok(())
    } else {
        let status = response.status();
        let error_text = response
            .text()
            .await
            .unwrap_or_else(|_| "Failed to read response text".to_string());
        println!("Request failed with status: {0} {1:#?}\n", status, error_text);
        Err(Box::new(std::io::Error::new(
            std::io::ErrorKind::Other,
            "Request failed",
        ))) // here we dont need a ?
    }
}

fn main() -> Result<(), Box<dyn std::error::Error>> {
    let body = async {
        test().await?;
        Ok(())
    };
    {
        return tokio::runtime::Builder::new_multi_thread()
            .enable_all()
            .build()
            .expect("Failed building the Runtime")
            .block_on(body);
    }
}

The inconsistency in using the ? operator appears to stem from differences in error propagation requirements.

        Err(Box::new(std::io::Error::new(
            std::io::ErrorKind::Other,
            "Request failed",
        )))

In the expanded Code 1, since the async block’s body must match the main() function’s Result type, ? is necessary to propagate the error up to main.

In Code 2, the error is returned directly from test(), and propagation is handled by test().await? in the async block within main(). Therefore, in test(), it’s possible to return the error directly without using ?.

@Darksonn Hi, could you help check if this explanation aligns with the intended behavior of the ? operator when using the macro, whenever you have a moment

[nurmohammed840]

Unfortunately, You are out of luck. this can't be fixed with the current implementation due to the use of async { ... } closures.

An attempt (#6882) was made in the past to address this using async fn, similar to what you suggested.

Drop order for async is tricky:

• The fields of the active enum variant are dropped in declaration order.
• The variables that a closure captures by move are dropped in an unspecified order.

Perhaps we can use async fn if there are no arguments involved. Well, I'll give it a try and send a fix!

[TomMonkeyMan]

hi @nurmohammed840, thanks for your detailed response! I'm still a bit new to this, so I wanted to ask: could you help let me know which part of your code changes specifically addresses the issue I mentioned? I'd like to understand the fix better.

Thanks again for your help!

[nurmohammed840]

When a function annotated with #[tokio::main] does not accept any arguments, the macro expands it into:

Input

use std::error::Error;
#[tokio::main]
async fn main() -> Result<(), Box<dyn Error>> {
    Err(Box::new(std::io::Error::new(
        std::io::ErrorKind::Other,
        "Request failed",
    )))
}

Output

fn main() -> Result<(), Box<dyn Error>> {
    async fn main() -> Result<(), Box<dyn Error>> {
        Err(Box::new(std::io::Error::new(
            std::io::ErrorKind::Other,
            "Request failed",
        )))
    }
    ...
}

[TomMonkeyMan]

yeah sure, I get it that you're fixing this issue related to not using a ?. I was just trying to ask which parts/lines in your code changes fixed this issue, as it looks like you're fixing a lot of things.

[nurmohammed840]

5509860

[TomMonkeyMan]

thanks man! I'll get this patch and try locally for a testing.