rstudio / shiny-examples

Other
1.97k stars 3.77k forks source link

Remove s from `https` for google logo image #109

Closed schloerke closed 6 years ago

schloerke commented 6 years ago

RSP has issues with downloading two HTTPS images from the same url one after another. However, this app doesn't need https to prove it is working, so they are removed and the RSP issue goes away.

For RSP, could instead use method = "wget", as "libcurl" or "curl" do not work.

Error received if the download.file is upgraded to a httr command

Error in curl::curl_fetch_disk: A PKCS # 11 module returned CKR_DEVICE_ERROR, indicating that a problem has occurred with the token or slot.

jcheng5 commented 6 years ago

Could there be more to this than that? I just tried two download.file from an .R script on RSP and it seems to work fine. Could it be a regular download.file followed by a future(download.file)? Does plan(multiprocess) -> plan(multisession) fix it too?

schloerke commented 6 years ago

@jcheng5

It is weird. When I've restarted R, everything works fine. I agree that it is due to a regular download.file followed by a future download file. It does work when using


Fails; multiprocess; download.file

library(promises); library(future)
plan(multiprocess)

get_file <- function(type) {
  url <- "https://www.google.com/images/logo.gif"
  path <- tempfile(fileext = ".gif")
  switch(
    type,
    "regular" = download.file(url, path, mode = "wb"),
    "httr" = httr::GET(url, httr::write_disk(path, overwrite = TRUE))
  )
  path
}

type <- "regular"
f0 %<-% future({
  get_file(type)
})
value(f0)
#> [1] "/tmp/RtmpeOEuVH/file5f22bf1452e.gif"
f1 %<-% future({
  get_file(type)
})
value(f1)
#> [1] "/tmp/RtmpeOEuVH/file5f42bf1452e.gif"

get_file(type)
#> [1] "/tmp/RtmpeOEuVH/file5dc2bf1452e.gif"
f2 %<-% future({
  get_file(type)
})
value(f2)
#> Error in download.file(url, path, mode = "wb"): cannot open URL 'https://www.google.com/images/logo.gif'

Created on 2018-10-02 by the reprex package (v0.2.1)


Fails; multiprocess; httr

library(promises); library(future)
plan(multiprocess)

get_file <- function(type) {
  url <- "https://www.google.com/images/logo.gif"
  path <- tempfile(fileext = ".gif")
  switch(
    type,
    "regular" = download.file(url, path, mode = "wb"),
    "httr" = httr::GET(url, httr::write_disk(path, overwrite = TRUE))
  )
  path
}

type <- "httr"
f0 %<-% future({
  get_file(type)
})
value(f0)
#> [1] "/tmp/RtmpktjELL/file62b4cd19405.gif"
f1 %<-% future({
  get_file(type)
})
value(f1)
#> [1] "/tmp/RtmpktjELL/file62d4cd19405.gif"

get_file(type)
#> [1] "/tmp/RtmpktjELL/file6154cd19405.gif"
f2 %<-% future({
  get_file(type)
})
value(f2)
#> Error in curl::curl_fetch_disk(url, x$path, handle = handle): Bulk data encryption algorithm failed in selected cipher suite.

Created on 2018-10-02 by the reprex package (v0.2.1)


Works; multisession; download.file

library(promises); library(future)
plan(multisession)

get_file <- function(type) {
  url <- "https://www.google.com/images/logo.gif"
  path <- tempfile(fileext = ".gif")
  switch(
    type,
    "regular" = download.file(url, path, mode = "wb"),
    "httr" = httr::GET(url, httr::write_disk(path, overwrite = TRUE))
  )
  path
}

type <- "regular"
f0 %<-% future({
  get_file(type)
})
value(f0)
#> [1] "/tmp/RtmpMwk5xQ/file6ce775d6c.gif"
f1 %<-% future({
  get_file(type)
})
value(f1)
#> [1] "/tmp/RtmpMwk5xQ/file6ce25df6f3d.gif"

get_file(type)
#> [1] "/tmp/RtmpQ3T7ff/file6b7ec3984e.gif"
f2 %<-% future({
  get_file(type)
})
value(f2)
#> [1] "/tmp/RtmpMwk5xQ/file6ce14ce631f.gif"

Created on 2018-10-02 by the reprex package (v0.2.1)


Works; multiprocess; download.file; http

library(promises); library(future)
plan(multiprocess)

get_file <- function(type) {
  url <- "http://www.google.com/images/logo.gif"
  path <- tempfile(fileext = ".gif")
  switch(
    type,
    "regular" = download.file(url, path, mode = "wb"),
    "httr" = httr::GET(url, httr::write_disk(path, overwrite = TRUE))
  )
  path
}

type <- "regular"
f0 %<-% future({
  get_file(type)
})
value(f0)
#> [1] "/tmp/RtmppZkE9E/file71a50ab37a3.gif"
f1 %<-% future({
  get_file(type)
})
value(f1)
#> [1] "/tmp/RtmppZkE9E/file71c50ab37a3.gif"

get_file(type)
#> [1] "/tmp/RtmppZkE9E/file70450ab37a3.gif"
f2 %<-% future({
  get_file(type)
})
value(f2)
#> [1] "/tmp/RtmppZkE9E/file71f1513a43.gif"

Created on 2018-10-02 by the reprex package (v0.2.1)

jcheng5 commented 6 years ago

OK, the problem here is fork without exec (which is how plan(multicore) a.k.a. plan(multiprocess) on Linux is implemented), which is fundamentally a bit dangerous. It looks like NSS detects when it's in this situation and purposefully errors. See https://bugzilla.redhat.com/show_bug.cgi?id=1317691

I'd prefer a change to plan(multisession) instead of changing to http, I think that's what people should be doing if they're doing async downloading just to be safe.

jcheng5 commented 6 years ago

FYI when I wrote the documentation for promises I included a section on how to choose a future plan. In retrospect maybe the warnings around fork-without-exec are not strong enough.