curl support

[nbenn]

I'm interested in using webmockr for a test suite of mine, but I'm currently using jeroen/curl for making http requests.

Are there plans on supporting curl or is this not happening anytime soon?

[sckott]

thanks for the nudge @nbenn

Integrating with a http client library (e.g., curl) does require tweaking the curl package a tiny bit to have a hook for webmockr. I'll see if he's interested in doing it.

what functions do you use in curl that you'd want to mock?

[nbenn]

Thanks for the quick response.

The package I'm hoping to use webmockr with uses both easy handles (curl::curl_fetch_memory()) as well as the multi interface (curl::curl_fetch_multi()/curl::multi_run()).

[sckott]

thanks for the clarification @nbenn - will update on any progress

[nbenn]

Let me know if there is anything I can do to help. I'm happy to.

[sckott]

@nbenn okay, try after installing like remotes::install_github("ropensci/webmockr@adapter-curl"), which will also install a fork of curl pkg from my gh account

see examples here https://github.com/ropensci/webmockr/blob/adapter-curl/R/adapter-curl.R#L37-L66

it's a bit of a hack - it's not easy to inspect the curl handle so we use curl::curl_echo to make a fake request to localhost, which gives us the headers, http method, etc, which we can then pass through webmockr to mock the request, so no real HTTP request is done to the web, but we do need to do a localhost request

thx to @jeroen for help. he and I will talk more about possibly avoiding doing the curl_echo call

[nbenn]

@sckott thanks a lot for moving so quickly with this and I am sorry that it took a bit of time for me to have a look at it.

Naturally, down the line, I'm interested in getting webmockr and vcr to work with curl.

But first, in order to get the automated recording of requests working, we'd need the webmockr_net_connect_allowed section of the curl adapter to work. This seems doable, as (apart from the hacky curl::mock(FALSE)/curl::mock(TRUE), currently the problem is that req$called does not hold the necessary information to evaluate the original curl call. But this does not seem like a big problem either, as we could just modify the curl functions to return the function calls, e.g. nbenn/curl@50571d8 and evaluate this later as

curl::mock(FALSE) # tempoarily disable
resp <- eval(req$called)
curl_resp <- build_curl_response(req, resp)
curl::mock(TRUE) # re-enable

Or am I missing something here? Are you interested in PRs?

[sckott]

yeah, PRs welcome. makes total sense you'd need net connect allowed working. does nbenn/curl@50571d8 make net connect allowed work?

does the use of curl_echo bother you at all? or are you okay with that? It sounds like Jeroen may be able to sort out a curl handle inspection thing some day, but it wouldn't happen any time soon.

[sckott]

fix for allow net connect for curl adapter in 850b31f

[sckott]

@nbenn can you reinstall remotes::install_github("ropensci/webmockr@adapter-curl") and try real connections again with webmockr loaded

[sckott]

@nbenn did you get a chance to try this yet?

[nbenn]

@sckott I haven't tried any webmockr related stuff, sorry. I though I'd wait until the infx review process is over, before taking this up again.

[sckott]

ok, thanks

[sckott]

changes in 3b05bc1

[sckott]

@nbenn if you have a chance, just updated the integration with curl - reinstall from remotes::install_github("ropensci/webmockr@adapter-curl")

[nbenn]

@sckott sorry for the crazy long delay on my side. I got round to testing vcr/webmockr in conjunction with curl. The short story: with the current state of things I don't get very far. I'm happy to elaborate a bit:

For me, webmockr fails when trying to evaluate the original call, because req$called holds a string rather that a proper call/expression object. My call stack at the point of failure looks something like

...
16: curl::curl_fetch_memory(as.character(eval(urls[[i]])), handle = create_hand...
17: webmockr::curl_mock_req(url, handle, called = called)
18: adap$handle_request(req)
19: eval(parse(text = paste0("curl::", strsplit(req$called, "\\\"")[[1]][1])))
20: parse(text = paste0("curl::", strsplit(req$called, "\\\"")[[1]][1]))

and req$called holds

"curl::curl_fetch_memoryas.characterevalurls[[i]], handle = create_handlebodies[[i]]"

finally, the error that is thrown is

Error in parse(text = paste0("curl::", strsplit(req$called, "\\\"")[[1]][1])) :
  <text>:1:11: unexpected '::'
1: curl::curl::

Of course, only selectively pasting "curl::" in front of req$called will not fix the real problem, as this call can no longer be evaluated.

What reason do you have for doing

called <- gsub("\\(|\\)", "", deparse(sys.call()))

You already have url and handle, which suffice to re-build the original call later on, no? Are you only after the function name? In that case, why not something like deparse(match.call()[[1L]])?

[nbenn]

Is vcr supposed to work in conjunction with the curl branch of webmockr? I can't get it to store a response.

[sckott]

Is vcr supposed to work in conjunction with the curl branch of webmockr? I can't get it to store a response.

I thought I had started work on curl integration in vcr, but I don't think I have. I'll start a branch for that. we need a https://github.com/ropensci/vcr/blob/master/R/request_handler-crul.R equivalent for curl

[sckott]

thanks for testing this @nbenn

I'm not surprised you ran into trouble as I probably only tested it in a very narrow set of cases.

First, note that I'm not positive @jeroen will integrate webmockr in curl - see the PR at jeroen/curl#174 (comment) - in a call somewhat recently we talked about a different approach than what is being used now, i'm not sure what jeroen's current thinking is on webmocrk in curl

for

called <- gsub("\(|\)", "", deparse(sys.call()))

i don't know for sure why, i think your pr is better

[sckott]

your two PR's are merged now.

[sckott]

@nbenn sorry for long delay on this, jeroen and I talked again the other day and we decided curl wont integrate with webmockr mostly b/c curl is just too important to possibly introduce edge case bugs into