tokio-rs / tracing

Application level tracing for Rust.
https://tracing.rs
MIT License
5.29k stars 694 forks source link

`[tracing::instrument]` triggers `async_yields_async` with `actix_web` route handler #1450

Open Tseyang opened 3 years ago

Tseyang commented 3 years ago

Bug Report

Version

On v1.53.0-nightly.

🕙[20:11:56] ❯ cargo tree | grep tracing
│   │   │   └── tracing v0.1.26
│   │   │       ├── tracing-attributes v0.1.15 (proc-macro)
│   │   │       └── tracing-core v0.1.18
├── tracing v0.1.26 (*)
├── tracing-futures v0.2.5
│   └── tracing v0.1.26 (*)
└── tracing-subscriber v0.2.19
    ├── tracing v0.1.26 (*)
    ├── tracing-core v0.1.18 (*)
    ├── tracing-log v0.1.2
    │   └── tracing-core v0.1.18 (*)
    └── tracing-serde v0.1.2
        └── tracing-core v0.1.18 (*)

Platform

🕙[20:13:37] ❯ uname -a
Darwin Lims-MBP.attlocal.net 20.5.0 Darwin Kernel Version 20.5.0: Sat May  8 05:10:33 PDT 2021; root:xnu-7195.121.3~9/RELEASE_X86_64 x86_64

Description

I tried adding tracing::instrument to a function handling an actix_web endpoint and it triggered clippy's async_yields_async lint. Without the decorator, this doesn't appear. The minimal repro I managed to find was:

Running cargo clippy -- -D warnings on

use actix_web::web;
use actix_web::{App, HttpRequest, HttpResponse, HttpServer};

#[actix_web::main]
async fn main() {
    HttpServer::new(move || App::new().route("/route1", web::get().to(handler1))).run();
}

// Removing the line below makes the lint go away
#[tracing::instrument(name = "route1")]
async fn handler1(_req: HttpRequest) -> HttpResponse {
    HttpResponse::Ok().finish()
}

I expected nothing to happen, but instead:

    Checking async_yields_async v0.1.0 (/Users/Yangz/Documents/async_yields_async)
error: an async construct yields a type which is itself awaitable
  --> src/main.rs:10:54
   |
10 |   async fn handler1(_req: HttpRequest) -> HttpResponse {
   |  ______________________________________________________^
11 | |     HttpResponse::Ok().finish()
12 | | }
   | | ^
   | | |
   | |_outer async construct
   |   awaitable value not awaited
   |
   = note: `#[deny(clippy::async_yields_async)]` on by default
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#async_yields_async
help: consider awaiting this value
   |
10 | async fn handler1(_req: HttpRequest) -> HttpResponse {
11 |     HttpResponse::Ok().finish()
12 | }.await
   |

error: aborting due to previous error

error: could not compile `async_yields_async`

To learn more, run the command again with --verbose.

When I comment out the attribute:

    Checking async_yields_async v0.1.0 (/Users/Yangz/Documents/async_yields_async)
    Finished dev [unoptimized + debuginfo] target(s) in 0.59s
miterst commented 1 year ago

@Tseyang I'm not sure if this is really a tracing::instrument problem. There's impl Future for HttpResponse ... for older versions of actix_web (ex. version 4.0.0-beta.1 re-exposes this: https://docs.rs/actix-http/3.0.0-beta.1/src/actix_http/response.rs.html#284).

In that case the clippy lint saying async_yields_async is right.

This should't be a problem anymore with the newer versions of actix_web.

Tseyang commented 1 year ago

I haven't really tested this since opening the issue so feel free to close if its no longer relevant.