Stack trace detection do not work in colored console

[laeubi]

If the coloured output includes a stacktrace, then eclipse cant detect stack traces anymore.

@mihnita I noticed this with m2e and a failing maven build (e.g. one can enable -e or -X to get stack traces), I assume this is because if you copy this it results in and this is probably the thing also the Stack-Trace detection uses and then can't link the result to code:

Caused by�[m: java.lang.NullPointerException: �[1;31mCannot invoke "org.eclipse.equinox.internal.provisional.p2.core.eventbus.IProvisioningEventBus.addListener(org.eclipse.equinox.internal.provisional.p2.core.eventbus.ProvisioningListener)" because "this.eventBus" is null�[m
    �[1mat�[m org.eclipse.equinox.internal.p2.repository.helpers.AbstractRepositoryManager.<init> (�[1mAbstractRepositoryManager.java:107�[m)
    �[1mat�[m org.eclipse.equinox.internal.p2.artifact.repository.ArtifactRepositoryManager.<init> (�[1mArtifactRepositoryManager.java:41�[m)
    �[1mat�[m org.eclipse.equinox.internal.p2.artifact.repository.ArtifactRepositoryComponent.createService (�[1mArtifactRepositoryComponent.java:27�[m)
    �[1mat�[m org.eclipse.tycho.p2maven.transport.RemoteArtifactRepositoryManagerAgentFactory.createService (�[1mRemoteArtifactRepositoryManagerAgentFactory.java:33�[m)
    �[1mat�[m org.eclipse.tycho.p2maven.DefaultProvisioningAgent.lambda$getAgentFactoryService$0 (�[1mDefaultProvisioningAgent.java:73�[m)
    �[1mat�[m java.util.concurrent.ConcurrentHashMap.computeIfAbsent (�[1mConcurrentHashMap.java:1708�[m)
    �[1mat�[m org.eclipse.tycho.p2maven.DefaultProvisioningAgent.getAgentFactoryService (�[1mDefaultProvisioningAgent.java:70�[m)
    �[1mat�[m org.eclipse.tycho.p2maven.DefaultProvisioningAgent.getService (�[1mDefaultProvisioningAgent.java:53�[m)
    �[1mat�[m org.eclipse.equinox.p2.core.IProvisioningAgent.getService (�[1mIProvisioningAgent.java:86�[m)
    �[1mat�[m org.eclipse.tycho.p2maven.InstallableUnitGenerator.init (�[1mInstallableUnitGenerator.java:130�[m)
    �[1mat�[m org.eclipse.tycho.p2maven.InstallableUnitGenerator.getInstallableUnits (�[1mInstallableUnitGenerator.java:102�[m)
    �[1mat�[m org.eclipse.tycho.p2maven.MavenProjectDependencyProcessor.computeProjectDependencyClosure (�[1mMavenProjectDependencyProcessor.java:83�[m)
    �[1mat�[m org.eclipse.tycho.build.TychoGraphBuilder.build (�[1mTychoGraphBuilder.java:148�[m)
    �[1mat�[m org.apache.maven.DefaultMaven.buildGraph (�[1mDefaultMaven.java:532�[m)
    �[1mat�[m org.apache.maven.DefaultMaven.doExecute (�[1mDefaultMaven.java:219�[m)
    �[1mat�[m org.apache.maven.DefaultMaven.doExecute (�[1mDefaultMaven.java:192�[m)
    �[1mat�[m org.apache.maven.DefaultMaven.execute (�[1mDefaultMaven.java:105�[m)
    �[1mat�[m org.apache.maven.cli.MavenCli.execute (�[1mMavenCli.java:960�[m)
    �[1mat�[m org.apache.maven.cli.MavenCli.doMain (�[1mMavenCli.java:293�[m)
    �[1mat�[m org.apache.maven.cli.MavenCli.main (�[1mMavenCli.java:196�[m)
    �[1mat�[m jdk.internal.reflect.NativeMethodAccessorImpl.invoke0 (�[1mNative Method�[m)
    �[1mat�[m jdk.internal.reflect.NativeMethodAccessorImpl.invoke (�[1mNativeMethodAccessorImpl.java:77�[m)
    �[1mat�[m jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke (�[1mDelegatingMethodAccessorImpl.java:43�[m)
    �[1mat�[m java.lang.reflect.Method.invoke (�[1mMethod.java:568�[m)
    �[1mat�[m org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced (�[1mLauncher.java:282�[m)
    �[1mat�[m org.codehaus.plexus.classworlds.launcher.Launcher.launch (�[1mLauncher.java:225�[m)
    �[1mat�[m org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode (�[1mLauncher.java:406�[m)
    �[1mat�[m org.codehaus.plexus.classworlds.launcher.Launcher.main (�[1mLauncher.java:347�[m)

[iloveeclipse]

See various console pattern match listeners defined in https://github.com/eclipse-jdt/eclipse.jdt.debug/blob/master/org.eclipse.jdt.debug.ui/plugin.xml#L3503

I guess they can't parse stack traces with ASNI escape characters.

[mihnita]

How can I reproduce this?

I've triggered an exception (java.text.MessageFormat.format("Today is {date}, OK?", "date", new Date());), and the result does not use escape sequences to color the output.

The output is in the standard error color, the file names have the link color, underlined, I can click on them, and they take me there.

The behavior seems identical with the ANSI support enabled / disabled.
And with it enabled and with "show the escape sequences" checked (good for debugging) does not show any escapes (because the links are formatted before the ANSI support kicks in, not with ANSI, and the styling is preserved.

Thanks,
Mihai

[mihnita]

Note: I've tried this from a small app (just put that code in main and running it), and from maven, in an unit test.
M.

[laeubi]

Good question, I assume the follwing should work:

1. latest m2e + eclipse with color console
2. simple maven project with compile error
3. run maven from withing eclipse and enable "Debug output" in the run config.

[mihnita]

Sorry, I can't figure out how to reproduce it.

I've created a maven project from scratch, and tried building and running tests, both from right click -- Run as -- Maven install,
and creating a run configuration under Maven Build with the goal install

And tried to trigger the dumping of the extension by introducing of some code that fails at runtime (in the unit tests).
then some incorrect code that would fail at compile time.
Everything seems to behave the same with ANSI support both enabled and disabled.
And nothing produces this kind of line:

    �[1mat�[m org.eclipse.tycho.build.TychoGraphBuilder.build (�[1mTychoGraphBuilder.java:148�[m)

They do, but not with those escape sequences (\033[1m is bold, \033[m is reset attributes)

[laeubi]

Cant you simply have a main with sytem out my example?

[mihnita]

OK, I finally got it!
I guess I didn't know what to look for...

I don't know what would create that kind of output, I didn't manage to find a way to create an exception with those escapes.
But "faking it" with System.out worked.

String exception = ""
    + "Caused by\033[m: java.lang.NullPointerException: \033[1;31mCannot invoke \"org.eclipse.equinox.internal.provisional.p2.core.eventbus.IProvisioningEventBus.addListener(org.eclipse.equinox.internal.provisional.p2.core.eventbus.ProvisioningListener)\" because \"this.eventBus\" is null\033[m\n"
    + "    \033[1mat\033[m org.eclipse.equinox.internal.p2.repository.helpers.AbstractRepositoryManager.<init> (\033[1mAbstractRepositoryManager.java:107\033[m)\n"
    ...
    + "    \033[1mat\033[m org.codehaus.plexus.classworlds.launcher.Launcher.main (\033[1mLauncher.java:347\033[m)\n";
  System.out.println(exception);

I'll dig into it this week-end (well, starting in less than 24h from now :-)

[laeubi]

I don't know what would create that kind of output, I didn't manage to find a way to create an exception with those escapes.

Yeah it might be hard to reproduce, as I'm often developing maven-plugs (in contrast to using them in a build) you can screw up maven in different ways and it might throw exceptions/print messages at you that are not show in a regular maven build, so using system-out seems the most suitable here for validation.

[mihnita]

Created PR (eclipse-jdt/eclipse.jdt.debug#148)

Thanks @iloveeclipse for the pointer, it was right on and saved me a lot of digging time.

It is kind of clunky, and detection for the link seems to be done in two places.

Once in JavaConsoleTracker.java, which determines where in the console is the hyperlink (to apply attributes),
and once again in JavaStackTraceHyperlink.java, which gets the line and finds the place to jump to.

I wonder if there is a way to refactor and reuse the same code.

[mihnita]

Future cleanup?

In the current fix I've tried to not change things too much (to not shake the boat after M2 :-)

But I would look into reusing the code for link detection.

And probably even better would be to implement the idea in issue #539

[mihnita]

Gentle ping?
If this looks reasonable to you I will sync to the head and make sure it builds.
So that maybe have it in RC1?

Thank you,
Mihai

[laeubi]

@mihnita as you are a commiter you don't need to wait (unless RC period) for approval of others, if you are fine with it, just go ahead. If you like you can even request reviewer in a PR.

So I would suggest, update your PR with the latest master, check that builds are running and then ask for approval for RC1 any of the project-leads here can approve:
https://projects.eclipse.org/projects/eclipse.platform/who

[akurtakov]

One clarification. Patch is in JDT so one needs approval/review/push from https://projects.eclipse.org/projects/eclipse.jdt/who

[mihnita]

Thanks. Still learning the ropes :-)

Being in JDT I also don't have the right to assign the PR to someone.
I suppose I would just mention them in a comment and they will get notified.
Is that enough?

Thank you,
Mihai

[laeubi]

@mihnita do you plan to work on this to get this fixed in this release?

You should be able to reproduce it the following way:

1. Check out the following commit: eclipse-tycho/tycho@681b2e3
2. run mvn clean install -DskipTests -T1C -Dtycho.localArtifacts=ignore to build this state of Tycho
3. then in eclipse import the project tycho/demo/testing/surefire/with-maven-layout and run its pom.xml as a maven build.

The build will fail with an internal error

no stack traces are detected and one can't click to jump into the code.

[mihnita]

A good discussion in this closed PR: eclipse-jdt/eclipse.jdt.debug#148

I will summarize it a bit, but you can go there for the complete thread

The main part, from laeubi:

What I'm wondering is if TextConsole should be extended in a way to get the "plaintext",
this can then return just the normal text as a default implementation but the ansi console
might return the unescaped text.
...
I think we should not apply a quick-fix here as it unnecessary complicates things. The stacktraces are just one example and I think the ANSI console should best "hide" the ANSI nature that is when I copy/paste things, copy without escapes should be default and for API it should return a plain text and all else (HTML, RTF, with escapes) should be special cases one needs to explcitily trigger.

I've looked again at how the detection is implemented, and it is simply

private TextConsole fConsole;
...
...
String text = fConsole.getDocument().get(offset, length);

So it looks like changing the TextConsole would not do much, I would really need to do the hiding in the IDocument (more precise ConsoleDocument).

In the PR:

Yes, I think that would be a good solution. So I've spent some time on it over the holydays.
But is it not a simple refactoring, so I don't have yet a working implementation.
I think the way to go would be updating the the ConsoleDocument, not TextConsole?
And the "raw content" of the document would be plain text, no escapes, but with Positions.
Do you think that it sounds like a bad direction to go?
It is also weird that there are 2 ConsoleDocument classes (org.eclipse.m2e.core.ui.internal.console.ConsoleDocument in m2e-core and org.eclipse.ui.internal.console.ConsoleDocument in eclipse.platform.debug)
So making something to share between the two might be tricky :-(

[laeubi]

So it looks like changing the TextConsole would not do much, I would really need to do the hiding in the IDocument (more precise ConsoleDocument).

That sounds like a good idea! Maybe one can have ConsoleDocument#getRawText(offset, length) that returns it with all encoded stuff and the ConsoleDocument.get(offset, length); returns it without the special ansi chars.