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

Quote arguments to do.call() for nicer stack traces #1856

Merged
merged 1 commit into from Oct 31, 2017
Merged

Conversation

wch
Copy link
Collaborator

@wch wch commented Sep 26, 2017

This fixes #1851. Quoting the function to do.call() and (where possible) quote-ing the arguments makes them smaller in a stack trace.

This change doesn't cover all cases of do.call in Shiny, but it does cover the most common ones. There are many other places where the printed call is very large, but it is not easy to fix. For example, some cases are from Rcpp calls (in httpuv) to an R function, and others are of the form withXYZ( <expr> ), where <expr> is an expression. For example, see: https://gist.github.com/wch/964e18796a05f83b3aa786c0e24d0867

Below is the partial output of sys.calls() before and after the change. Before:

[[4]]
(function (appDir = getwd(), port = getOption("shiny.port"),
    launch.browser = getOption("shiny.launch.browser", interactive()),
    host = getOption("shiny.host", "127.0.0.1"), workerId = "",
    quiet = FALSE, display.mode = c("auto", "normal", "showcase"),
    test.mode = getOption("shiny.testmode", FALSE))
{
    on.exit({
        handlerManager$clear()
    }, add = TRUE)
    if (.globals$running) {
        stop("Can't call `runApp()` from within `runApp()`. If your ",
            "application code contains `runApp()`, please remove it.")
    }
    .globals$running <- TRUE
    on.exit({
        .globals$running <- FALSE
    }, add = TRUE)
    oldOptionSet <- .globals$options
    on.exit({
        .globals$options <- oldOptionSet
    }, add = TRUE)
    ops <- options(warn = max(1, getOption("warn", default = 1)),
        pool.scheduler = scheduleTask)
    on.exit(options(ops), add = TRUE)
    appParts <- as.shiny.appobj(appDir)
    appOps <- appParts$options
    findVal <- function(arg, default) {
        if (arg %in% names(appOps))
            appOps[[arg]]
        else default
    }
    if (missing(port))
        port <- findVal("port", port)
    if (missing(launch.browser))
        launch.browser <- findVal("launch.browser", launch.browser)
    if (missing(host))
        host <- findVal("host", host)
    if (missing(quiet))
        quiet <- findVal("quiet", quiet)
    if (missing(display.mode))
        display.mode <- findVal("display.mode", display.mode)
    if (missing(test.mode))
        test.mode <- findVal("test.mode", test.mode)
    if (is.null(host) || is.na(host))
        host <- "0.0.0.0"
    workerId(workerId)
    if (inShinyServer()) {
        ver <- Sys.getenv("SHINY_SERVER_VERSION")
        if (utils::compareVersion(ver, .shinyServerMinVersion) <
            0) {
            warning("Shiny Server v", .shinyServerMinVersion,
                " or later is required; please upgrade!")
        }
    }
    setShowcaseDefault(0)
    .globals$testMode <- test.mode
    if (test.mode) {
        message("Running application in test mode.")
    }
    if (is.character(appDir)) {
        desc <- file.path.ci(if (tolower(tools::file_ext(appDir)) ==
            "r")
            dirname(appDir)
        else appDir, "DESCRIPTION")
        if (file.exists(desc)) {
            con <- file(desc, encoding = checkEncoding(desc))
            on.exit(close(con), add = TRUE)
            settings <- read.dcf(con)
            if ("DisplayMode" %in% colnames(settings)) {
                mode <- settings[1, "DisplayMode"]
                if (mode == "Showcase") {
                  setShowcaseDefault(1)
                  if ("IncludeWWW" %in% colnames(settings)) {
                    .globals$IncludeWWW <- as.logical(settings[1,
                      "IncludeWWW"])
                    if (is.na(.globals$IncludeWWW)) {
                      stop("In your Description file, `IncludeWWW` ",
                        "must be set to `True` (default) or `False`")
                    }
                  }
                  else {
                    .globals$IncludeWWW <- TRUE
                  }
                }
            }
        }
    }
    if (is.null(.globals$IncludeWWW) || is.na(.globals$IncludeWWW)) {
        .globals$IncludeWWW <- TRUE
    }
    display.mode <- match.arg(display.mode)
    if (display.mode == "normal") {
        setShowcaseDefault(0)
    }
    else if (display.mode == "showcase") {
        setShowcaseDefault(1)
    }
    require(shiny)
    if (is.null(port)) {
        for (i in 1:20) {
            if (!is.null(.globals$lastPort)) {
                port <- .globals$lastPort
                .globals$lastPort <- NULL
            }
            else {
                while (TRUE) {
                  port <- p_randomInt(3000, 8000)
                  if (!port %in% c(3659, 4045, 6000, 6665:6669,
                    6697)) {
                    break
                  }
                }
            }
            tmp <- try(startServer(host, port, list()), silent = TRUE)
            if (!inherits(tmp, "try-error")) {
                stopServer(tmp)
                .globals$lastPort <- port
                break
            }
        }
    }
    on.exit({
        .globals$onStopCallbacks$invoke()
        .globals$onStopCallbacks <- Callbacks$new()
    }, add = TRUE)
    unconsumeAppOptions(appParts$appOptions)
    if (!is.null(appParts$onStop))
        on.exit(appParts$onStop(), add = TRUE)
    if (!is.null(appParts$onStart))
        appParts$onStart()
    server <- startApp(appParts, port, host, quiet)
    on.exit({
        stopServer(server)
    }, add = TRUE)
    if (!is.character(port)) {
        browseHost <- if (identical(host, "0.0.0.0"))
            "127.0.0.1"
        else host
        appUrl <- paste("http://&quot;, browseHost, ":", port, sep = "")
        if (is.function(launch.browser))
            launch.browser(appUrl)
        else if (launch.browser)
            utils::browseURL(appUrl)
    }
    else {
        appUrl <- NULL
    }
    callAppHook("onAppStart", appUrl)
    on.exit({
        callAppHook("onAppStop", appUrl)
    }, add = TRUE)
    .globals$reterror <- NULL
    .globals$retval <- NULL
    .globals$stopped <- FALSE
    ..stacktraceoff..(captureStackTraces({
        scheduleFlush()
        while (!.globals$stopped) {
            serviceApp()
            Sys.sleep(0.001)
        }
    }))
    if (isTRUE(.globals$reterror)) {
        stop(.globals$retval)
    }
    else if (.globals$retval$visible)
        .globals$retval$value
    else invisible(.globals$retval$value)
})(list(httpHandler = function (req)
{
    if (!identical(req$REQUEST_METHOD, "GET"))
        return(NULL)
    if (!isTRUE(grepl(uiPattern, req$PATH_INFO)))
        return(NULL)
    textConn <- file(open = "w+")
    on.exit(close(textConn))
    showcaseMode <- .globals$showcaseDefault
    if (.globals$showcaseOverride) {
        mode <- showcaseModeOfReq(req)
        if (!is.null(mode))
            showcaseMode <- mode
    }
    testMode <- .globals$testMode %OR% FALSE
    bookmarkStore <- getShinyOption("bookmarkStore", default = "disable")
    if (bookmarkStore == "disable") {
        restoreContext <- RestoreContext$new()
    }
    else {
        restoreContext <- RestoreContext$new(req$QUERY_STRING)
    }
    withRestoreContext(restoreContext, {
        uiValue <- NULL
        if (is.function(ui)) {
            if (length(formals(ui)) > 0) {
                uiValue <- ..stacktraceon..(ui(req))
            }
            else {
                uiValue <- ..stacktraceon..(ui())
            }
        }
        else {
            if (getCurrentRestoreContext()$active) {
                warning("Trying to restore saved app state, but UI code must be a function for this to work! See ?enableBookmarking")
            }
            uiValue <- ui
        }
    })
    if (is.null(uiValue))
        return(NULL)
    renderPage(uiValue, textConn, showcaseMode, testMode)
    html <- paste(readLines(textConn, encoding = "UTF-8"), collapse = "\\n")
    return(httpResponse(200, content = enc2utf8(html)))
}, serverFuncSource = function ()
{
    server
}, onStart = NULL, options = list(port = 1234), appOptions = list(
    appDir = "/Users/winston/Dropbox/Projects/shiny", bookmarkStore = NULL)),
    port = 1234)After:
[[4]]
runApp(x, port = 1234)

@jcheng5 jcheng5 merged commit cad20a0 into master Oct 31, 2017
@jcheng5 jcheng5 deleted the wch-do-call branch October 31, 2017 18:28
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.

Consider changing do.call(f) to do.call("f")
2 participants