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

Error: Cannot find progress bar (req_perform_parallel) #594

Open
JBGruber opened this issue Dec 11, 2024 · 13 comments · May be fixed by #602
Open

Error: Cannot find progress bar (req_perform_parallel) #594

JBGruber opened this issue Dec 11, 2024 · 13 comments · May be fixed by #602
Labels
bug an unexpected problem or unintended behavior
Milestone

Comments

@JBGruber
Copy link

JBGruber commented Dec 11, 2024

When I run a req_perform_parallel with a progress bar and abort the operation, every new call to req_perform_parallel will error until the R session is restarted. Here is a minimal example:

library(httr2)
reqs <- list(
  request(example_url()) |>
    req_url_path("/delay") |> 
    req_url_path_append(sample(1:10, 1))
) |>
  rep(100)

resps <- req_perform_parallel(reqs, progress = TRUE)

### stop with Esc ##
resps <- req_perform_parallel(reqs, progress = TRUE)
#> Error in cli::cli_progress_update(..., id = id) : 
#>   Cannot find progress bar `cli-25573-7`
#> Error in cli::cli_progress_update(..., id = id) : 
#>   Cannot find progress bar `cli-25573-7`

This does not apply to req_perform_sequential.

@hadley
Copy link
Member

hadley commented Dec 11, 2024

Hmmm create_progress_bar() should probably add an exit handler to the calling environment that automatically closes the progress bar. But I'm a bit surprised that's necessary because I don't understand why it would try and reuse the previous progress bar.

@JBGruber
Copy link
Author

I also have no idea. And here is some more weirdness. I just noticed that it does not happen every time! If I hit Esc too fast, it does not seem to reuse the id???

Screencast_20241211_183723.webm

@hadley
Copy link
Member

hadley commented Dec 11, 2024

@gaborcsardi any idea what's going on here? The progress bar is created in a helper function:

httr2/R/utils.R

Lines 242 to 284 in e972770

create_progress_bar <- function(total,
name,
config,
env = caller_env(),
config_arg = caller_arg(config),
error_call = caller_env()) {
if (is_false(config)) {
return(list(
update = function(...) {},
done = function() {}
))
}
if (is.null(config) || is_bool(config)) {
args <- list()
} else if (is_scalar_character(config)) {
args <- list(name = config)
} else if (is.list(config)) {
args <- config
} else {
stop_input_type(
config,
what = c("a bool", "a string", "a list"),
arg = config_arg,
call = error_call
)
}
args$name <- args$name %||% name
# Can be removed if https://github.com/r-lib/cli/issues/630 is fixed
if (is.infinite(total)) {
total <- NA
}
args$total <- total
args$.envir <- env
id <- exec(cli::cli_progress_bar, !!!args)
list(
update = function(...) cli::cli_progress_update(..., id = id),
done = function() cli::cli_progress_done(id = id)
)
}

Maybe something is going wrong with how I set .envir?

@gaborcsardi
Copy link
Member

I am not sure, but my guess is that the progress bar's env is already removed, but the curl multi-handle is still alive and you get a final update() call for it.

@hadley
Copy link
Member

hadley commented Dec 11, 2024

Ooooh I bet that's it.

@hadley
Copy link
Member

hadley commented Dec 11, 2024

I think we can fix this by using a separate pool for each req_perform_parallel() call, rather than relying on the default global pool.

@gaborcsardi
Copy link
Member

I think you can also tryCatch() and ignore the error.

@hadley hadley added the bug an unexpected problem or unintended behavior label Dec 19, 2024
@hadley hadley added this to the v1.1.0 milestone Dec 19, 2024
@hadley
Copy link
Member

hadley commented Dec 20, 2024

I can't reliably reproduce this. I tried what I thought would illustrate it more reliably:

library(httr2)
options(cli.progress_show_after = 0)

reqs <- list(
  request(example_url()) |> req_url_path("/delay/10"),
  request(example_url()) |> req_url_path("/delay/1"),
  request(example_url()) |> req_url_path("/delay/10"),
  request(example_url()) |> req_url_path("/delay/1"),
  request(example_url()) |> req_url_path("/delay/10"),
  request(example_url()) |> req_url_path("/delay/1")
) 
resps <- req_perform_parallel(reqs, progress = TRUE)

But I only manage to see the error once, so I suspect it might be more of a problem on windows.

@JBGruber
Copy link
Author

JBGruber commented Dec 20, 2024

I'm on Arch Linux (BTW)

It only seems to happen when I interrupt the process after a couple of seconds and not every time. Which makes this the weirdest behaviour that I've experienced so far in R.

Can you start another process after you see the error once? I need to restart R.

@hadley
Copy link
Member

hadley commented Dec 20, 2024

But my experiments suggest that there might be a bigger problem — I think terminating req_perform_parallel() doesn't actually terminate the other ongoing requests.

...

Hmmmm, some experiments suggest that maybe that's not the case?

hadley added a commit that referenced this issue Dec 20, 2024
When `req_perform_parallel()` is terminated, it seems like it's possible for a curl progress call to complete after the progress bar has been terminated.

Fixes #594
@hadley hadley linked a pull request Dec 20, 2024 that will close this issue
@hadley
Copy link
Member

hadley commented Dec 20, 2024

@JBGruber could you please try #602 and see if it fixes the problem for you?

@JBGruber
Copy link
Author

This fixes the error 😊. But I observed some other unexpected behaviour.

  1. After interrupting, I get ! Operation timed out after 10000 milliseconds with 0 bytes received on the next attempt when that shouldn't be the case. I decreased the delay to 2 seconds on the second attempt to make sure this isn't a coincidence. If the delay of first + second attempt is below 10 seconds, it does not seem to happen.
  2. If I wait until 50% requests are done and then interrupt, the return object is still produced, with half of responses missing (if I wait until 80%, 20% are empty and so on)

Sorry for the screen recording, but I don't know how else to show it. Until second 46, you see the first issue. I wait a little bit and then show issue 2 from secon 56.

httr2-pr-behaviour.webm

I think this is all far less problematic than having to restart R to be able to run req_perform_parallel() with a progress bar. But I think it suggests that you might have been right and terminating req_perform_parallel() doesn't actually terminate the other ongoing requests.

@hadley
Copy link
Member

hadley commented Dec 21, 2024

@JBGruber I did a bunch of experiments and convinced myself that they're getting cancelled on my machine, but maybe that code path doesn't work correctly on linux? I'll think about how to test that hypothesis.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug an unexpected problem or unintended behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants