Feature #18438: Add `Exception#additional_message` to show additional error information

• Status: Closed
• Assignee: -
• Target version: -

[mame (Yusuke Endoh)]

Proposal

I'd like to introduce a method Exception#additional_message, and let the Ruby error printer show it after Exception#message.

class MyError < StandardError
  def message = "my error!"
  def additional_message = "This is\nan additional\nmessage"
end

raise MyError

$ ./miniruby test.rb
test.rb:6:in `<main>': my error! (MyError)
| This is
| an additional
| message

PoC implementation: https://github.com/ruby/ruby/pull/5351

Motivation

At the present time, did_you_mean and error_highlight overrides Exception#message to add their suggestions.

begin; 1.time; rescue NoMethodError; pp $!.message; end
#=> "undefined method `time' for 1:Integer\n" +
#   "\n" +
#   "  1.time\n" +
#   "   ^^^^^\n" +
#   "Did you mean?  times"

This implementation approach has a practical problem. Because it changes the return value of Exception#message, it breaks a test that checks the return value of Exception#message.
Though such a test is never recommended, I encountered some actual cases when creating error_highlight. See the change of tests of minitest as a typical example: https://github.com/seattlerb/minitest/pull/880/files

Currently, error_highlight shows hint information only for NoMethodError because it is relatively rare to check the message of NameError. Still, it broke some tests unfortunately, though. If possible, I'd like to add suggestions to other kinds of errors, but it will break much more tests.

If Exception#additional_message is introduced, and if did_you_mean and error_highlight overrides the method to add their suggestions, this problem will not occur because they no longer changes the result value of #message method.

Cooperation needed

Currently, many Ruby/Rails users montiors their production services by using application monitoring services such as Sentry, DataDog, ScoutAPM, etc. The original motivation of error_highlight is for production (see #17930), so it will lose the significance if such services do not support Exception#additional_message. So, I'd like to hear opinions from developers of such services. If they are against this proposal or if we can't get their cooperation, I don't think my proposal should be accepted.

If you are a developer of these services, I would be very grateful if you could comment on this ticket. @ivoanjo (Ivo Anjo)

Bikesheds

• I'm unsure if Exception#additional_message is a good name. Please propose alternatives if it is not good.
• Currently, the result of addtional_message is printed with no escape. This may be a more compatible solution against https://bugs.ruby-lang.org/issues/18367.
• It may be good to let Exception#additional_message accept highlight keyword as boolean information whether the output target is a terminal or not. Currently Exception#full_message accepts it. I have no plan to use the information in error_highlight, though. Not only highlight but also any keywords may be forwarded from full_message(**opt) to additional_message(**opt) for future use case.
• My current PoC adds prefixs "| " before each line of addtional_message. I'm unsure if this is good or bad. I'm happy to change or remove the prefixes.

[st0012 (Stan Lo)]

• File 截圖 2021-12-27 15.56.00.png 截圖 2021-12-27 15.56.00.png added

Hello, I'm Stan, the maintainer of Sentry's Ruby SDK.
I think Sentry currently displays the overridden exception message as intended (see the image below). So to us, this separation is not needed atm.
But if you want to extract the message as additional_message to keep things clean, that'll work for us too. Since Sentry doesn't accept any complementary attribute for exceptions, I'll just concat it with message and it should look the same.

Thanks for implementing such a great feature :-)

[mame (Yusuke Endoh)]

@st0012 (Stan Lo) Thank you very much for your reply!

So to us, this separation is not needed atm.

The motivation of this separation is not for your services. Some Ruby programmers write a test to check the exception message like this.

exc = (raise SomeError rescue $!)
assert_equal("exception message", exc.message)

This test will break if error_highlight extends SomeError class because currently error_highlight changes the return value of SomeError#message. If error_highlight overrides #additional_message, this kind of failures will not occur.

But if you want to extract the message as additional_message to keep things clean, that'll work for us too. Since Sentry doesn't accept any complementary attribute for exceptions, I'll just concat it with message and it should look the same.

Yes, I think that's a good enough way to handle the change!

[ivoanjo (Ivo Anjo)]

Thanks for the ping!

I'll need to sync with my colleagues at Datadog and may take a few days because a lot of people are off, but I have a few questions:

1. Would additional_message only be something that gems like did_you_mean and error_highlight use (e.g. better debugging/developer experience tools), or do you see normal user code using it as well?
2. The prefixed | seem harmless, but could you clarify what their use-case is? Is it to make it clear, when printing, which part is the regular message and which part is the additional one?

[byroot (Jean Boussier)]

What if instead of having two distinct #message and #additional_message that need to be concatenated, there would simply be an alternative, more descriptive method?

e.g.

Exception#description which would be:

def message
  @message
end

def description(highlight: false)
  "#{message}\nSome extra info"
end

def full_message(highlight: false, order: ...)
  "#{description(highlight: highlight)}\n#{backtrace...}"
end

The advantage being that it allows description to modify message.

[mame (Yusuke Endoh)]

ivoanjo (Ivo Anjo) wrote in #note-3:

1. Would additional_message only be something that gems like did_you_mean and error_highlight use (e.g. better debugging/developer experience tools), or do you see normal user code using it as well?

My current motivation is only did_you_mean and error_highlight. But it is difficult to force everyone else not to use it, so I guess some users will use it for any reason that I don't see currently.

2. The prefixed | seem harmless, but could you clarify what their use-case is? Is it to make it clear, when printing, which part is the regular message and which part is the additional one?

I have no strong use case. I'm okay to remove the prefix. I thought that they were useful to distinguish between a regular message and additional one, but it is possible to "fake" it by crafting a #message, such as "the first line\n| faked additional message".

byroot (Jean Boussier) wrote in #note-4:

What if instead of having two distinct #message and #additional_message that need to be concatenated, there would simply be an alternative, more descriptive method?
snip
The advantage being that it allows description to modify message.

I wonder if it is a good idea or not. Allowing to modify message might be useful in some cases, but I'm afraid if it would bring rather confusion.

[Dan0042 (Daniel DeLorme)]

So this serves roughly the same purpose as #18296

I also have a slight preference for a description method that includes the message. It seem simpler for everyone (including Sentry, Datadog, etc.) to use e.description instead of e.message + "\n" + e.additional_message, and also easier to support older rubies. But I'm curious if the additional_message approach has some advantage I overlooked.

There's also the question of who/what is responsible for the ansi/terminal formatting? It seems in this design, additional_message is a simple string and all formatting (bold) is handled by full_message. Nice and simple. With description we can (should?) delegate all formatting to this method, so it's very flexible and powerful.

byroot (Jean Boussier) wrote in #note-4:

Exception#description which would be:

I had the same thought initially but there's a hiccup: the description needed by full_message should include the (underlined) error type, but the description needed by Sentry should not. That makes the design somewhat less elegant. You would need something like

class Exception
  def description(highlight: false, append_type: false, **)
    msg = message
    if append_type
      type = self.class.name
      type = ANSI_UNDERLINE_ON + type + ANSI_UNDERLINE_OFF if highlight
      msg = msg.sub(/$/, " (" + type + ")")
    end
    msg = ANSI_BOLD_ON + msg + ANSI_BOLD_OFF if highlight
    msg
  end

  def full_message(**opts)
    description(highlight: true, append_type: true, **opts) + "\n" + backtrace_str
  end
end

[Eregon (Benoit Daloze)]

• Related to Feature #18296: Custom exception formatting should override `Exception#full_message`. added

[Eregon (Benoit Daloze)]

@mame (Yusuke Endoh) Thanks for making this proposal, I believe it goes in the right direction.

Dan0042 (Daniel DeLorme) wrote in #note-6:

I had the same thought initially but there's a hiccup: the description needed by full_message should include the (underlined) error type, but the description needed by Sentry should not. That makes the design somewhat less elegant. You would need something like

That part, showing the exception class name, should be handled by #full_message, like it currently is.
description would only return "a more complete message" but not try to do any formatting which is already handled by #full_message (like the exception class, backtrace, causes, etc).
In fact, full_message is far more complicated than what is shown in @byroot's example (which is fine since it's illustrative).
See https://github.com/oracle/truffleruby/blob/39d740bd3eb8abafe737656055850fc7b3a4d1dc/src/main/ruby/truffleruby/core/exception.rb#L90-L130 and https://github.com/oracle/truffleruby/blob/39d740bd3eb8abafe737656055850fc7b3a4d1dc/src/main/ruby/truffleruby/core/truffle/exception_operations.rb#L94-L112 for example.

I think the description approach is nice because it's easier to call (no manual concat) and also more flexible (access to original message, access to kwargs). +1 from me.
Monitoring services can then just call description(highlight: false) instead of message.

[marcotc (Marco Costa)]

👋, I maintain Datadog's application monitoring gem, alongside Ivo.

ivoanjo (Ivo Anjo) wrote in #note-3:

I'll need to sync with my colleagues at Datadog and may take a few days because a lot of people are off

Following up here, I agree with the description proposal:

Eregon (Benoit Daloze) wrote in #note-8:

I think the description approach is nice because it's easier to call (no manual concat) and also more flexible (access to original message, access to kwargs). +1 from me.
Monitoring services can then just call description(highlight: false) instead of message.

For us, calling description(highlight: false) to have the richest possible textual information, then separately calling backtrace to get the code location would be the ideal APIs for error reporting.

[nobu (Nobuyoshi Nakada)]

Exception#description seems to add too much flexibility and make responsibilities vague.

[Eregon (Benoit Daloze)]

@nobu (Nobuyoshi Nakada) How so? Do you have an example?

The responsibilities for description are to call super and include it in the return value. That's it, nothing else.

The same goes for additional_message and is necessary if there are multiple definitions of additional_message in the ancestors.
Except it's less clear for additional_message because the default additional_message returns "".
And so how to avoid an extra \n before? That'd probably means harder to write a correct additional_message => msg = super; msg.empty? ? extra : "#{msg}\n#{extra}".
This is not a problem for description, where super always returns a non-empty String (unless the original message is empty, but that's then basically a bug of who raised it) => "#{super}\n#{extra}.

The class, backtraces and causes are added later by full_message, because there is no value to add them before and it's not trivial to add them (the class is always shown on the first line, even for a multi-line message, that's already current behavior).

[Dan0042 (Daniel DeLorme)]

@nobu (Nobuyoshi Nakada) did you mean that the name "description" is vague or the concept itself? Would you find it less vague if it was named something else? (e.g. "detailed_message" or something)

[st0012 (Stan Lo)]

It seem simpler for everyone (including Sentry, Datadog, etc.) to use e.description instead of e.message + "\n" + e.additional_message, and also easier to support older rubies.

Not saying I like that but I think it's generally ok because

1. Not many developers need to do this. Perhaps just SDK authors and some gem authors.
2. We still need to make the change anyway and maintain compatibility between versions.

Exception#description seems to add too much flexibility and make responsibilities vague.

I also think description could be confusing. I'd say it's mainly because of the name instead of the concept. If I understand it correctly, it looks like full_message will be built on top of description, which will be message + additional information (like from error_lighlight)?

To me, it's hard to tell the difference between description, message and full_message from their names and certainly doesn't help understand the relationship between them.

I think complemented_message will be a clearer (but also verbose) name?

Then we'll have:

• message - the most fundamental message of the exception.
• complemented_message - message with additional information to help debugging
• full_message - all of the above + type info + backtrace...etc.

[mame (Yusuke Endoh)]

We discussed this topic yesterday at the dev-meeting. @matz (Yukihiro Matsumoto) basically liked the API style of Exception#description, but disliked the name. full_message calls description, and description calls message, which looked awkward to matz. Anyone have a good suggestion about alternative names? Maybe *_message which comes between full_message and just `message would be preferable.

[Eregon (Benoit Daloze)]

detailed_message maybe? (also the term used in https://bugs.ruby-lang.org/issues/18296#note-20)

IMHO description is fine and a good term in relation to message.
full_message is a weird name to me because it includes the backtrace and it's not really a "message" anymore, but that's how it is and seems unlikely worth to be renamed now that it is established.

[mame (Yusuke Endoh)]

@matz (Yukihiro Matsumoto) accepted the name detailed_message. I'll update and improve my PR (or volunteer is welcome)

Note: the rdoc of detailed_message should explicitly state that the method should accept highlight keyword that allows using ANSI terminal escape sequences to highlight the error message. Also, the method should accept and ignore any unknown keywords because some gems (say, error_highlight) can accept a custom keyword to enable their own formatting customization (say, error.full_message(error_highlight: false).

[Dan0042 (Daniel DeLorme)]

Very happy about this.

mame (Yusuke Endoh) wrote in #note-16:

Note: the rdoc of detailed_message should explicitly state that the method should accept highlight keyword that allows using ANSI terminal escape sequences to highlight the error message.

I'd like to ask for clarification regarding this. In #full_message, the #detailed_message is wrapped with bold, correct? In that case the highlight keyword for #detailed_message is for any highlighting except bold? That would also mean #detailed_message must not use "\e[0m" otherwise that would break the highlighting of #full_message? I think it would be good to clarify how the responsability of terminal highlighting is shared between #full_message and #detailed_message.

[mame (Yusuke Endoh)]

I talked with @matz (Yukihiro Matsumoto) and @nobu (Nobuyoshi Nakada) about that.

The current implementation of Exception#full_message adds \e[1m (i.e., bold start) for each beginning of line of #message, and e[m (i.e., bold end) for each end. (Also, it adds " (RuntimeError)" or something to the last of the first line.)
With that in mind, #detailed_message can freely add their own escape sequences. Exception#full_message won't check the result of detailed_message.

BTW, the behavior of Exception#full_message (adding \e[1m and e[m) will be moved to Exception#detailed_message. That is, a rough spec will be like the following.

class Exception
  def detailed_message
    lines = message.lines.map { "\e[1m" + _1.chomp + "\e[m" }
    lines[0] << " (#{ self.class.inspect })"
    lines.join("\n")
  end

  def full_message
    # frame is a string like "file.rb:lineno:in `method_name'"
    head_frame, *tail_frames = backtrace
    "#{ head_frame }: #{ detailed_message }\n" + tail_frames.map { "\tfrom " + _1 }.join("\n")
  end
end

class MyError < StandardError
  def detailed_message
    # In general, user-defined detailed_message should prepend the result of "super"
    "#{ super }\naddtional message"
  end
end

Note that the "addtional message" part of MyError#detailed_message will not be automatically highlighted.

[Dan0042 (Daniel DeLorme)]

Thank you, it's much clearer now.

Follow-up question: Should #detailed_message include the class of the error
a) always (unlike #message currently)
b) only if highlight keyword is true
c) based on some other keyword (like in #note-6)
?

nitpick: "\e[0m" or "\e[m" is for full reset; imho it would be better to use "\e[22m" for bold-off (and "\e[24m" for underline-off). It allows things like adding color: "\e[31m" + err.detailed_message(highlight:true) + "\e[39m".

[mame (Yusuke Endoh)]

I have created another ticket to summarize the final spec of this proposal: #18564.

[mame (Yusuke Endoh)]

@Dan0042 (Daniel DeLorme) I think your last comment is a different topic. I'd like to keep one proposal as small as I can.

[Dan0042 (Daniel DeLorme)]

My concern was that Sentry will now use #detailed_message instead of #message, but it will now include the exception class, unlike before. But if Sentry is ok with that there's no problem.

[mame (Yusuke Endoh)]

Dan0042 (Daniel DeLorme) wrote in #note-22:

My concern was that Sentry will now use #detailed_message instead of #message, but it will now include the exception class, unlike before. But if Sentry is ok with that there's no problem.

Ah, that's a good point. @matz (Yukihiro Matsumoto) what do you think?

[st0012 (Stan Lo)]

My concern was that Sentry will now use #detailed_message instead of #message, but it will now include the exception class, unlike before. But if Sentry is ok with that there's no problem.

I think it'll be fine but I'm not sure why the class name needs to be added as well?

And just for confirmation: so if I want to preserve the content of Ruby 3.1's exception.message (with additional information), I'll need to use exception.detailed_message(highlight: false) (without ANSI sequences)?

[mame (Yusuke Endoh)]

Thank you for your comment!

st0012 (Stan Lo) wrote in #note-24:

I think it'll be fine but I'm not sure why the class name needs to be added as well?

The current error printer inserts the exception class name.

$ ruby -e 'raise "foo"'
-e:1:in `<main>': foo (RuntimeError)

This part (RuntimerError) is emphasized by an underline, so the method responsible for highlight should insert it. Now Exception#detailed_message has a responsibility to highlight the message, so it need to insert the class name.

$ ./miniruby -e 'begin; raise "foo"; rescue; p $!.detailed_message; end'
"foo (RuntimeError)"

$ ./miniruby -e 'begin; raise "foo"; rescue; p $!.detailed_message(highlight: true); end'
"\e[1mfoo (\e[1;4mRuntimeError\e[m\e[1m)\e[m"

But I agree that this is not a big deal.

And just for confirmation: so if I want to preserve the content of Ruby 3.1's exception.message (with additional information), I'll need to use exception.detailed_message(highlight: false) (without ANSI sequences)?

Yes, you are right. More precisely, exception.respond_to?(:detailed_message) ? exception.detailed_message : exception.message would work. Note that highlight keyword is false by default.

If a feature to remove a class name is needed, maybe we need to write exception.detailed_message(exception_class_name: false) or something.

[Dan0042 (Daniel DeLorme)]

mame (Yusuke Endoh) wrote in #note-25:

But I agree that this is not a big deal.

Exactly. At this point it's kind of a bikeshed, it's just that according to #note-1 Sentry displays the exception class above the message, so it would be displayed twice:

NoMethodError
undefined method `[]' for nil:NilClass (NoMethodError)

So if @st0012 (Stan Lo) wants to parse out the "(NoMethodError)", might as well not show it by default, and use detailed_message(exception_class_name: true) in #full_message (the only place it's needed).

[st0012 (Stan Lo)]

Now Exception#detailed_message has a responsibility to highlight the message, so it need to insert the class name

I see. Thanks for the explanation!

maybe we need to write exception.detailed_message(exception_class_name: false) or something.
So if st0012 (Stan Lo) wants to parse out the "(NoMethodError)", might as well not show it by default, and use detailed_message(exception_class_name: true) in #full_message (the only place it's needed).

I don't think adding another option just for that is necessary. I'll first display the detailed_message and if we receive any complain about the repeated class name, I'll remove it in the SDK.

[mame (Yusuke Endoh)]

I talked with @matz (Yukihiro Matsumoto). He decided to go ahead with it as is, as it is not a serious problem.

If you want an exact compatibility with the traditional Exception#message, please remove a class name from detailed_message.

exc = RuntimeError.new("foo\nbar\nbaz")
p exc.detailed_message                                    #=> "foo (RuntimeError)\nbar\nbaz"
p exc.detailed_message.sub(/\A(.*?)(?: \(\w+\))/) { $1 }  #=> "foo\nbar\nbaz"

We may consider adding a dedicated keyword argument for it if there are many requests.

[mame (Yusuke Endoh)]

The following PR adds a guideline about usable escape sequences in Exception#detailed_message.

https://github.com/ruby/ruby/pull/6119

Background:

An error message is primarily rendered in a terminal emulator, but is also shown in a browser by converting it to a HTML fragment, such as by Rack. However, the conversion would be unreasonably difficult if the message includes any escape sequence (such as cursor move or screen clear).

The safe and consertive way is to use highlight: false, but @ioquatix (Samuel Williams) suggested that we also want a way to convert the text attributes to HTML. (https://github.com/rack/rack/pull/1925#issuecomment-1179470336)
I talked about the issue with @matz (Yukihiro Matsumoto), and we agreed to document a guideline for the use of escape sequences in Exception#detailed_message:

• Use only widely-supported escape sequences: bold, underline, and basic eight foreground colors (except white and black).
• Make the message readable if all escape sequences are ignored.

I think these limitations would make conversion to HTML easier.

[mame (Yusuke Endoh)]

• Status changed from Open to Closed

I have already merged the PR. Closing.