Project

General

Profile

Actions

Feature #18370

closed

Call Exception#full_message to print exceptions reaching the top-level

Added by Eregon (Benoit Daloze) about 3 years ago. Updated almost 3 years ago.

Status:
Closed
Assignee:
-
Target version:
-
[ruby-core:106347]

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)

Related issues 2 (0 open2 closed)

Related to Ruby master - Feature #18296: Custom exception formatting should override `Exception#full_message`.ClosedActions
Related to Ruby master - Feature #18367: Stop the interpreter from escaping error messagesClosedActions
Actions #1

Updated by Eregon (Benoit Daloze) about 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) about 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 (just Exception#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 to full_message such as highlight:, or even introduce their own additional options (keyword arguments).
    For instance error_highlight could use highlight: 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) about 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) about 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) about 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) about 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.

https://github.com/ruby/ruby/blob/b01657c4707eadd9de9573ce9818d0b5f0fe3047/test/ruby/test_exception.rb#L917-L935

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.

https://github.com/ruby/ruby/blob/b01657c4707eadd9de9573ce9818d0b5f0fe3047/test/ruby/test_exception.rb#L941-L952

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.

Actions #11

Updated by mame (Yusuke Endoh) almost 3 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]

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0