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

Add support for serving static files on background thread #177

Merged
merged 60 commits into from
Dec 3, 2018
Merged

Conversation

wch
Copy link
Collaborator

@wch wch commented Nov 28, 2018

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.

s <- startServer("127.0.0.1", 5000,
  list(
    staticPaths = list("/" = "content")
  )
)

In the next example, it will try to serve requests that have /assets in the URL path from the local assets subdirectory. If the file isn't found, the request will "fall through" -- that is, it will be forwarded to the R call function, like requests normally are with httpuv.

s <- startServer("127.0.0.1", 5000,
  list(
    call = function(req) {
      return(list(
        status = 404,
        body = paste0("404 file not found: ", req$PATH_INFO)
      ))
    },
    staticPaths = list("/assets" = "assets"),
    staticPathOptions = staticPathOptions(
      indexhtml = TRUE,
      fallthrough = TRUE
    )
  )
)

wch added 30 commits October 18, 2018 14:40
This also adds more checks for the paths specified by the user.
R/server.R Show resolved Hide resolved
R/static_paths.R Outdated Show resolved Hide resolved
R/server.R Outdated Show resolved Hide resolved
R/server.R Outdated Show resolved Hide resolved
#' @include httpuv.R

#' @importFrom R6 R6Class
Server <- R6Class("Server",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs documentation

R/static_paths.R Outdated Show resolved Hide resolved
src/fs.h Outdated Show resolved Hide resolved
src/staticpath.cpp Show resolved Hide resolved
int ret = pFDS->initialize(Rcpp::as<std::string>(response["bodyFile"]),
Rcpp::as<bool>(response["bodyFileOwned"]));
if (ret != 0) {
REprintf(pFDS->lastErrorMessage().c_str());
Copy link
Member

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?

Copy link
Collaborator Author

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 Show resolved Hide resolved
// Couldn't read the file
delete pDataSource;

if (sp.options.fallthrough.get()) {
Copy link
Member

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).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented in f3e487d and 4f499a8.

src/webapplication.cpp Outdated Show resolved Hide resolved
@wch wch merged commit d868f99 into master Dec 3, 2018
@wch wch deleted the static-serving branch December 3, 2018 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants