rstudio / pins-r

Pin, Discover and Share Resources
https://pins.rstudio.com
Other
301 stars 62 forks source link

Improved error reporting for redirected mistyped pins in board_connect_url #810

Closed slodge closed 7 months ago

slodge commented 7 months ago

Disclaimer: I'm not sure whether I should log this with Connect rather than with pins-r

If I have a pin with a vanity url, but request that vanity url without a trailing slash, then I get a confusing error message about "Pin does not declare file type" - from https://github.com/rstudio/pins-r/blob/a3e52c78e799d24359b72dffd4cac33a7bc0fe98/R/pin-read-write.R#L230

It might be helpful if this reported a message like "incorrect url - have you tried adding a trailing slash" instead - or perhaps it could even return the data?

Reproduced using:

library(pins)
board <- board_connect_url(
  list(
    mypin_works = "https://my.connect.com/test/data_cs/",
    mypin_not_works = "https://my.connect.com/test/data_cs"
  ),
  headers = connect_auth_headers(key = Sys.getenv("MY_KEY")))
)
data1 <- pin_read(board, "mypin_works")
data2 <- pin_read(board, "mypin_not_works")

If I look at the http transfer... I see that the "mypin_works" requests GET /test/data_cs/data.txt while "mypin_not_works" requests GET /test/data_cs which gets redirected to /test/data_cs/ which returns the Connect HTML page

httr::with_verbose({
  pin_read(board, "mypin_not_works")
}, data_in=TRUE, ssl=TRUE)

image

juliasilge commented 7 months ago

Ah, that's so unfortunate that this is the situation! Thanks for the report.

The board_url() infrastructure is very flexible and also allows for this kind of pin:

board <- board_url(c(
  letters = "https://raw.githubusercontent.com/rstudio/pins-r/master/tests/testthat/pin-files/first.txt"
))

This means we can't unfortunately remove a trailing slash because we can't know what the user is intending. However, we can make that error message better. 👍

github-actions[bot] commented 7 months ago

This issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with a reprex: https://reprex.tidyverse.org) and link to this issue.