From 49971550b4c380c43bacc4b32ee781abd1a5bd21 Mon Sep 17 00:00:00 2001 From: Alberto Mengali Date: Mon, 4 Mar 2024 15:45:07 +0100 Subject: [PATCH 1/8] move error throwing to save_payload. Only sends warning during `start()` --- src/PlotlyKaleido.jl | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/src/PlotlyKaleido.jl b/src/PlotlyKaleido.jl index f2631c7..06fc153 100644 --- a/src/PlotlyKaleido.jl +++ b/src/PlotlyKaleido.jl @@ -20,6 +20,15 @@ const P = Pipes() const _mathjax_url_path = "https://cdnjs.cloudflare.com/ajax/libs/mathjax" const _mathjax_last_version = v"2.7.9" +const _error_msg = Ref("") +haserror() = !isempty(_error_msg[]) +function seterror(s::String) + _error_msg[] = s + @error "$(_error_msg[])" + kill_kaleido() + return nothing +end + kill_kaleido() = is_running() && (kill(P.proc); wait(P.proc)) is_running() = isdefined(P, :proc) && isopen(P.stdin) && process_running(P.proc) @@ -49,7 +58,7 @@ function readline_noblock(io) schedule(task) wait(task) out = take!(msg) - out === "Stopped" && error("It looks like the kaleido process is hanging. + out === "Stopped" && seterror("It looks like the kaleido process is hanging. If you are on windows this might be caused by known problems with Kaleido v0.2 on windows. You might want to try forcing a downgrade of the kaleido library to 0.1. Check the Package Readme at https://github.com/JuliaPlots/PlotlyKaleido.jl/tree/main#windows-note for more details") @@ -63,6 +72,7 @@ function start(; kwargs..., ) is_running() && return + _error_msg[] = "" # The kaleido executable must be ran from the artifact directory BIN = Cmd(kaleido(); dir = Kaleido_jll.artifact_dir) # We push the mandatory plotly flag @@ -126,9 +136,11 @@ function start(; P.proc = kproc res = readline_noblock(P.stdout) # {"code": 0, "message": "Success", "result": null, "version": "0.2.1"} - length(res) == 0 && error("Kaleido startup failed.") - code = JSON.parse(res)["code"] - code == 0 || error("Kaleido startup failed with code $code.") + length(res) == 0 && seterror("Kaleido startup failed.") + if !haserror() + code = JSON.parse(res)["code"] + code == 0 || seterror("Kaleido startup failed with code $code.") + end return end @@ -139,6 +151,7 @@ const TEXT_FORMATS = ["svg", "json", "eps"] function save_payload(io::IO, payload::AbstractString, format::AbstractString) + haserror() && error(_error_msg[]) format in ALL_FORMATS || error("Unknown format $format. Expected one of $ALL_FORMATS") bytes = transcode(UInt8, payload) From 0d3a56ee0545a3f10200b5d158afac2d0d2f8830 Mon Sep 17 00:00:00 2001 From: Alberto Mengali Date: Mon, 4 Mar 2024 18:09:36 +0100 Subject: [PATCH 2/8] use Pipes for the error msg --- src/PlotlyKaleido.jl | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/PlotlyKaleido.jl b/src/PlotlyKaleido.jl index 06fc153..cbc863c 100644 --- a/src/PlotlyKaleido.jl +++ b/src/PlotlyKaleido.jl @@ -12,6 +12,7 @@ mutable struct Pipes stdout::Pipe stderr::Pipe proc::Base.Process + error_msg::String Pipes() = new() end @@ -20,11 +21,10 @@ const P = Pipes() const _mathjax_url_path = "https://cdnjs.cloudflare.com/ajax/libs/mathjax" const _mathjax_last_version = v"2.7.9" -const _error_msg = Ref("") -haserror() = !isempty(_error_msg[]) +haserror() = !isempty(P.error_msg) function seterror(s::String) - _error_msg[] = s - @error "$(_error_msg[])" + P.error_msg = s + @error "$(s)" kill_kaleido() return nothing end @@ -72,7 +72,6 @@ function start(; kwargs..., ) is_running() && return - _error_msg[] = "" # The kaleido executable must be ran from the artifact directory BIN = Cmd(kaleido(); dir = Kaleido_jll.artifact_dir) # We push the mandatory plotly flag @@ -134,6 +133,7 @@ function start(; P.stdout = kstdout P.stderr = kstderr P.proc = kproc + P.error_msg = "" res = readline_noblock(P.stdout) # {"code": 0, "message": "Success", "result": null, "version": "0.2.1"} length(res) == 0 && seterror("Kaleido startup failed.") @@ -151,7 +151,7 @@ const TEXT_FORMATS = ["svg", "json", "eps"] function save_payload(io::IO, payload::AbstractString, format::AbstractString) - haserror() && error(_error_msg[]) + haserror() && error(P.error_msg) format in ALL_FORMATS || error("Unknown format $format. Expected one of $ALL_FORMATS") bytes = transcode(UInt8, payload) From d193055a7d52c0452621a3829858086ad917a3c6 Mon Sep 17 00:00:00 2001 From: Alberto Mengali Date: Mon, 4 Mar 2024 18:31:58 +0100 Subject: [PATCH 3/8] increase sleep? --- src/PlotlyKaleido.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PlotlyKaleido.jl b/src/PlotlyKaleido.jl index cbc863c..a0fcc8f 100644 --- a/src/PlotlyKaleido.jl +++ b/src/PlotlyKaleido.jl @@ -48,7 +48,7 @@ function readline_noblock(io) end interrupter = Task() do - sleep(5) + sleep(10) if !istaskdone(task) Base.throwto(task, InterruptException()) end From c81b7451edae50efeb509b4a2d975424e6ff7aad Mon Sep 17 00:00:00 2001 From: Alberto Mengali Date: Mon, 4 Mar 2024 18:40:57 +0100 Subject: [PATCH 4/8] make timeout a kwarg and specify its use in the log error --- src/PlotlyKaleido.jl | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/PlotlyKaleido.jl b/src/PlotlyKaleido.jl index a0fcc8f..d8ba99f 100644 --- a/src/PlotlyKaleido.jl +++ b/src/PlotlyKaleido.jl @@ -36,7 +36,7 @@ is_running() = isdefined(P, :proc) && isopen(P.stdin) && process_running(P.proc) restart(; kwargs...) = (kill_kaleido(); start(; kwargs...)) # The content of this function is inspired from https://discourse.julialang.org/t/readline-with-default-value-if-no-input-after-timeout/100388/2?u=disberd -function readline_noblock(io) +function readline_noblock(io; timeout = 10) msg = Channel{String}(1) task = Task() do @@ -48,7 +48,7 @@ function readline_noblock(io) end interrupter = Task() do - sleep(10) + sleep(timeout) if !istaskdone(task) Base.throwto(task, InterruptException()) end @@ -57,11 +57,14 @@ function readline_noblock(io) schedule(interrupter) schedule(task) wait(task) + kaleido_version = read(joinpath(Kaleido_jll.artifact_dir, "version"), String) out = take!(msg) - out === "Stopped" && seterror("It looks like the kaleido process is hanging. -If you are on windows this might be caused by known problems with Kaleido v0.2 on windows. + out === "Stopped" && seterror("It looks like the Kaleido (version $(kaleido_version)) process is hanging. +If you are on Windows this might be caused by known problems with Kaleido v0.2 on Windows. You might want to try forcing a downgrade of the kaleido library to 0.1. -Check the Package Readme at https://github.com/JuliaPlots/PlotlyKaleido.jl/tree/main#windows-note for more details") +Check the Package Readme at https://github.com/JuliaPlots/PlotlyKaleido.jl/tree/main#windows-note for more details. + +If you think this is not your case, you might try using a longer timeout to check if the process is not responding (defaults to 10 seconds) by passing the desired value in seconds using the `timeout` kwarg when calling `PlotlyKaleido.start` or `PlotlyKaleido.restart`") return out end @@ -69,6 +72,7 @@ function start(; plotly_version = missing, mathjax = missing, mathjax_version::VersionNumber = _mathjax_last_version, + timeout = 10, kwargs..., ) is_running() && return @@ -135,7 +139,7 @@ function start(; P.proc = kproc P.error_msg = "" - res = readline_noblock(P.stdout) # {"code": 0, "message": "Success", "result": null, "version": "0.2.1"} + res = readline_noblock(P.stdout; timeout) # {"code": 0, "message": "Success", "result": null, "version": "0.2.1"} length(res) == 0 && seterror("Kaleido startup failed.") if !haserror() code = JSON.parse(res)["code"] From 21a1ff65971682b85586146eccf0cfc625853afa Mon Sep 17 00:00:00 2001 From: Alberto Mengali Date: Mon, 4 Mar 2024 18:52:38 +0100 Subject: [PATCH 5/8] improve error message --- src/PlotlyKaleido.jl | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/PlotlyKaleido.jl b/src/PlotlyKaleido.jl index d8ba99f..6574ff8 100644 --- a/src/PlotlyKaleido.jl +++ b/src/PlotlyKaleido.jl @@ -59,9 +59,11 @@ function readline_noblock(io; timeout = 10) wait(task) kaleido_version = read(joinpath(Kaleido_jll.artifact_dir, "version"), String) out = take!(msg) - out === "Stopped" && seterror("It looks like the Kaleido (version $(kaleido_version)) process is hanging. -If you are on Windows this might be caused by known problems with Kaleido v0.2 on Windows. -You might want to try forcing a downgrade of the kaleido library to 0.1. + out === "Stopped" && seterror("It looks like the Kaleido process is hanging. +The unresponsive process will be killed, but this means that you will not be able to save figures using `savefig`. + +If you are on Windows this might be caused by known problems with Kaleido v0.2 on Windows (you are using version $(kaleido_version)). +You might want to try forcing a downgrade of the Kaleido_jll library to 0.1. Check the Package Readme at https://github.com/JuliaPlots/PlotlyKaleido.jl/tree/main#windows-note for more details. If you think this is not your case, you might try using a longer timeout to check if the process is not responding (defaults to 10 seconds) by passing the desired value in seconds using the `timeout` kwarg when calling `PlotlyKaleido.start` or `PlotlyKaleido.restart`") From f42e6ef53c8b8743e337ae735e2766f0749e0a98 Mon Sep 17 00:00:00 2001 From: Alberto Mengali Date: Tue, 5 Mar 2024 08:45:10 +0100 Subject: [PATCH 6/8] defaults to warn rather than error. Remove the error message variable from Pipes --- src/PlotlyKaleido.jl | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/PlotlyKaleido.jl b/src/PlotlyKaleido.jl index 6574ff8..83b8845 100644 --- a/src/PlotlyKaleido.jl +++ b/src/PlotlyKaleido.jl @@ -12,7 +12,6 @@ mutable struct Pipes stdout::Pipe stderr::Pipe proc::Base.Process - error_msg::String Pipes() = new() end @@ -21,10 +20,8 @@ const P = Pipes() const _mathjax_url_path = "https://cdnjs.cloudflare.com/ajax/libs/mathjax" const _mathjax_last_version = v"2.7.9" -haserror() = !isempty(P.error_msg) -function seterror(s::String) - P.error_msg = s - @error "$(s)" +function warn_and_kill(s::String) + @warn "$s" kill_kaleido() return nothing end @@ -59,7 +56,7 @@ function readline_noblock(io; timeout = 10) wait(task) kaleido_version = read(joinpath(Kaleido_jll.artifact_dir, "version"), String) out = take!(msg) - out === "Stopped" && seterror("It looks like the Kaleido process is hanging. + out === "Stopped" && warn_and_kill("It looks like the Kaleido process is not responding. The unresponsive process will be killed, but this means that you will not be able to save figures using `savefig`. If you are on Windows this might be caused by known problems with Kaleido v0.2 on Windows (you are using version $(kaleido_version)). @@ -139,13 +136,12 @@ function start(; P.stdout = kstdout P.stderr = kstderr P.proc = kproc - P.error_msg = "" res = readline_noblock(P.stdout; timeout) # {"code": 0, "message": "Success", "result": null, "version": "0.2.1"} - length(res) == 0 && seterror("Kaleido startup failed.") + length(res) == 0 && warn_and_kill("Kaleido startup failed.") if !haserror() code = JSON.parse(res)["code"] - code == 0 || seterror("Kaleido startup failed with code $code.") + code == 0 || warn_and_kill("Kaleido startup failed with code $code.") end return end @@ -157,7 +153,9 @@ const TEXT_FORMATS = ["svg", "json", "eps"] function save_payload(io::IO, payload::AbstractString, format::AbstractString) - haserror() && error(P.error_msg) + isrunning() || error("It looks like the Kaleido process is not running, so you can not save plotly figures. +Remember to start the process before using `savefig` by calling `PlotlyKaleido.start()`. +If the process was killed due to an error during initialization, you will receive a warning when the `PlotlyKaleido.start` function is executing") format in ALL_FORMATS || error("Unknown format $format. Expected one of $ALL_FORMATS") bytes = transcode(UInt8, payload) From 7a15b75acff7528e00161fe63e6391f69f9cd8f4 Mon Sep 17 00:00:00 2001 From: Alberto Mengali Date: Tue, 5 Mar 2024 08:47:30 +0100 Subject: [PATCH 7/8] typo --- src/PlotlyKaleido.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/PlotlyKaleido.jl b/src/PlotlyKaleido.jl index 83b8845..c70086a 100644 --- a/src/PlotlyKaleido.jl +++ b/src/PlotlyKaleido.jl @@ -139,7 +139,7 @@ function start(; res = readline_noblock(P.stdout; timeout) # {"code": 0, "message": "Success", "result": null, "version": "0.2.1"} length(res) == 0 && warn_and_kill("Kaleido startup failed.") - if !haserror() + if !is_running() code = JSON.parse(res)["code"] code == 0 || warn_and_kill("Kaleido startup failed with code $code.") end @@ -153,7 +153,7 @@ const TEXT_FORMATS = ["svg", "json", "eps"] function save_payload(io::IO, payload::AbstractString, format::AbstractString) - isrunning() || error("It looks like the Kaleido process is not running, so you can not save plotly figures. + is_running() || error("It looks like the Kaleido process is not running, so you can not save plotly figures. Remember to start the process before using `savefig` by calling `PlotlyKaleido.start()`. If the process was killed due to an error during initialization, you will receive a warning when the `PlotlyKaleido.start` function is executing") format in ALL_FORMATS || error("Unknown format $format. Expected one of $ALL_FORMATS") From 5a0c00fc1da85f49925dbfbf25d1dd7446cc416c Mon Sep 17 00:00:00 2001 From: Alberto Mengali Date: Tue, 5 Mar 2024 08:54:50 +0100 Subject: [PATCH 8/8] other typo --- src/PlotlyKaleido.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PlotlyKaleido.jl b/src/PlotlyKaleido.jl index c70086a..97ca3f7 100644 --- a/src/PlotlyKaleido.jl +++ b/src/PlotlyKaleido.jl @@ -139,7 +139,7 @@ function start(; res = readline_noblock(P.stdout; timeout) # {"code": 0, "message": "Success", "result": null, "version": "0.2.1"} length(res) == 0 && warn_and_kill("Kaleido startup failed.") - if !is_running() + if is_running() code = JSON.parse(res)["code"] code == 0 || warn_and_kill("Kaleido startup failed with code $code.") end