seanmonstar / reqwest

An easy and powerful Rust HTTP Client
https://docs.rs/reqwest
Apache License 2.0
9.82k stars 1.11k forks source link

reqwest::blocking should advertise that it doesn't like tokio #1233

Open ppershing opened 3 years ago

ppershing commented 3 years ago

This bug was extremely hard to track down. I was wondering why my tokio runtime was failing even when spawned on separate thread. Note that I do know that running blocking tasks on tokio isn't a good idea. But I was trying to slowly migrate from blocking to non-blocking and this was interim state.

What would be great is 1) the documentation should explicitly state that instantiating client::blocking inside tokio ends up with fireworks 2) reqwest::blocking::Client should actually use tokio's enter(false) to check for runtime-inside-runtime and at least should fatal early on (e.g. something like "You should not run reqwest::blocking inside existing tokio runtime")

The smallest repro is

use reqwest;
use tokio;

#[tokio::main(flavor = "current_thread")]
async fn worker() {
    println!("worker started!");
    let client = reqwest::blocking::Client::new();
    println!("worker ended");
}

pub fn main() {
    worker();
}

Running this fails with

worker started!
thread 'main' panicked at 'Cannot drop a runtime in a context where blocking is not allowed. This happens when a runtime is dropped from within an asynchronous context.', /home/ppershing/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.4.0/src/runtime/blocking/shutdown.rs:51:21
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

which isn't helpful at all if it happens inside larger codebase

seanmonstar commented 3 years ago

Good point! reqwest does try to explicitly blowup early, by checking if it can create a runtime as a debug assertion. But it seems the panic message has changed, and so it's less clear. It would be a good improvement to add a note to the docs, and see if enter or similar can give us a better panic message.

dmosc commented 1 year ago

Got the same issue while migrating from reqwest::blocking::Client to reqwest::Client. I was building the asynchronous method alongside the synchronous implementation, thus importing and implementing both clients in the scope at the same time.

Removing either one of the clients fixed it for me.

daMichl commented 12 months ago

This also happened to me today. only found the error with the help of trusty RUST_BACKTRACE. Issue came from another library crate of mine where i used the blocking version of reqwest.

without backtrace it just shows this atm:


Cannot drop a runtime in a context where blocking is not allowed. This happens when a runtime is dropped from within an asynchronous context.
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace```
PeterGrace commented 4 months ago

I just ran into this when trying to leverage the prometheus rust crate's push-metrics support. push-metrics loads in reqwest in blocking mode as a dependency. Was surprisingly difficult to track this down.