Feature #18370
closedCall Exception#full_message to print exceptions reaching the top-level
Description
Extracted from https://bugs.ruby-lang.org/issues/18296#note-6.
I think this a clear gain to improve consistency in how exceptions are shown, and it also makes it easier to evolve exception formatting in the future.
It would also solve https://bugs.ruby-lang.org/issues/18367.
https://bugs.ruby-lang.org/issues/18296#note-7 has more specifics, I'll copy here for convenience:
mame (Yusuke Endoh) wrote in #note-6:
Does this proposal include that the ruby interpreter should use
#full_message
to show the error information? This is an incompatibility, is it acceptable?
Yes, let's fix that.
I don't think there is much if any compatibility issue here.
The output of the uncaught exception handler is already the same as the default Exception#full_message AFAIK, let's actually call it.
TruffleRuby already calls exc.full_message
for the uncaught exception handler.
If the custom exc.full_message
raises an exception, then it's best to report that exception and the original exception using the default Exception#full_message
(written in C).
This is the current TruffleRuby output for that case and I think it's clear:
$ ruby -e 'class Foo < Exception; def full_message(**); raise "bar"; end; end; raise Foo, "message"'
Error while formatting Ruby exception:
-e:1:in `full_message': bar (RuntimeError)
from <internal:core> core/truffle/exception_operations.rb:183:in `get_formatted_backtrace'
Original Ruby exception:
-e:1:in `<main>': message (Foo)
Updated by Eregon (Benoit Daloze) almost 3 years ago
- Related to Feature #18296: Custom exception formatting should override `Exception#full_message`. added
- Related to Feature #18367: Stop the interpreter from escaping error messages added
Updated by mame (Yusuke Endoh) almost 3 years ago
Please clearly write the motivation first. Cite from https://bugs.ruby-lang.org/issues/18296#note-14
- Callers can choose whether they want
did_you_mean/error_highlight
(individually) in the exception message or not, and they have a way to get the original exception message (justException#message
would be the original message). - It makes it possible to choose more dynamically than global --disable-
did_you_mean/error_highlight
. Given e.g. error_highlight has a non-trivial performance impact, it seems useful to be able only get this output in some situations where the caller might know whether it is useful. -
did_you_mean/error_highlight
can find out about options given tofull_message
such ashighlight:
, or even introduce their own additional options (keyword arguments).
For instanceerror_highlight
could usehighlight:
to decide whether to use ANSI sequences to colorize the failing call, or to use^^^^
on the next line (I know @ioquatix (Samuel Williams) cares about this flexibility).
This can be useful e.g. to have different output for Exception#inspect (don't want ANSI escape sequences, and probably not extra information) and printing the exception to a terminal (all information and ANSI escape sequences). Some test harness might choose different trade-offs, and this lets them choose. - It's a cleaner integration than overriding
#message
, it can allow more gems to customize exceptions and there is no problem if some exception subclasses override#message
.
To be honest, I don't feel the need so much.
Even if the interpreter uses #full_message
for an uncaught exception, I will extend #message
in error_highlight for a while.
The reason is because of some application monitoring services like Sentry, DataDog, ScoutAPM, etc. I believe that they are not using #full_message
because they are providing their own backtrace filtering. It is not trivial to use #full_message
for their use case.
I may change my opinion if the developers of Sentry, DataDog, and/or ScoutSPM join this discussion.
I guess that we need the following changes in addition to eregon's proposal.
- Introduce
Exception#additional_message(highlight: true, **)
-
full_message(**opt)
should output#message
,#additional_message(*opt)
, and backtrace in turn. - did_you_mean and error_highlight overwrites
#additional_message
instead of#message
In this case, APM services can use #additional_message
to construct a message string. In other words, this requires APM services to support #additional_message
. I'm not positive against this change unless the APM developers join this dicussion. Rails developer may want to join the discussion because they also have their own error message renderer.
Updated by Eregon (Benoit Daloze) almost 3 years ago
Main motivation for this issue is consistency.
AFAIK Exception#full_message
was introduced to have exactly the same output as the top-level handler (with the default keywords), but be able to produce it from anywhere (including test harness, rails top handler, etc).
So it's also a bug for any difference in output there.
The easiest way to ensure they stay in sync is to use Exception#full_message
for the top-level handler.
Also I guess this might remove a fair bit of duplicated code in CRuby.
Updated by nobu (Nobuyoshi Nakada) almost 3 years ago
I think it is a bug that Exception#full_message
doesn't escape control chars.
https://github.com/nobu/ruby/pull/new/full_message-escape
Updated by Eregon (Benoit Daloze) almost 3 years ago
#18367 wants the opposite, i.e., not escape control chars.
As already mentioned, JRuby & TruffleRuby both currently already have the behavior to not escape control chars.
BTW, wouldn't that change even escape \n? (looking at https://github.com/ruby/ruby/blob/6ff9fcdfa8c6d55474e6de70ad241625b9265a5b/string.c#L6340-L6356 + ("\x00"..." ").to_a
)
Updated by mame (Yusuke Endoh) almost 3 years ago
This ticket was discussed at today's dev-meeting (https://github.com/ruby/dev-meeting-log/blob/master/DevelopersMeeting20211209Japan.md). We need to first decide whether #18367 is acceptable or not.
If we were to accept #18367, this proposal is one of the possible approaches. @matz (Yukihiro Matsumoto) said that it would be worth considering if it really saves code duplication. Matz also said that "consistency" is a very weak reason to change anything.
Updated by mame (Yusuke Endoh) almost 3 years ago
Discussed on the dev-meeting.
#18367 was accepted, so @matz (Yukihiro Matsumoto) said "give it a try". Need to create a PR.
Updated by Eregon (Benoit Daloze) almost 3 years ago
Great!
@mame (Yusuke Endoh) Do you plan to make a PR or should I give it a try?
Updated by mame (Yusuke Endoh) almost 3 years ago
I have given it a try, but I faced two non-trivial test failures.
One is TestException#test_cause_exception_in_cause_message.
The test raises an exception whose "cause" has a broken to_s.
The current error printer first outputs an error message successfully, and then attempts to output "cause", which fails due to the broken "to_s".
Because it is impossible to get "full_message" from this exception, a naive approach to make the error printer use "full_message" shows nothing, which is not very useful.
If we keep the current incremental error printer, we cannot save code duplication.
The other is TestException#test_output_string_encoding.
The current Exception#full_message
returns an ASCII-8BIT string.
When the encodings of the full_message and $stderr
is incompatible, writing the string to $stderr
fails.
I'm unsure if it is trivial to restore the encoding to it.
I'll first put effort on the implementation of detailed_message #18438
Updated by Dan0042 (Daniel DeLorme) almost 3 years ago
Would it be feasible for #full_message to accept a String/IO argument to which it writes the parts of the message? So the error printer could call err.full_message(output_to: $stderr)
in order to avoid buffering.
Updated by mame (Yusuke Endoh) over 2 years ago
- Status changed from Open to Closed
Applied in changeset git|35ff545bb689f5af93ac603ea1f512705e0dc249.
Exception#detailed_message is added
Also, the default error printer and Exception#full_message use the
method instead of Exception#message
to get the message string.
Exception#detailed_message
calls Exception#message
, decorates and
returns the result. It adds some escape sequences to highlight, and the
class name of the exception to the end of the first line of the message.
[Feature #18370]