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

failure on Windows #158

Open
maelle opened this issue Feb 26, 2020 · 29 comments
Open

failure on Windows #158

maelle opened this issue Feb 26, 2020 · 29 comments

Comments

@maelle
Copy link
Member

maelle commented Feb 26, 2020

cf https://github.com/ropensci/ropenaq/runs/469919162?check_suite_focus=true

I'm guessing it's an encoding issue 😉 😭

@sckott
Copy link
Collaborator

sckott commented Feb 26, 2020

did you try setting preserve_exact_body_bytes = TRUE yet? it may be that yaml pkg is not handling the characters well, but might be fine if its a base64 encoded string

@maelle
Copy link
Member Author

maelle commented Feb 26, 2020

Thanks! I get new failures now https://github.com/ropensci/ropenaq/runs/470099508

@sckott
Copy link
Collaborator

sckott commented Feb 26, 2020

it looks like you have the same cassettes though - i should have said use preserve_exact_body_bytes and re-record cassettes

@maelle
Copy link
Member Author

maelle commented Feb 26, 2020

I thought I had ropensci-archive/ropenaq@3be1989 but I'll retry

@maelle
Copy link
Member Author

maelle commented Feb 26, 2020

This is my config

library("vcr")
invisible(vcr::vcr_configure(
  dir = "../fixtures",
  preserve_exact_body_bytes = TRUE
))

Re-recording doesn't seem to change much ropensci-archive/ropenaq@d68e840

@sckott
Copy link
Collaborator

sckott commented Feb 26, 2020

what version of vcr do you have?

@maelle
Copy link
Member Author

maelle commented Feb 27, 2020

0.4.0, I'll upgrade to the dev version!

@maelle
Copy link
Member Author

maelle commented Feb 27, 2020

Ok, I updated webmockr and vcr using their dev version (and set the Remotes field of DESCRIPTION to point to them), recorded the cassettes now I get only one failure in one test. The problem is no longer encoding in the body, but in the URI. 😬

https://github.com/ropensci/ropenaq/runs/471886535?check_suite_focus=true

An HTTP request has been made that vcr does not know how to handle:
GET https://api.openaq.org/v1/measurements?country=DE&city=J%C3%AF%C2%BF%C2%BDrgen%20Friesel&limit=1&page=1
vcr is currently using the following cassette:
  - ../fixtures/buildQueries_accents.yml
    - record_mode: once
    - match_requests_on: method, uri

It's a test called "Queries work with spaces and accents". Cassette; test.

@sckott
Copy link
Collaborator

sckott commented Feb 27, 2020

thanks, I'll have a look

@sckott
Copy link
Collaborator

sckott commented Feb 28, 2020

works fine locally. ugh, i guess an encoding issue on windows only. imagine it's in the code for comparing requests.

@sckott
Copy link
Collaborator

sckott commented Feb 28, 2020

I was able to replicate the issue on a windows VM - I can't figure out what the non-escaped query string should look like for the failing test. I get

curl::curl_unescape("J%EF%BF%BDrgen+Friesel")
#> [1] "J�rgen+Friesel"

to clarify (and still on windows): I could only replicate the error when using your original casette file in your github repo. Hoewever, if I deleted the cassette file, then ran tests again, it did not fail. So perhaps the cassette is recorded on a different operating system than where the error is happening, and somehow the encoding is handled differently

@maelle
Copy link
Member Author

maelle commented Feb 28, 2020

So perhaps the cassette is recorded on a different operating system than where the error is happening, and somehow the encoding is handled differently

So how should I proceed to try and help debug this next week?

@sckott
Copy link
Collaborator

sckott commented Feb 28, 2020

we do a pretty simple comparison of the URIs here https://github.com/ropensci/vcr/blob/master/R/request_matcher_registry.R#L48 - i guess we need to figure out a way to make sure the two strings are not messed up on windows, not sure yet

@maelle
Copy link
Member Author

maelle commented Mar 21, 2020

maybe the two URIs should be compared after being URLencode()ed or so?

@maelle
Copy link
Member Author

maelle commented Mar 23, 2020

maelle added a commit to ropensci-archive/ropenaq that referenced this issue Mar 23, 2020
@sckott
Copy link
Collaborator

sckott commented Mar 23, 2020

thanks, okay, but we can't test r-devel-linux-x86_64-debian-clang on rhub right now though right? rub bug or something

@maelle
Copy link
Member Author

maelle commented Mar 23, 2020

Right. That said any R-hub Windows platform should reproduce the bug.

@sckott
Copy link
Collaborator

sckott commented Sep 5, 2020

Sorry for the delay on this @maelle - I assume this is still a problem, yes? If so, what branch should I use to test

@maelle
Copy link
Member Author

maelle commented Sep 5, 2020

I think so but haven't looked into it for a long time (thankfully you know I'll soon spend more time thinking about http testing!)

The test was https://github.com/ropensci/ropenaq/blob/ecd9d8635f761811edc99968609c3ff3ae62b21f/tests/testthat/test-buildQueries.R#L83

I see another test is failing at the moment, unrelated to vcr, sorry about that.

@maelle

This comment has been minimized.

@maelle
Copy link
Member Author

maelle commented Nov 27, 2020

I tried adding URLencode() at https://github.com/ropensci/vcr/blob/master/R/request_matcher_registry.R?rgh-link-date=2020-02-28T17%3A56%3A12Z#L48, it didn't work.

It'd be very nice to fix this. 🤔

@maelle
Copy link
Member Author

maelle commented Nov 27, 2020

Could it work to encode the uri at the time of serializing it to YAML?

@maelle
Copy link
Member Author

maelle commented Nov 27, 2020

With JSON the failure is different. https://github.com/maelle/vcrencoding/runs/1464200742?check_suite_focus=true

@sckott
Copy link
Collaborator

sckott commented Mar 11, 2021

Did this get sorted yet?

@maelle
Copy link
Member Author

maelle commented Mar 12, 2021

I don't think so, do you need write access to https://github.com/maelle/vcrencoding?

@yogat3ch
Copy link

yogat3ch commented Jun 10, 2021

Hi @sckott & @maelle,
It looks like I'm also encountering this issue on a Windows 10 machine - it appears to be that URIs initially sent as un-encoded URIs are URL encoded in the yml files. When a test is run again, vcr is not matching the un-encoded request URI with the URIs in the yml files that are encoded. It seems like this can be fixed by simply adding utils::URLencode at the right location.
Do we need another reprex of this behavior or does @maelle's reprex work?

@sckott
Copy link
Collaborator

sckott commented Jun 10, 2021

Thanks for chiming in here @yogat3ch

A reprex would be great

@yogat3ch
Copy link

yogat3ch commented Jun 11, 2021

@sckott Ok, finally figured out how to make a reprex for this behavior. It's a tricky one:

.url <- httr::parse_url("https://httpbin.org/get")
`%>%` <- dplyr::`%>%`

.url$query <- lubridate::ymd_hms("2021-06-11 14:28:13") %>% 
  list(start = ., end = . - 1) %>% 
  purrr::map(~stringr::str_replace(format(.x, "%Y-%m-%dT%H:%M:%S%z"), "[\\+\\-](\\d{2})(\\d{2})$", "-\\1:\\2")) %>% 
  append(list(limit = 10000, timeframe = "1Min"))
.t <- .url$query$start
.url <- httr::build_url(.url)
vcr::vcr_configure(write_disk_path = ".", dir = ".")
vcr::use_cassette("test1", {
  (resp <- httr::GET(utils::URLdecode(.url)))
  testthat::test_that("blah", {
    testthat::expect_true(httr::content(resp)$args$start == .t)
  })  
})

It definitely has to do with the URL encoding because if utils::URLdecode is removed it works fine.

Thank you for looking into this!

@sckott
Copy link
Collaborator

sckott commented Jun 11, 2021

thanks @yogat3ch will have another look at this

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

No branches or pull requests

3 participants