-
Notifications
You must be signed in to change notification settings - Fork 86
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
Add support for serving static files on background thread #177
Conversation
This also adds more checks for the paths specified by the user.
#' @include httpuv.R | ||
|
||
#' @importFrom R6 R6Class | ||
Server <- R6Class("Server", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs documentation
int ret = pFDS->initialize(Rcpp::as<std::string>(response["bodyFile"]), | ||
Rcpp::as<bool>(response["bodyFileOwned"])); | ||
if (ret != 0) { | ||
REprintf(pFDS->lastErrorMessage().c_str()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it correct that there's no return or anything here? We just plow forward with this pFDS?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the existing behavior was to do that. IIRC, it resulted in an error being thrown in getData()
, and this message: Error reading: <filename>
. If we fix this, I suggest doing it in a separate PR.
src/webapplication.cpp
Outdated
// Couldn't read the file | ||
delete pDataSource; | ||
|
||
if (sp.options.fallthrough.get()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure we should fall through here. If a file/directory is missing it's one thing, but if it's present but we fail when reading it, that feels like a true error (and maybe 500 instead of 404).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR adds support for serving files on the background thread that handles I/O.
This minimal example will serve static files out of a subdir of the current directory named
content
. If there's an index.html file, it will be served if the request is for/
(the same is true for subdirectories). If the file isn't found, a minimal 404 page will be served, without hitting the main R thread.In the next example, it will try to serve requests that have
/assets
in the URL path from the localassets
subdirectory. If the file isn't found, the request will "fall through" -- that is, it will be forwarded to the Rcall
function, like requests normally are with httpuv.