Crystal: Feature request: ENV var for showing full error trace

[paulcsmith]

The new error messages are a pleasure to use, but sometimes they do not display enough information.

The problem

We have the --error-trace flag which is nice but has a couple issues:

• Some people always want to see the full error trace. Including this flag every time is a pain
• People often run Crystal programs through other CLI programs (foreman, overmind, etc.) which would need to manually pass error-trace to the Crystal command or would require changing the Procfile just to get an error trace. It can be confusing when people try foreman --error-trace or lucky watch --error-trace and nothing changes.

Proposed solution - show full error trace with ENV var

Add an ENV var that can be used for enabling full error traces. This solves both issues

• Users can add the ENV var to their shell to always show traces if they want
• You can prepend the ENV var when running the program

I'm not sure about naming. Maybe CRYSTAL_ERROR_TRACE or FULL_CRYSTAL_ERROR_TRACE?

Implementation

• I'd suggest either 1 or true would be treated as truthy and would enable the full error trace
• I would also suggest showing the ENV var instead of the flag since it is works in more situations:

Current message:
https://github.com/crystal-lang/crystal/blob/f7c0e753663b4e851a2cd25adc8c3ba810d93532/src/compiler/crystal/semantic/exception.cr#L118

Suggested change:

Showing last frame. Set CRYSTAL_ERROR_TRACE=1 for full trace.

I'm not 100% sold on the exact phrasing but I think that shows the rough idea.

Unanswered questions?

Should we just remove the --error-trace flag and use the ENV var? I think it should stay, but open to other thoughts on it!

[r00ster91]

I think we should perhaps instead add a more general environment variable called something like CRYSTAL_FLAGS and then you can set CRYSTAL_FLAGS to --error-trace.
CRYSTAL_FLAGS would always automatically passed to the compiler. You could then of course also pass multiple flags.
The advantage of this is that you can then do this for more flags than just --error-trace because why only --error-trace? Maybe for other people it's a pain to always pass --stats or something.
Like this much more cases would be covered.

[j8r]

Agree with @r00ster91.
It will be useful when using shards build, because currently some compiler flags are missing.

[paulcsmith]

I totally agree @r00ster91. That sounds like an awesome idea. I still think we should change the error message somehow to make it clear since I've seen so many issues with this.

Maybe

Showing last frame. For full trace use --error trace or set CRYSTAL_FLAGS=--error-trace

[asterite]

In which cases the trimmed error message isn't enough? Maybe we should try to improve the existing short messages. Do you have examples?

[paulcsmith]

@asterite Here's a minimal example of the problem

https://play.crystal-lang.org/#/r/7xcz

# Imagine this is in a shard
# Also a problem in your own code, but especially problematic in shards because its harder to see source
module FrameworkCode
  extend self

  def public_api(should_be_an_int)
    private_api(should_be_an_int)
  end

  private def private_api(should_be_an_int : Int)
  end
end

# Call framework code from app code
FrameworkCode.public_api("string")

Which results in the message:

Showing last frame. Use --error-trace for full trace.

error in line 5
Error: no overload matches 'FrameworkCode.private_api' with type String

Overloads are:
 - FrameworkCode#private_api(should_be_an_int : Int)

So the error message is fine/correct, but you see the error is for private_api, but the real problem is that we passed a String to public_api. That is very hard to tell without a more complete trace. The other problem is that if this FrameworkCode was in a shard you would have a hard time figuring out what code you called that eventually called the private_api since you can't easily see it and you didn't write it.

Here's with the error trace:

In trace.cr:12:15

 12 | FrameworkCode.public_api("string")
                    ^---------
Error: instantiating 'FrameworkCode:Module#public_api(String)'


In trace.cr:5:5

 5 | private_api(should_be_an_int)
     ^----------
Error: no overload matches 'FrameworkCode.private_api' with type String

Overloads are:
 - FrameworkCode#private_api(should_be_an_int : Int)

This allows you to see that you called public_api and shows where you called it from.

Of course this could be resolved by adding a type restriction to the public API, but not everyone does this, and sometimes the problem is fairly deep in framework code so it is hard to tell what method you called that caused an error.

I think we could change the error messaging with full trace disabled to make this easier, but I think we should still make showing a full trace with an ENV var easier for cases like these

[asterite]

I understand that a long trace can be scary, but why not always show the full trace? You still get the last frame at the bottom and you can scroll up if you need more info.

(I can't remember why we removed the full trace, but many times so far I wanted to have the full trace, and having the full trace never bothered me and it always helped me)

[paulcsmith]

For new users (and experienced ones too) there was very little visual differentiation between the frames so it was hard to see find where the last one was. It's also super helpful to keep macros more concise. Sometimes there can be hundreds of lines so that made it very difficult to find the problem.

With that said I think we could find a really good middle ground. I'm planning to open an issue that I think will help a lot with the "error-trace" disabled

[paulcsmith]

8410 is my suggestion for making the default errors concise and super readable (IMO). I think it'd still be useful to add the ability to toggle this setting (and other flags) with ENV vars, but I think #8410 is even more important and would make compile time errors so nice

[asterite]

Oh, that's a great idea! Show the full error for the last frame, but show just the call and location of the previous traces to better figure out the data flow that led to the error.

[asterite]

I was going to say that on second though, if instead of passing "string" you pass a variable name with type String, you'll see this error:

- FrameworkCode.public_api(should_be_an_int)
  trace.cr:12

In trace.cr:5:5

 5 | private_api(should_be_an_int)
     ^----------
Error: no overload matches 'FrameworkCode.private_api' with type String

Overloads are:
 - FrameworkCode#private_api(should_be_an_int : Int)

I thought that was not going to be useful, but it is because in this case you happen to be forwarding a variable.

But in more complex scenarios the trace, with types removed, might not provide enough information to figure out what went wrong. But we'll see.