rstudio / httpuv

HTTP and WebSocket server package for R
Other
228 stars 85 forks source link

CRAN release broke curl #123

Closed jeroen closed 6 years ago

jeroen commented 6 years ago

The curl package has a function curl_echo() that was broken with the new httpuv release.

library(curl)
example(curl_echo)
 Error in curl_fetch_memory(paste0("http://localhost:", port, "/"), handle = handle) : 
  Callback aborted 

I think the problem is that previously httpuv::service() always returned TRUE in case of success. However as if the new version it randomly returns FALSE and TRUE.

Is this changed expected?

wch commented 6 years ago

Currently, service() calls later::run_now(), which, according to the docs returns:

A logical indicating whether any callbacks were actually run.

In the past, it basically called uv_run() (from libuv). If you called service() without setting a timeout value, or with a non-NA value, it called uv_run() with UV_RUN_ONCE: https://github.com/rstudio/httpuv/blob/v1.3.6.2/src/httpuv.cpp#L405

The return values uv_run() with UV_RUN_ONCE: http://docs.libuv.org/en/v1.x/loop.html#c.uv_run

UV_RUN_ONCE: Poll for i/o once. Note that this function blocks if there are no pending callbacks. Returns zero when done (no active handles or requests left), or non-zero if more callbacks are expected (meaning you should run the event loop again sometime in the future).

So if I'm understanding it correctly, in the past, it should have returned TRUE only if there were more callbacks for libuv to process. I'll experiment with it some more though.

wch commented 6 years ago

FWIW, in httpuv 1.3.6.2, in a clean R session, this is what I get:

> print(httpuv::service())
[1] TRUE
> print(httpuv::service())
[1] FALSE
> print(httpuv::service())
[1] FALSE

@jeroen I think you were relying on some incidental behavior of service() that happened to work for your case. We could have it always return TRUE. Would that fix your problem, or does curl expect service() to return FALSE in some cases?

jeroen commented 6 years ago

I think my assumption was that httpuv::service() would return TRUE on success, and FALSE on failure, for example if there is nothing to service.

wch commented 6 years ago

We currently use later::run_now(), which returns TRUE or FALSE depending on whether any callbacks are run -- but these callbacks could be from anywhere, not just httpuv.

If we make service() always return TRUE, will that work for you?

jeroen commented 6 years ago

Yes I guess that would work. I don't fully recall what edge case I was trying to cover.

wch commented 6 years ago

OK, done.

jeroen commented 6 years ago

Thanks! So there is no easy way to make it return FALSE when there is no server running? For example when httpuv::startServer() was never called or it crashed?

wch commented 6 years ago

One complication is that you now can have multiple servers running at the same time. So if the check is, "is a server running?", and the answer is yes, you can't be sure that it's your server or someone else's.

startServer has always returned a handle to the server though. So can you do something along these lines?

handle <- NULL
try( handle <- startServer(...) )

if (!is.null(handle)) {
  # Do something
}

Note that if startServer() fails (for example, if the port is already occupied), it'll throw an error.

All that said, it might make sense to add a function that returns a list of running servers.

@jeroen I'll also forward you an email I wrote about the changes to httpuv.

jeroen commented 6 years ago

OK thanks. In my code I run only one server. I think the main case I was trying to cover is checking if the server is still alive. But I suppose that never worked the way I thought it did.