From 370c79e3e598d789173074b09a3c8bf8ba341c13 Mon Sep 17 00:00:00 2001 From: Ryan Patrick Kyle Date: Mon, 12 Aug 2019 00:53:07 -0400 Subject: [PATCH 1/5] rename pruned_errors to prune_errors --- R/dash.R | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/R/dash.R b/R/dash.R index e073b982..a27f2fed 100644 --- a/R/dash.R +++ b/R/dash.R @@ -286,7 +286,7 @@ Dash <- R6::R6Class( output_value <- getStackTrace(do.call(callback, callback_args), debug = private$debug, - pruned_errors = private$pruned_errors) + prune_errors = private$prune_errors) # reset callback context private$callback_context_ <- NULL @@ -525,7 +525,7 @@ Dash <- R6::R6Class( port = Sys.getenv('DASH_PORT', 8050), block = TRUE, showcase = FALSE, - pruned_errors = TRUE, + dev_tools_prune_errors = TRUE, debug = FALSE, dev_tools_ui = NULL, dev_tools_props_check = NULL, @@ -545,7 +545,7 @@ Dash <- R6::R6Class( self$config$props_check <- FALSE } - private$pruned_errors <- pruned_errors + private$prune_errors <- dev_tools_prune_errors private$debug <- debug self$server$ignite(block = block, showcase = showcase, ...) @@ -569,7 +569,7 @@ Dash <- R6::R6Class( # initialize flags for debug mode and stack pruning, debug = NULL, - pruned_errors = NULL, + prune_errors = NULL, stack_message = NULL, # callback context From f7adc8385e89ad336658f09a6457e13155693ef9 Mon Sep 17 00:00:00 2001 From: Ryan Patrick Kyle Date: Mon, 12 Aug 2019 00:54:06 -0400 Subject: [PATCH 2/5] rename pruned to prune --- R/utils.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/R/utils.R b/R/utils.R index d8d67664..9c50dcfa 100644 --- a/R/utils.R +++ b/R/utils.R @@ -685,7 +685,7 @@ stackTraceToHTML <- function(call_stack, # and capture the call stack. By default, the call # stack will be "pruned" of error handling functions # for greater readability. -getStackTrace <- function(expr, debug = FALSE, pruned_errors = TRUE) { +getStackTrace <- function(expr, debug = FALSE, prune_errors = TRUE) { if(debug) { tryCatch(withCallingHandlers( expr, @@ -711,7 +711,7 @@ getStackTrace <- function(expr, debug = FALSE, pruned_errors = TRUE) { reverseStack <- rev(calls) - if (pruned_errors) { + if (prune_errors) { # this line should match the last occurrence of the function # which raised the error within the call stack; prune here indexFromLast <- match(TRUE, lapply(reverseStack, function(currentCall) { From b170cfa72dc52a335a3d86763fad731fa27fcb6c Mon Sep 17 00:00:00 2001 From: Ryan Patrick Kyle Date: Tue, 13 Aug 2019 11:02:06 -0400 Subject: [PATCH 3/5] :hammer: handle stop errors --- R/utils.R | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/R/utils.R b/R/utils.R index 9c50dcfa..d9dfb393 100644 --- a/R/utils.R +++ b/R/utils.R @@ -692,9 +692,20 @@ getStackTrace <- function(expr, debug = FALSE, prune_errors = TRUE) { error = function(e) { if (is.null(attr(e, "stack.trace", exact = TRUE))) { calls <- sys.calls() + reverseStack <- rev(calls) attr(e, "stack.trace") <- calls - errorCall <- e$call[[1]] - + + if (!is.null(e$call[[1]])) + errorCall <- e$call[[1]] + else { + # attempt to capture the error or warning if thrown by + # simpleError or simpleWarning (which may arise for user-defined errors) + # + # the first matching call in the reversed stack will always be + # getStackTrace, so we select the second match instead + errorCall <- reverseStack[grepl(x=reverseStack, "simpleError|simpleWarning")][[2]] + } + functionsAsList <- lapply(calls, function(completeCall) { currentCall <- completeCall[[1]] @@ -709,8 +720,6 @@ getStackTrace <- function(expr, debug = FALSE, prune_errors = TRUE) { }) - reverseStack <- rev(calls) - if (prune_errors) { # this line should match the last occurrence of the function # which raised the error within the call stack; prune here @@ -728,14 +737,18 @@ getStackTrace <- function(expr, debug = FALSE, prune_errors = TRUE) { # to stop at the correct position. if (is.function(currentCall[[1]])) { identical(deparse(errorCall), deparse(currentCall[[1]])) - } else { + } else if (currentCall[[1]] == "stop") { + # handle case where function developer deliberately invokes a stop + # condition and halts function execution + identical(deparse(errorCall), deparse(currentCall)) + } + else { FALSE } } ) ) - # the position to stop at is one less than the difference # between the total number of calls and the index of the # call throwing the error @@ -746,12 +759,14 @@ getStackTrace <- function(expr, debug = FALSE, prune_errors = TRUE) { functionsAsList <- removeHandlers(functionsAsList) } + # use deparse in case the call throwing the error is a symbol, + # since this cannot be "printed" without deparsing the call warning(call. = FALSE, immediate. = TRUE, sprintf("Execution error in %s: %s", - functionsAsList[[length(functionsAsList)]], + deparse(functionsAsList[[length(functionsAsList)]]), conditionMessage(e))) stack_message <- stackTraceToHTML(functionsAsList, - functionsAsList[[length(functionsAsList)]], + deparse(functionsAsList[[length(functionsAsList)]]), conditionMessage(e)) assign("stack_message", value=stack_message, From b9d397de40ced27e0bb1a97aef44b92fe151d514 Mon Sep 17 00:00:00 2001 From: Ryan Patrick Kyle Date: Tue, 3 Sep 2019 15:13:50 -0400 Subject: [PATCH 4/5] :see_no_evil: remove redundant reverseStack --- R/utils.R | 2 -- 1 file changed, 2 deletions(-) diff --git a/R/utils.R b/R/utils.R index 282e9a3e..92d6e261 100644 --- a/R/utils.R +++ b/R/utils.R @@ -771,8 +771,6 @@ getStackTrace <- function(expr, debug = FALSE, prune_errors = TRUE) { }) - reverseStack <- rev(calls) - if (prune_errors) { # this line should match the last occurrence of the function # which raised the error within the call stack; prune here From 1029ec48b4ee76c3c6b85ad999532876a98c5b38 Mon Sep 17 00:00:00 2001 From: Ryan Patrick Kyle Date: Fri, 6 Sep 2019 00:34:28 -0400 Subject: [PATCH 5/5] :rotating_light: add test for stop errors --- .../integration/callbacks/test_handle_stop.py | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 tests/integration/callbacks/test_handle_stop.py diff --git a/tests/integration/callbacks/test_handle_stop.py b/tests/integration/callbacks/test_handle_stop.py new file mode 100644 index 00000000..fce8831f --- /dev/null +++ b/tests/integration/callbacks/test_handle_stop.py @@ -0,0 +1,46 @@ +from selenium.webdriver.support.select import Select + +app = """ +library(dash) +library(dashHtmlComponents) +library(dashCoreComponents) + +app <- Dash$new() + +app$layout( + htmlDiv( + list( + dccDropdown(options = list( + list(label = "Red", value = "#FF0000"), + list(label = "Throw error", value = "error") + ), + id = "input-choice", + value = "error"), + htmlDiv(id="div-choice") + ) + ) +) + +app$callback(output(id = 'div-choice', property = 'children'), + list(input(id = 'input-choice', property = 'value')), + function(choice) { + if (choice == "error") { + stop(simpleError("Throwing an error by request")) + } + if (!is.null(unlist(choice))) { + return(sprintf("Choice was %s", choice)) + } else { + return(sprintf("Make a choice")) + } + }) + +app$run_server(debug=TRUE) +""" + + +def test_rshs001_handle_stop(dashr): + dashr.start_server(app) + dashr.wait_for_text_to_equal( + ".dash-fe-error__title", + "Callback error updating div-choice.children" + )