Redesign for displaying exceptions

[alt-romes]

CLC Proposal for Exception Messages

Recently, https://github.com/haskell/core-libraries-committee/issues/231 and https://github.com/haskell/core-libraries-committee/issues/261, two proposals regarding exceptions and backtraces, have been accepted and implemented. These proposals display new information on thrown exceptions: type, module, and package of the exception that was thrown.

However, I believe the formatting/layout of this new information, together with the changes added to the header of the exception message, cause the exception message as a whole to be confusing, unhelpful, and noisy -- undoing the benefits of adding this information.

Example

main = error "this list should never be empty"

Compiled with GHC head (with all implemented proposals), the program will output the following:

Main: Exception:

this list should never be empty
CallStack (from HasCallStack):
  error, called at Main.hs:1:8 in main:Main

Package: ghc-internal
Module: GHC.Internal.Exception
Type: ErrorCall

HasCallStack backtrace:
  collectBacktraces, called at libraries/ghc-internal/src/GHC/Internal/Exception.hs:92:13 in ghc-internal:GHC.Internal.Exception
  toExceptionWithBacktrace, called at libraries/ghc-internal/src/GHC/Internal/Exception.hs:128:3 in ghc-internal:GHC.Internal.Exception

Diagnosis

I think this message is problematic

• It's hard to relate any of the lines Package: ghc-internal, Module: GHC.Internal.Exception, or Type: ErrorCall to the program that we wrote. What does it mean?!
  • Unlike the example in the accepted proposals BlockedIndefinitelyOnMVar, ErrorCall is a generic name of a much more common exception type which to the user likely reads as something internal leaking -- it's hard to connect it to the type of the exception that is being thrown.
  • This information is given entire paragraphs below the line saying Exception -- it's hard to link these two bits. It relates more easily with the HasCallStack backtrace below which at least also refers ghc-internal.
  • Using three separate lines wastes screen space in a message that is ideally compact, but, perhaps more importantly, it makes the three lines seem unrelated
    • The whitespace around every paragraph only seems needed to compensate for that to try and group them.
  • Spelling out Package, Module, and Type is also redundant since this information is only for developers who would just as well understand the triple <package>:<module>.<type>.
• The leading Main: Exception: is also confusingly short when compared with how much whitespace and other blocks of text there are -- it's just hanging up there instead of relating to the exception type!

This confusion is exacerbated when exceptions are re-thrown according to https://github.com/haskell/core-libraries-committee/issues/202 (example below)

Proposal

Considering this, I believe the solution is straightforward and both makes the message prettier and responds to my critiques above. Here are the key points:

• The header of the message should relate the exception that was thrown, and its type, all in the same line. The message can be inline or in the next line.
  • Using the compact full representation of the type: <package>:<module>.<type>
  • Connect this type to it being the type of the exception that was thrown (not just some random internal type -- recall, internal exceptions type are seldom known to users, like ErrorCall, and would be perceived as such)
  • Being more compact and relating the error type to its message is what other languages do
    • Java:

      Exception in thread "main" java.lang.NullPointerException: Cannot invoke "String.length()" because "<local1>" is null
         at GFG.main(J.java:13)
      

    • Python

      Traceback (most recent call last):
        File "/private/var/folders/tv/35hlch6s3y15hfvndc71l6d40000gn/T/tmp.tUIx5JL667/x.py", line 4, in <module>
          f(None)
        File "/private/var/folders/tv/35hlch6s3y15hfvndc71l6d40000gn/T/tmp.tUIx5JL667/x.py", line 2, in f
          x[0]
          ~^^^
      TypeError: 'NoneType' object is not subscriptable
      

So here's a suggestion for what the message should instead look like:

T24807: Uncaught exception ghc-internal:GHC.Internal.Exception.ErrorCall:

this list should never be empty
CallStack (from HasCallStack):
  error, called at T24807.hs:1:8 in main:Main

HasCallStack backtrace:
  collectBacktraces, called at libraries/ghc-internal/src/GHC/Internal/Exception.hs:92:13 in ghc-internal:GHC.Internal.Exception
  toExceptionWithBacktrace, called at libraries/ghc-internal/src/GHC/Internal/Exception.hs:127:5 in ghc-internal:GHC.Internal.Exception

---

Suggestion applied to example from the original proposal

With the original proposal:

λ. ./Main
Main: Exception:

thread blocked indefinitely in an MVar operation

Package: ghc-internal
Module: GHC.Internal.IO.Exception
Type: BlockedIndefinitelyOnMVar

HasCallStack backtrace:
  collectBacktraces, called at libraries/ghc-internal/src/GHC/Internal/Exception.hs:92:13 in ghc-internal:GHC.Internal.Exception
  toExceptionWithBacktrace, called at libraries/ghc-internal/src/GHC/Internal/IO.hs:260:11 in ghc-internal:GHC.Internal.IO
  throwIO, called at Main.hs:33:16 in main:Main

With this proposal:

λ. ./Main
Main: Uncaught exception ghc-internal:GHC.Internal.IO.Exception.BlockedIndefinitelyOnMVar:

thread blocked indefinitely in an MVar operation

HasCallStack backtrace:
  collectBacktraces, called at libraries/ghc-internal/src/GHC/Internal/Exception.hs:92:13 in ghc-internal:GHC.Internal.Exception
  toExceptionWithBacktrace, called at libraries/ghc-internal/src/GHC/Internal/IO.hs:260:11 in ghc-internal:GHC.Internal.IO
  throwIO, called at Main.hs:33:16 in main:Main

---

Interaction with rethrown exceptions

When re-thrown exceptions are thrown into the mix it's worse because these large blocks appear for every "while handling":

cgrun025: Exception:

hello, error
CallStack (from HasCallStack):
  error, called at cgrun025.hs:25:75 in main:Main

Package: ghc-internal
Module: GHC.Internal.Exception
Type: ErrorCall

While handling __WURBLE__: getEnv: does not exist (no environment variable)

  Package: ghc-internal
  Module: GHC.Internal.IO.Exception
  Type: IOException
  
  HasCallStack backtrace:
    collectBacktraces, called at libraries/ghc-internal/src/GHC/Internal/Exception.hs:92:13 in ghc-internal:GHC.Internal.Exception
    toExceptionWithBacktrace, called at libraries/ghc-internal/src/GHC/Internal/IO.hs:284:11 in ghc-internal:GHC.Internal.IO
    throwIO, called at libraries/ghc-internal/src/GHC/Internal/IO/Exception.hs:315:19 in ghc-internal:GHC.Internal.IO.Exception
    ioException, called at libraries/ghc-internal/src/GHC/Internal/System/Environment.hs:192:26 in ghc-internal:GHC.Internal.System.Environment

HasCallStack backtrace:
  collectBacktraces, called at libraries/ghc-internal/src/GHC/Internal/Exception.hs:92:13 in ghc-internal:GHC.Internal.Exception
  toExceptionWithBacktrace, called at libraries/ghc-internal/src/GHC/Internal/Exception.hs:128:3 in ghc-internal:GHC.Internal.Exception

After the proposal, the re-thrown exception example would become:

cgrun025: Uncaught exception ghc-internal:GHC.Internal.Exception.ErrorCall:

hello, error
CallStack (from HasCallStack):
  error, called at cgrun025.hs:25:75 in main:Main

While handling ghc-internal:GHC.Internal.IO.Exception.IOException:

    __WURBLE__: getEnv: does not exist (no environment variable)

    HasCallStack backtrace:
      collectBacktraces, called at libraries/ghc-internal/src/GHC/Internal/Exception.hs:92:13 in ghc-internal:GHC.Internal.Exception
      toExceptionWithBacktrace, called at libraries/ghc-internal/src/GHC/Internal/IO.hs:284:11 in ghc-internal:GHC.Internal.IO
      throwIO, called at libraries/ghc-internal/src/GHC/Internal/IO/Exception.hs:315:19 in ghc-internal:GHC.Internal.IO.Exception
      ioException, called at libraries/ghc-internal/src/GHC/Internal/System/Environment.hs:192:26 in ghc-internal:GHC.Internal.System.Environment

HasCallStack backtrace:
  collectBacktraces, called at libraries/ghc-internal/src/GHC/Internal/Exception.hs:92:13 in ghc-internal:GHC.Internal.Exception
  toExceptionWithBacktrace, called at libraries/ghc-internal/src/GHC/Internal/Exception.hs:127:5 in ghc-internal:GHC.Internal.Exception
  error, called at cgrun025.hs:25:75 in main:Main

Impact

The originally accepted proposal is yet unreleased. It will become available in 9.12 unless this proposal is accepted in the meantime.

Testsuites logging the stderr of haskell programs could suffer from changes in the exception messages, even if minor, so it would be ideal, if agreed upon, to get this change in before 9.12 is released.

Implementation

Done in https://gitlab.haskell.org/ghc/ghc/-/merge_requests/13280. Note the differences in the testsuite accepted tests. Net reduction in 600 lines in exceptions printing.

[alt-romes]

I'm going to ping those involved in the original message design to initiate discussion: @tomjaguarpaw @tbidne @adamgundry @parsonsmatt

[tomjaguarpaw]

That seems fine by me.

[tbidne]

The rethrown example is compelling. Agreed that this is an improvement. To keep discussion centralized, I'll respond to @bgamari's questions on the original MR here:

Exception type data added to SomeException's displayException, rather than the handler.

it is not clear to me why this implementation approach was taken. This doesn't appear to be consistent with what was proposed in the CLC proposal and means that users have no way of opting out of this output. Perhaps you can shed some light on the reasoning here?

1. That decision was based on the discussion in the 2nd issue. The (light) consensus was that changing the format from:

   <msg>
   
   <backtraces>
   
   <type>
   

   to

   <msg>
   
   <type>
   
   <backtraces>
   

   improved clarity, and doing so required moving the type info to the instance, since that is where msg and backtraces were handled.

   This PR changes it to

   <type>
   
   <msg>
   
   <backtraces>
   

   so AFAICT it would be possible now to move the <type> to the handler instead of the instance. But perhaps this is less important now since the message is far more compact. Moreover, given that we can have multiple exceptions in play with WhileHandling, I think it makes sense to leave it on the instance since that would ensure each exception receives the type info.

2. As far as "opting out", that would of course be possible with a global mechanism like setBacktraceMechanismState, but it's more complexity that no one asked for at the time, and hopefully it's less important with the proposed concise output.

[adamgundry]

Thanks for taking a careful look at the quality of exception messages @alt-romes, I think this is very valuable.

On reflection, it does seem to me problematic for displayException to show anything other than the user-friendly message, because it is the normal mechanism for developers to use when rendering exceptions for end users, and they may do so in contexts where backtraces/types are not desired. So I think displayException for SomeException should give only the displayException of the underlying exception, but we could have another function that displays the full glorious details, expose that function to users and call it in the default handler. I regret not picking up on this in the previous discussion.

[alt-romes]

I'm currently making an effort to get exceptions as a whole in a better state before the 9.12 release.

As I've come to realize when implementing WhileHandling/re-throwing of exceptions, there are a few other minor bugs and improvements hanging around that I believe we should act on swiftly, but which do require GHC proposals.

@adamgundry suggested I try to keep the discussion within a single CLC proposal for all the incremental improvements to the exceptions. Accordingly, I've updated this proposal with additional part 2 to 5.

Thank you for commenting on the proposal for better exception messages already. The part 1 of the exception (which you originally read) is kept unchanged. I would appreciate it if you could comment on the new parts of the proposal for improving the exceptions as a whole.

I've implemented these suggestions in this MR

[tomjaguarpaw]

I don't use exceptions myself[^1] so I'll defer to exception users regarding what they need. However, I am surprised about this:

[displayException] is the normal mechanism for developers to use when rendering exceptions for end users

I always understood than an uncaught exception was considered hard abort, if not programmer error: something has gone unexpectedly wrong. In such cases one typically wants as much information as possible dumped to the screen to help with debugging or bug reporting. That seems to me approximately what displayException is currently providing, and I thought that was good. But am I now to understand that displayException is used for formatting error messages intended to be read by end users? If so, should it really be? Why not instead catch the exception and format a nice message of your own choosing?

[^1]: At least not in the standard throw/throwIO and then catch in IO (or not) – instead I use well-scoped exceptions, i.e. Either/MTL/Bluefin etc.

[parsonsmatt]

If so, should it really be? Why not instead catch the exception and format a nice message of your own choosing?

The dynamic nature of SomeException means that you can do this, if you explicitly list out the types you expect to catch. And if you missed one, you need a different message.

foo `catches`
  [ Handler (\KnownException -> print0)
  , Handler (\(OtherKnownException a) -> print1 a)
  , Handler (\(SomeException unknown) -> putStrLn $ displayException unknown)
   ]

In that fallback case, all we "know" is (forall e. Exception e => e), which means we only have the methods on Exception and Show to do anything useful with it.

[alt-romes]

You implement displayException as part of instance Exception that describes how to print an exception. Then, when an exception is not caught, it will be handled by uncaughtExceptionHandler (which can be set with setUncaughtExceptionHandler).

By default, the uncaught excp. handler prints out as much information as possible, as you said. At the moment, this includes the type of the exception and the callstack.

However, if you have business-logic exceptions being thrown in your program, you may want to override the exception handler to print out the exception cleanly to the user in a way custom to your program.

The problem is the handler receives the exception wrapped in SomeException. You'd want to have displayException (SomeException e) = displayException e so that your definition of displayException is the one printed out.

But at the moment it's not. displayException @SomeException will also display type and backtrace.

The proposal is to define displayExceptionWithInfo in terms of displayException, but which wraps it with the type and callstack. Then, the default handler uses displayExceptionWithInfo which will print all the details. However, you can override the handler and call displayException -- opting out of the extra details

But am I now to understand that displayException is used for formatting error messages intended to be read by end users? If so, should it really be? Why not instead catch the exception and format a nice message of your own choosing?

You could have a top-level handler that tries to catch all exceptions relevant to the user and manually prints them differently. But I also think it is clean to have proper exceptions relevant to the user simply being surfaced to the top level and handled by the exception handler.

[tomjaguarpaw]

By default, the uncaught excp. handler prints out as much information as possible, as you said. At the moment, this includes the type of the exception and the callstack.

Right, so far I understand.

However, if you have business-logic exceptions being thrown in your program, you may want to override the exception handler to print out the exception cleanly to the user in a way custom to your program.

But now I don't understand. Why override the exception handler? Why not catch the exception, and then do whatever you like with it? I feel I must be missing something that's very clear to everyone else.

You could have a top-level handler that tries to catch all exceptions relevant to the user and manually prints them differently. But I also think it is clean to have proper exceptions relevant to the user simply being surfaced to the top level and handled by the exception handler.

That doesn't seem clean to me at all. Since I don't use exceptions like this I may be missing something that seems obvious to others, but letting unhandled exceptions get to the default, top-level, handler, and expecting them to be printed nicely for users, doesn't seem, to me, like something we should be supporting.

[alt-romes]

That doesn't seem clean to me at all. Since I don't use exceptions like this I may be missing something that seems obvious to others, but letting unhandled exceptions get to the default, top-level, handler, and expecting them to be printed nicely for users, doesn't seem, to me, like something we should be supporting.

(Even if there is disagreement on the merits of this style of programming, the proposed solution enables strictly more styles while keeping everything the same for any user who doesn't touch the default uncaught exception handler).

I think SomeException should ideally be "transparent", since it's just an existential wrapper over an Exception. And with the proposal we do indeed get:

displayException (SomeException e) = displayException e

While keeping exactly the same all of the extra information when exceptions are uncaught and reported.

[adamgundry]

But now I don't understand. Why override the exception handler? Why not catch the exception, and then do whatever you like with it?

One answer: the exception may be thrown in library code I don't control, on a thread forked by the library. There's no way AFAIK to add an exception handler in such a case. Obviously this is a bad/buggy library, but such libraries exist! Thus the application programmer needs to be able to specify a top-level exception handler to report the exception using whatever logging mechanism is appropriate for their application.

For (a not entirely hypothetical) example, I'm writing a concurrent web server with a multitude of request-handling and background worker threads. I can add a top-level handler so that if any of these threads dies with an exception, the exception gets serialised to JSON and sent to a cloud logging service. With @alt-romes's proposal, in the logging code I can just call displayException to get the human-friendly description to populate one field of the log object (plus populate other fields as needed e.g. with types/backtraces). Whereas with the status quo, I have to be careful to call displayException not at type SomeException but only on the inner exception object (otherwise I'll end up accidentally logging the backtraces in the "description" field).

[tomjaguarpaw]

@alt-romes

(Even if there is disagreement on the merits of this style of programming, the proposed solution enables strictly more styles while keeping everything the same for any user who doesn't touch the default uncaught exception handler).

That sounds great! But I don't understand. This proposal suggests a change to the behaviour of the default uncaught exception handler, doesn't it?

I think SomeException should ideally be "transparent", since it's just an existential wrapper over an Exception. And with the proposal we do indeed get:

displayException (SomeException e) = displayException e

This seems pretty reasonable to me, because the thing that is thrown an caught is always a SomeException, right? And then we only check what instance of Exception it is by casting it with fromException. So it seems to me to never make sense to show the top-level SomeException wrapper. But I'm yet again confused: why was the wrapper ever shown? Is this a long-standing issue?

(To be honest I'm beginning to think that it would be easier to deal with this proposal in separate parts. Some parts, like this one, seem like they would be quite quick to resolve.)

@adamgundry

the exception may be thrown in library code I don't control, on a thread forked by the library. There's no way AFAIK to add an exception handler in such a case

I don't follow. Could you elaborate? Maybe with an example?

[alt-romes]

The change to the behavior of the default handler together with the change to displayException @SomeException keeps the program output on an uncaught exception exactly the same.

It's about moving the part adding the call stack to the handler from the instance, achieving all our goals without changing the default output.

[tomjaguarpaw]

why was the wrapper ever shown?

Ah, looks like the wrapper was never shown, but the difference is that recently we added more stuff to the displayException of SomeException.

https://gitlab.haskell.org/ghc/ghc/-/commit/ff158fcd84f080a3450e09320819ef1d950a2c2d?page=2#03cfee2d5ff1e2f3ab92a8cb79be769da8656d93

Is the idea that should "more stuff" shouldn't be in the displayException of SomeException, but should rather be added by a handler? If so I think things are becoming clearer for me. Could you confirm?

The change to the behavior of the default handler together with the change to displayException @SomeException keeps the program output on an uncaught exception exactly the same.

Great, I must have missed that. I think that's worth calling out prominently and explicitly in the proposal.

[alt-romes]

Is the idea that should "more stuff" shouldn't be in the displayException of SomeException, but should rather be added by a handler? If so I think things are becoming clearer for me. Could you confirm?

Yes, that's exactly right.

[michaelpj]

This looks good to me. I only have one small comment on Part 1 and the interaction with rethrown exceptions. It would be nice if the WhileHandling annotations were the last thing printed. As it is, you can see in your example that you have a substantial block for the WhileHandling exception, and then you have to "pop" your reading context back out to the top-level exception to read the backtraces. That is, I think it would be nice if we could get the following formatting for you example:

cgrun025: Uncaught exception ghc-internal:GHC.Internal.Exception.ErrorCall:

hello, error
CallStack (from HasCallStack):
  error, called at cgrun025.hs:25:75 in main:Main

HasCallStack backtrace:
  collectBacktraces, called at libraries/ghc-internal/src/GHC/Internal/Exception.hs:92:13 in ghc-internal:GHC.Internal.Exception
  toExceptionWithBacktrace, called at libraries/ghc-internal/src/GHC/Internal/Exception.hs:127:5 in ghc-internal:GHC.Internal.Exception
  error, called at cgrun025.hs:25:75 in main:Main

While handling ghc-internal:GHC.Internal.IO.Exception.IOException:

    __WURBLE__: getEnv: does not exist (no environment variable)

    HasCallStack backtrace:
      collectBacktraces, called at libraries/ghc-internal/src/GHC/Internal/Exception.hs:92:13 in ghc-internal:GHC.Internal.Exception
      toExceptionWithBacktrace, called at libraries/ghc-internal/src/GHC/Internal/IO.hs:284:11 in ghc-internal:GHC.Internal.IO
      throwIO, called at libraries/ghc-internal/src/GHC/Internal/IO/Exception.hs:315:19 in ghc-internal:GHC.Internal.IO.Exception
      ioException, called at libraries/ghc-internal/src/GHC/Internal/System/Environment.hs:192:26 in ghc-internal:GHC.Internal.System.Environment

[tomjaguarpaw]

Is the idea that should "more stuff" shouldn't be in the displayException of SomeException, but should rather be added by a handler? If so I think things are becoming clearer for me. Could you confirm?

Yes, that's exactly right.

OK, great. I see this is actually spelled out in the following sentence:

Updating the default handler and the instance guarantees that exceptions by default still get printed with the type and backtrace, as they currently do.

but I think it would be worth calling out more explicitly that this change doesn't change the behaviour of the default handler.

I propose that ErrorCall stops propagating a callstack manually

Currently,

data ErrorCall = ErrorCallWithLocation String String

(see https://www.stackage.org/haddock/lts-22.35/base-4.18.2.1/Control-Exception.html#t:ErrorCall) Are you suggesting changing this constructor? Perhaps changing it back to just ErrorCall String? (There is a already a pattern synonym ErrorCall.)

Since I'm already changing this code, let's finally remove errorWithStackTrace, which has been deprecated since GHC 8.0 (2015), i.e. for 9 years.

I'm against this. errorWithStackTrace does not require any ongoing support or maintenance. There is a chance (albeit probably tiny) that removing it could break someone's code and make them hate Haskell, and I don't see why we should take that risk. I appreciate the desire to "tidy up", so as a compromise I would accept the following, which at the point of use tells the user exactly what they have to do to fix their code:

errorWithStackTrace ::
  Unsatisfiable
    (Text "'errorWithStackTrace' no longer exists. Use 'error' instead.") =>
  String ->
  a
errorWithStackTrace = unsatisfiable

test28.hs:12:7: error: [GHC-22250]
    • 'errorWithStackTrace' no longer exists. Use 'error' instead.
    • In the expression: errorWithStackTrace "error message"
      In an equation for ‘foo’: foo = errorWithStackTrace "error message"
   |
12 | foo = errorWithStackTrace "error message"
   |       ^^^^^^^^^^^^^^^^^^^

[tomjaguarpaw]

(And to be clear, I'm +1 on the rest of the proposal.)

[alt-romes]

Currently,

data ErrorCall = ErrorCallWithLocation String String

(see https://www.stackage.org/haddock/lts-22.35/base-4.18.2.1/Control-Exception.html#t:ErrorCall) Are you suggesting changing this constructor? Perhaps changing it back to just ErrorCall String? (There is an already a pattern synonym ErrorCall.)

That's right. The commit is https://gitlab.haskell.org/ghc/ghc/-/merge_requests/13301/diffs?commit_id=c0a2cb447c3fec71edc6653fe7f26619b751ce5f.

We're deleting the ErrorCall pattern synonym, deleting the ErrorCallWithLocation constructor, and instead making ErrorCall :: String -> ErrorCall the only constructor of ErrorCall.

This means that nothing changes for those already using the pattern synonym.

Since I'm already changing this code, let's finally remove errorWithStackTrace, which has been deprecated since GHC 8.0 (2015), i.e. for 9 years.

I'm against this. errorWithStackTrace does not require any ongoing support or maintenance. There is a chance (albeit probably tiny) that removing it could break someone's code and make them hate Haskell, and I don't see why we should take that risk. I appreciate the desire to "tidy up", so as a compromise I would accept the following, which at the point of use tells the user exactly what they have to do to fix their code:

In that case I've amended the proposal to not touch errorWithStackTrace. I had no strong motivation to do this other than clean up. FWIW, the 9-year-old deprecation message already refers error as the supposed substitute:

-- | Like the function 'error', but appends a stack trace to the error
-- message if one is available.
--
-- @since base-4.7.0.0
{-# DEPRECATED errorWithStackTrace "'error' appends the call stack now" #-}
  -- DEPRECATED in 8.0.1
errorWithStackTrace :: String -> a
errorWithStackTrace x = unsafeDupablePerformIO $ throwIO (ErrorCall x)

[alt-romes]

@michaelpj I think your suggestion would look better, but it's not clear to me how to achieve this with the exception annotations.

However, for the WhileHandling display I intend to use vertical bars | to better distinguish nested levels:

ghc: Uncaught exception ghc-9.11-inplace:GHC.Utils.Panic.GhcException:

default output name would overwrite the input file; must specify -o explicitly
Usage: For basic information, try the `--help' option.

While handling ghc-9.11-inplace:GHC.Utils.Panic.GhcException:
  |
  | default output name would overwrite the input file; must specify -o explicitly
  | Usage: For basic information, try the `--help' option.
  |
  | While handling ghc-9.11-inplace:GHC.Utils.Panic.GhcException:
  |   |
  |   | default output name would overwrite the input file; must specify -o explicitly
  |   | Usage: For basic information, try the `--help' option.
  |   |
  |   | While handling ghc-9.11-inplace:GHC.Utils.Panic.GhcException:
  |   |   |
  |   |   | default output name would overwrite the input file; must specify -o explicitly
  |   |   | Usage: For basic information, try the `--help' option.
  |   |   |
  |   |   | HasCallStack backtrace:
  |   |   |   collectBacktraces, called at libraries/ghc-internal/src/GHC/Internal/Exception.hs:91:13 in ghc-internal:GHC.Internal.Exception
  |   |   |   toExceptionWithBacktrace, called at libraries/ghc-internal/src/GHC/Internal/Exception.hs:83:32 in ghc-internal:GHC.Internal.Exception
  |   |   |   throw, called at compiler/GHC/Utils/Panic.hs:180:21 in ghc-9.11-inplace:GHC.Utils.Panic
  |   |
  |   | HasCallStack backtrace:
  |   |   bracket_, called at libraries/semaphore-compat/src/System/Semaphore.hs:320:23 in semaphore-compat-1.0.0-inplace:System.Semaphore
  |
  | HasCallStack backtrace:
  |   throwIO, called at libraries/exceptions/src/Control/Monad/Catch.hs:371:12 in exceptions-0.10.7-inplace:Control.Monad.Catch
  |   throwM, called at libraries/exceptions/src/Control/Monad/Catch.hs:860:84 in exceptions-0.10.7-inplace:Control.Monad.Catch
  |   onException, called at compiler/GHC/Driver/Make.hs:2986:23 in ghc-9.11-inplace:GHC.Driver.Make

HasCallStack backtrace:
  bracket, called at compiler/GHC/Driver/Make.hs:2953:3 in ghc-9.11-inplace:GHC.Driver.Make

This makes it more-OK that the call stack appears at the bottom only. Also, the Haskell call stack is read from bottom-to-top, so that's also aligned with the outermost exception showing at the bottom.

[alt-romes]

To add a datapoint, today we got a user bug report about the duplicate call stacks part of this proposal (Part 3): https://gitlab.haskell.org/ghc/ghc/-/issues/25311

[ulysses4ever]

Hey @alt-romes . Following your invitation to comment on the proposal, I have one suggestion regarding presentation. In particular, the example in the top post has the following formatting on the "Proposal" part:

this list should never be empty
CallStack (from HasCallStack):
  error, called at T24807.hs:1:8 in main:Main

I think the message and the call stack are two different paragraphs in the text, semantically. Therefore, I'd expect them to be separated by an empty line:

this list should never be empty

CallStack (from HasCallStack):
  error, called at T24807.hs:1:8 in main:Main

Bonus points would gain an extra indentation for the message because morally it's a quote. The reason I think of it as a quote is because I read the whole output as authored by the RTS, whereas the error message was written by the programmer.

[alt-romes]

I think the message and the call stack are two different paragraphs in the text, semantically. Therefore, I'd expect them to be separated by an empty line:

this list should never be empty

CallStack (from HasCallStack):
  error, called at T24807.hs:1:8 in main:Main

the two lines being together is determined by the instance of Exception for ErrorCall, where its constructor ErrorCallWithLocation holds the error and the callstack. However, we're getting rid of the callstack from ErrorCall in favour of the exception context backtrace altogether as proposed in Part 3, since they were duplicate.

Therefore, there's no line to add since the ErrorCall callstack will just be gone.

As for the exception context callstack, there's already a line between it and the user message (example from part 3):

T24807: Uncaught exception ghc-internal:GHC.Internal.Exception.ErrorCall:

this list should never be empty

HasCallStack backtrace:
  collectBacktraces, called at libraries/ghc-internal/src/GHC/Internal/Exception.hs:92:13 in ghc-internal:GHC.Internal.Exception
  toExceptionWithBacktrace, called at libraries/ghc-internal/src/GHC/Internal/Exception.hs:127:5 in ghc-internal:GHC.Internal.Exception
  error, called at T24807.hs:1:8 in main:Main

Thanks!

[9999years]

Therefore, there's no line to add since the ErrorCall callstack will just be gone.

Sounds like we should add a blank line there when there's no ErrorCall callstack, then?

[alt-romes]

Sounds like we should add a blank line there when there's no ErrorCall callstack, then?

After removing the ErrorCall callstack in favour of the exception-propagation callstack, we have:

T24807: Uncaught exception ghc-internal:GHC.Internal.Exception.ErrorCall:

this list should never be empty

HasCallStack backtrace:
  collectBacktraces, called at libraries/ghc-internal/src/GHC/Internal/Exception.hs:92:13 in ghc-internal:GHC.Internal.Exception
  toExceptionWithBacktrace, called at libraries/ghc-internal/src/GHC/Internal/Exception.hs:127:5 in ghc-internal:GHC.Internal.Exception
  error, called at T24807.hs:1:8 in main:Main

which eventually becomes

T24807: Uncaught exception ghc-internal:GHC.Internal.Exception.ErrorCall:

this list should never be empty

HasCallStack backtrace:
  error, called at T24807.hs:1:8 in main:Main

by freezing the callstack of error (part 4). As far as I can tell, there shouldn't be additional newlines in this message

[ulysses4ever]

@alt-romes any chance "this list should never be empty" can be indented two or four spaces so that it looks like a quote?

[alt-romes]

It could. What do you suggest would happen to the "WhileHandling" messages? In any case, the behavior for whileHandling could differ from the top level handler message.

Of course, if this is not unambiguous I won't, since I don't intend to add any friction to this proposal.

[Bodigrim]

I'm personally sympathetic with Part 1. However, in my role I must make a point of order.

There was time and place to raise concerns about #261. The CLC proposal was open for 3+ months. The corresponding MR was available for review as well. We cannot bikeshed this ad infinitum every time someone disagrees on aesthetic. Is there a material change validating re-evaluation? What would prevent someone to raise another proposal to change formatting in a week or two? How long shall we keep this proposal open to make sure that all interested parties had voiced their opinions?

[alt-romes]

We cannot bikeshed this ad infinitum every time someone disagrees on aesthetic. Is there a material change validating re-evaluation?

These changes are motivated by a holistic assessment of what exceptions as a whole have become in GHC, in light of all the recent proposals that have changed them. Unlike other MRs, this is not a standalone step in a direction, but rather a correction to exceptions as they stand whole. This could only have been done now, after the incremental ideas made their way in.

What would prevent someone to raise another proposal to change formatting in a week or two?

I don't think this is just about changing formatting (see above), but in any case, if they did, we would:

• Analyse the merits to the format change
• Against the drawbacks: changing it a third time, but this time it would be after a released version of the compiler with the message, which may break testsuites and confuse users (unlike this proposal, which should amend the exceptions messages before they are released with the current, previously unreleased, formatting).

How long shall we keep this proposal open to make sure that all interested parties had voiced their opinions?

I intend the implementation to be 100% finished next week, or the week after in the worst case scenario. 100% here means full CI pipeline finished, since the code implementation is essentially complete. The proposal is ideally voted on at that time.

[simonmichael]

Some related feedback from me in #GHC chat:

With ghc 9.10, readFile "nosuchfile" is printing a (useless) stack trace with the error, when compiled. Is it a known bug ? Is there an easy way to prevent it ?

I call it a bug because it should be possible (easy, default, as in past ghc versions) to keep stack traces out of user-visible output. And I (rudely) called it useless because it doesn't look like it'll ever provide useful information, though I could be wrong. Here's how it looked:

HasCallStack backtrace:
  collectBacktraces, called at libraries/ghc-internal/src/GHC/Internal/Exception.hs:92:13 in ghc-internal:GHC.Internal.Exception
  toExceptionWithBacktrace, called at libraries/ghc-internal/src/GHC/Internal/IO.hs:260:11 in ghc-internal:GHC.Internal.IO
  throwIO, called at libraries/ghc-internal/src/GHC/Internal/IO/Exception.hs:315:19 in ghc-internal:GHC.Internal.IO.Exception
  ioException, called at libraries/ghc-internal/src/GHC/Internal/IO/Exception.hs:319:20 in ghc-internal:GHC.Internal.IO.Exception