provide source file context for exception message

[maxandersen]

Description

saw https://github.com/laech/java-stacksrc today

turns:

org.opentest4j.AssertionFailedError: expected: <hello> but was: <hi>
	at org.junit...
	at com.example.MyTest.hello(MyTest.java:24)
	...

into

org.opentest4j.AssertionFailedError: expected: <hello> but was: <hi>
	at org.junit...
	at com.example.MyTest.hello(MyTest.java:24)

	   22    @Test
	   23    void compareStrings() {
	-> 24      assertEquals("hello", "hi");
	   25    }

	...

would be awesome to hook into quarkus dev and have that printed in logs and on error page with option to turn on/off.

Implementation ideas

The functionality is dead simple: https://github.com/laech/java-stacksrc/tree/main/core/src/main/java/nz/lae/stacksrc

3 files that just decorate any java stacktrace.

could even consider to just copy it in to avoid extra dependency.

hook in to JBoss logging of errors and the error page.

[maxandersen]

here is how it look if you just do it directly:

[maxandersen]

even better when more levels.

[geoand]

The easiest thing to do would be it add to every @QuarkusTest

[maxandersen]

For tests it just works to add dependency. It's for quarkus dev I'm after it being enabled.

[geoand]

I'm thinking of just copying the code so we can do whatever we like whenever we like

[geoand]

So you only want these enhanced stacktraces for exceptions thrown in application code in dev mode?

[maxandersen]

If doable for quarkustest so users can turn it off if it somehow disturbs other tooling then that's cool too.

My main thing was to have it for stacktraces thrown/logged and rendered during quarkus dev.

[maxandersen]

I'm thinking of just copying the code so we can do whatever we like whenever we like

Yes. It's trivial code. And just mark it clearly where it comes from(with a comment or so) to recognize original authors.

[geoand]

👌

[geoand]

@maxandersen I am kind confused about what you actually want here.

Do you want this for dev-mode only, @QuarkusTest and dev-mode, only for @QuarkusTest when running in continuous testing, something else?

[maxandersen]

Main thing was devmode.

If we can add it to quarkus test without too much conflicts and especially possible to turn off/on with same property then I think enabling it for all the things (dev/test but not prod) would be nice.

[geoand]

So how about we just add it to the error page you mentioned and see how much people like it or not before adding it elsewhere?

That's the approach I took in ##37034

[maxandersen]

I was hoping we add it to places where unhandled exceptions get logged. The upside of this approach is that even a massive long stack trace is easy to navigate as only the stuff that has source code is easy to spot.

The more I think of it it feels like something JBoss logging should allow enabling rather than we wrap it everywhere.

Thoughts @dmlloyd ?

[geoand]

Sure, but that requires support in logmanager

[dmlloyd]

I think it's definitely an interesting idea. For generalized support in the log manager though, we'd need some general way of finding the source code, with a clean fallback path, without introducing a dependency. We also would need a way to configure the maximum number of stack trace levels for which to provide a source listing (e.g. default 1), perhaps also with a maximum number of nested caused-by (e.g. default all). Perhaps we might expose a StackTraceFormatter interface, allowing Quarkus or WildFly to provide their own strategy.

Reversing the causes is also something we might consider in our exception formatter too; in that mode, instead of Caused by: we'd have e.g. Which caused: . Localization is a concern though.

/cc @jamezp who might have more ideas.

[maxandersen]

Stack trace has absolute path of source location if an available so it "just works".

I would think it would be good to be pluggable as you'll not want it enabled always.

[dmlloyd]

Right we can find a source file based on the package name and source file name, but we also need to know where to look. In general, the logmanager doesn't have any concept of a source path, and I'm not sure we want to introduce one either. But making it pluggable does solve that issue nicely I think.

[jamezp]

If it's something we want in the log manager, I think it definitely should be pluggable and off by default. I could definitely see it being useful as well though.

That said, IMO stack traces are pretty easy to read and rarely do I find the error in the first line of the stack trace entry :)

[maxandersen]

@dmlloyd look at the linked code.this code does not need to lookup based on package and class. It simply get source location from StacktraceElement and only renders if that path exists.

This means it does not get noisy with expanding code from all intermediate frameworks but mainly shows code you actually have ability to fix.

Fits quite natural
For business app devs imo.

[dmlloyd]

Right, all I was saying was that without having the path of a source zip, or the path of your Java source base directory, the information in the stack trace doesn't help you. It doesn't tell you where the source is in an absolute sense. It tells you where it is relative to some lookup base. While Quarkus does track that information in dev mode, the logmanager is a run time library so generally speaking there is not enough information for sources. Specifically in the case of Quarkus, there is. Also, specifically in the case where (for example) a source JAR can be downloaded from Maven, there also is enough information.

But since there isn't enough information in the general case, we can't add general support in the log manager - it's just not possible. That's why I suggested a plugin API that the logmanager can delegate to, so that in cases where we are running in a framework that knows where the sources are (e.g. Quarkus), that framework can decorate the stack traces (maybe using this library, for example).

The "source location" in the stack trace element amounts to a file name (which is not unique if you have more than one class - it's just the file name without a directory) and a declaring class name in human-readable form. So if I have foo/bar/Baz and bam/yip/Baz, my source file name will be Baz.java in both cases, but the "class name" field will be foo.bar.Baz in the first case and bam.yip.Baz in the second case. So if my source JAR contains foo/bar/Baz.java and bam/yip/Baz.java, I have to combine those two pieces of information to know the actual source location.

And to make matters more complex, that's really just a logical guess in the end, because unless I parse all of the sources in a given location enough to get their package declaration, we still might not be able to find them. The reason is that the sources could legitimately be in an non-package-oriented layout; after all, javac does not enforce a package-oriented layout: it's just a convention. So in my prior example, foo.bar.Baz might be in src1/whatever/Baz.java and bam.yip.Baz might be in src2/something/else/Baz.java, for whatever crazy reason the user randomly decided to do that.

So that's why I said "we can find a source file based on the package name and source file name", and furthermore I was just weakly implying all of the aforementioned magic. Of course, using a library in which all of this was already solved is really ideal: if there's some unsupported edge case, we can always file an upstream bug. :-)

[maxandersen]

having just dealt with issue where this could really have helped me :)

+1 on doing #37034 as start for the error page.