Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tests relying on reqwest::Client may fail with "dispatch task is gone: runtime dropped the dispatch task" #3852

Open
2 tasks done
rami3l opened this issue Jun 2, 2024 · 3 comments
Labels
Milestone

Comments

@rami3l
Copy link
Member

rami3l commented Jun 2, 2024

Verification

Problem

This is causing irregular failures for different CI platforms:

The error message here is identical (although the exact test that failed might differ):

running 395 tests
...
test suite::cli_exact::check_updates_with_update ... FAILED
error: could not download file from 'https://static.rust-lang.org/rustup/release-stable.toml' to '/tmp/rustup-updateD58GcC/release-stable.toml': failed to make network request: error sending request for url (https://static.rust-lang.org/rustup/release-stable.toml): client error (SendRequest): dispatch task is gone: runtime dropped the dispatch task

Notes

Possibly related to hyperium/hyper#2312, Azure/azure-sdk-for-rust#1550 or fussybeaver/bollard#387 (comment).

I'm pretty sure that #3367 has caused this.

@rami3l rami3l added the bug label Jun 2, 2024
@rami3l rami3l added this to the 1.28.0 milestone Jun 2, 2024
@rami3l rami3l changed the title suite::cli_exact::check_updates_with_update failed with "dispatch task is gone: runtime dropped the dispatch task" Tests relying on reqwest::Client may fail with "dispatch task is gone: runtime dropped the dispatch task" Jun 4, 2024
@rami3l
Copy link
Member Author

rami3l commented Jun 4, 2024

According to the error message format and my IDE searching the current codebase, this is a RustupError::DownloadingFile. This kind error message can only be emitted from here:

rustup/src/utils/utils.rs

Lines 154 to 190 in 6d43761

pub(crate) async fn download_file_with_resume(
url: &Url,
path: &Path,
hasher: Option<&mut Sha256>,
resume_from_partial: bool,
notify_handler: &dyn Fn(Notification<'_>),
) -> Result<()> {
use download::DownloadError as DEK;
match download_file_(url, path, hasher, resume_from_partial, notify_handler).await {
Ok(_) => Ok(()),
Err(e) => {
if e.downcast_ref::<std::io::Error>().is_some() {
return Err(e);
}
let is_client_error = match e.downcast_ref::<DEK>() {
// Specifically treat the bad partial range error as not our
// fault in case it was something odd which happened.
Some(DEK::HttpStatus(416)) => false,
Some(DEK::HttpStatus(400..=499)) | Some(DEK::FileNotFound) => true,
_ => false,
};
Err(e).with_context(|| {
if is_client_error {
RustupError::DownloadNotExists {
url: url.clone(),
path: path.to_path_buf(),
}
} else {
RustupError::DownloadingFile {
url: url.clone(),
path: path.to_path_buf(),
}
}
})
}
}
}

@rami3l rami3l modified the milestones: 1.28.0, On Deck Jun 12, 2024
@twuebi
Copy link

twuebi commented Nov 23, 2024

Hi @rami3l, I also encountered the same issue with tests, searching for the cause, I came across this thread on the rust-lang forums. We started encountering the problem over at lakekeeper (lakekeeper/lakekeeper#548) after making our clients static with lazy-locks like you are doing here, I think that is the cause for the failures you've been observing.

@rami3l
Copy link
Member Author

rami3l commented Nov 25, 2024

Hi @rami3l, I also encountered the same issue with tests, searching for the cause, I came across this thread on the rust-lang forums. We started encountering the problem over at lakekeeper (lakekeeper/lakekeeper#548) after making our clients static with lazy-locks like you are doing here, I think that is the cause for the failures you've been observing.

@twuebi Thanks a lot for the info! I think your observation is likely relevant as we currently use in-process testing for some commands even if it appears to be out-of-process (#3891).

It could indeed be possible that something like 2261e26 is required for reqwest::Client as well.

Anyway, I'll find some time to look into this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants