From 379b5b43d1a4c2244dd316ee9908d86c7230bba1 Mon Sep 17 00:00:00 2001
From: Nathan Daly <NHDaly@gmail.com>
Date: Fri, 17 Jan 2025 10:28:20 -0700
Subject: [PATCH] Add try/catch around handle_message() to catch errors during
 logging. (#57004)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Fixes https://github.com/JuliaLang/julia/issues/56889.

Before this PR, an exception thrown while constructing the objects to
log (the `msg`) would be caught and logged. However, an exception thrown
while _printing_ the msg to an IO would _not_ be caught, and can abort
the program. This breaks the promise that enabling verbose debug logging
shouldn't introduce new crashes.

After this PR, an exception thrown during handle_message is caught and
logged, just like an exception during `msg` construction:

```julia
julia> struct Foo end

julia> Base.show(::IO, ::Foo) = error("oh no")

julia> begin
           # Unexpectedly, the execption thrown while printing `Foo()` escapes
           @info Foo()
           # So we never reach this line! :'(
           println("~~~~~ ALL DONE ~~~~~~~~")
       end
┌ Error: Exception while generating log record in module Main at REPL[10]:3
│   exception =
│    oh no
│    Stacktrace:
│      [1] error(s::String)
│        @ Base ./error.jl:44
│      [2] show(::IOBuffer, ::Foo)
│        @ Main ./REPL[9]:1
...
│     [30] repl_main
│        @ ./client.jl:593 [inlined]
│     [31] _start()
│        @ Base ./client.jl:568
└ @ Main REPL[10]:3
~~~~~ ALL DONE ~~~~~~~~
```

This PR respects the change made in
https://github.com/JuliaLang/julia/pull/36600 to keep the codegen as
small as possible, by putting the new try/catch into a no-inlined
function, so that we don't have to introduce a new try/catch in the
macro-generated code body.

---------

Co-authored-by: Jameson Nash <vtjnash@gmail.com>
---
 base/logging.jl     | 14 +++++++++++++-
 test/corelogging.jl | 13 +++++++++++++
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/base/logging.jl b/base/logging.jl
index 96c133a239cc0..1bd5a7fbefa8b 100644
--- a/base/logging.jl
+++ b/base/logging.jl
@@ -398,7 +398,7 @@ function logmsg_code(_module, file, line, level, message, exs...)
                         end
                         line = $(log_data._line)
                         local msg, kwargs
-                        $(logrecord) && invokelatest($handle_message,
+                        $(logrecord) && $handle_message_nothrow(
                             logger, level, msg, _module, group, id, file, line;
                             kwargs...)
                     end
@@ -409,6 +409,18 @@ function logmsg_code(_module, file, line, level, message, exs...)
     end
 end
 
+@noinline function handle_message_nothrow(logger, level, msg, _module, group, id, file, line; kwargs...)
+    @nospecialize
+    try
+        @invokelatest handle_message(
+            logger, level, msg, _module, group, id, file, line;
+            kwargs...)
+
+    catch err
+        @invokelatest logging_error(logger, level, _module, group, id, file, line, err, true)
+    end
+end
+
 function process_logmsg_exs(_orig_module, _file, _line, level, message, exs...)
     @nospecialize
     local _group, _id
diff --git a/test/corelogging.jl b/test/corelogging.jl
index 9626f48e4b407..d373b9d591190 100644
--- a/test/corelogging.jl
+++ b/test/corelogging.jl
@@ -114,6 +114,19 @@ end
         @test only(collect_test_logs(logmsg)[1]).kwargs[:x] === "the y"
     end
 end
+@testset "Log message handle_message exception handling" begin
+    # Exceptions in log handling (printing) of msg are caught by default.
+    struct Foo end
+    Base.show(::IO, ::Foo) = 1 ÷ 0
+
+    # We cannot use `@test_logs` here, since test_logs does not actually _print_ the message
+    # (i.e. it does not invoke handle_message). To test exception handling during printing,
+    # we have to use `@test_warn` to see what was printed.
+    @test_warn r"Error: Exception while generating log record in module .*DivideError: integer division error"s @info Foo()
+
+    # Exceptions in log handling (printing) of attributes are caught by default
+    @test_warn r"Error: Exception while generating log record in module .*DivideError: integer division error"s @info "foo" x=Foo()
+end
 
 @testset "Special keywords" begin
     logger = TestLogger()