vcr icon indicating copy to clipboard operation
vcr copied to clipboard

Support {httr2}

Open dpprdan opened this issue 4 years ago • 18 comments

Now that {httr2} is on CRAN and supports mocking, could {vcr} support it as well?

dpprdan avatar Sep 27 '21 17:09 dpprdan

That's the idea! To have support for it we need support for httr2 in webmockr as well as here. Any extra time to contribute to these changes?

sckott avatar Sep 28 '21 19:09 sckott

Unfortunately I don't. I am not familiar with the webmockr/vcr codebase, so I'd need a lot of extra time too, I'm afraid.

dpprdan avatar Oct 01 '21 14:10 dpprdan

Okay, no problem. Will try to get to it soon unless someone else has time

sckott avatar Oct 02 '21 17:10 sckott

@sckott just wondering if you had any time to look into this? No worries if not, of course! (I would not know how :grimacing:)

maelle avatar Nov 09 '21 11:11 maelle

not much yet unfortunately. i started a while back on webmockr https://github.com/ropensci/webmockr/compare/master...httr2 but nothing useable yet.

sckott avatar Nov 09 '21 18:11 sckott

Almost there, left to do:

  • [x] The request body using a cassette is always character instaed of raw. Should this be fixed?
vcr::use_cassette("stuff", {
    res_vcr1 <- request("https://google.com") %>% req_perform()
})
vcr::use_cassette("stuff", {
    res_vcr2 <- request("https://google.com") %>% req_perform()
})

res_vcr1
#> <httr2_response>
#> GET https://google.com/
#> Status: 200 OK
#> Body: In memory (7070 bytes)

res_vcr2
#> <httr2_response>
#> GET https://google.com/
#> Status: 200 OK
#> Body: In memory (1 bytes)
  • [x] A warning arises when using httr2 with vcr, via use of httr2::local_mock in wemockr::httr2_mock. I'm not sure if this is a problem or not
Setting global deferred event(s).
i These will be run:
  * Automatically, when the R session ends.
  * On demand, if you call `withr::deferred_run()`.
i Use `withr::deferred_clear()` to clear them without executing.

sckott avatar Oct 28 '23 17:10 sckott

this is great! is there a public package where you're using httr2+vcr?

maelle avatar Oct 30 '23 16:10 maelle

is there a public package where you're using httr2+vcr?

No, not yet. I shjould make a silly small pkg just to demonstrate though.

@maelle do you have a sense about that warning above from withr? does that raise a red flag for you? I'm not sure how to suppress it, but i'm just not very familiar with withr

sckott avatar Oct 30 '23 16:10 sckott

do you get that warning when running the tests or else?

I actually blogged about deferred_run() recently which made me realize I had ignored that message forever no matter where I saw it :joy: https://masalmon.eu/2023/10/16/test-local-mess-reset/

maelle avatar Oct 30 '23 16:10 maelle

When you run use_cassette() that warning is thrown. It comes from here which uses httr2::local_mock, which uses withr::local_options, which uses withr::defer. That's the "hook" mechanism hadley built into httr2. I'll have a look at your blog post

sckott avatar Oct 30 '23 16:10 sckott

wrt that webmockr code, I cringe about having to change stuff in the global env, but that's the only way it works with the hook mechanism hadley used, 🤷🏽 🤷🏽 🤷🏽

sckott avatar Oct 30 '23 16:10 sckott

  • [ ] ping folks when httr2 integration is on cran:
    • https://github.com/ropenscilabs/allcontributors/issues/32
    • ...

sckott avatar Nov 03 '23 16:11 sckott

Would looking at https://github.com/nealrichardson/httptest2/tree/main/R help? I'm not familiar with how it works under the hood.

maelle avatar Nov 10 '23 10:11 maelle

(There was an issue I thought was related to the httr2 integration here, but was a redirect issue that's not germaine to httr2, see #267)

sckott avatar Jan 16 '24 23:01 sckott

In testing this I don't see the withr warnings when using vcr in another package. So i'm thinking it's all good, but will keep an eye out

sckott avatar Jan 17 '24 00:01 sckott

The test package for testing the httr2/vcr integration is up on my account https://github.com/sckott/rrr

sckott avatar Jan 17 '24 00:01 sckott

wowsa - this thing about httr2 turning HTTP error codes into R errors is really painful - i'm surprised that decision was made - definitely a pkg made for end users and not developers

sckott avatar Jan 17 '24 18:01 sckott

A few things i'm wokring on now that will hopefully be the last things:

  • [ ] Not capturing response body and reponse headers for some reason on requests that have cassettes already, probably something easy, just hard to find where it is
  • [ ] How to handle httr2's error behavior - that http errors are turned into R errors automatically. Considerations:
    • right now I'm adding httr2::req_error(is_error = function(resp) FALSE) to the intercepted real request and then running req_perform.
    • If we don't change anything about httr2 behavior we'd then have to use last_response to get the response after a 400/500 series response, which would probably work, will try that
    • On requests with an existing cassette for requests that have a 400/500 series status code you don't get the same behavior of http error -> R error because the request/response is mocked, it's not actuallly real. Could deal with this by seeing if we can trigger that http error -> R error behavior manually. Or leave this behavior as is and let people know?
    • I'd prefer httr2 didn't have this behavior at all, but given it's there, it's more important to match the behavior of the pkg as close as possible

sckott avatar Feb 05 '24 18:02 sckott