Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decorate stacktraces in dev mode error page #37034

Merged
merged 2 commits into from
Jul 26, 2024
Merged

Conversation

geoand
Copy link
Contributor

@geoand geoand commented Nov 13, 2023

This comment has been minimized.

@maxandersen maxandersen marked this pull request as ready for review July 12, 2024 08:21
@maxandersen
Copy link
Member

@phillip-kruger you wanna take this in?

This comment has been minimized.

@geoand
Copy link
Contributor Author

geoand commented Jul 15, 2024

🎉

This comment has been minimized.

@maxandersen
Copy link
Member

this really works awesome.

one thing I noticed was that with some compile errors I seem to get ansi characters in the output?

with this:

package org.acme;

import jakarta.ws.rs.GET;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.Produces;
import jakarta.ws.rs.core.MediaType;

@Path("/hello")
public class GreetingResource {

    @GET
    @Produces(MediaType.TEXT_PLAIN)
    public String hello() {
        inter var;
       // if (System.currentTimeMillis()!=0) throw new RuntimeException();
        return "Hello from Quarkus REST";
    }
}

I got:

java.lang.RuntimeException: �[91mCompilation Failed:
/Users/manderse/code/latestapp2/src/main/java/org/acme/GreetingResource.java:14: error: cannot find symbol
        inter var;
        ^
  symbol:   class inter
  location: class org.acme.GreetingResource�[0m
	at io.quarkus.deployment.dev.JavaCompila

but

package org.acme;

import jakarta.ws.rs.GET;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.Produces;
import jakarta.ws.rs.core.MediaType;

@Path("/hello")
public class GreetingResource {

    @GET
    @Produces(MediaType.TEXT_PLAIN)
    public String hello() {
        if (System.currentTimeMillis()!=0) throw new RuntimeException();
        return "Hello from Quarkus REST";
    }
}

gave me clean/no ansi:

java.lang.RuntimeException
	at org.acme.GreetingResource.hello(GreetingResource.java:14)

	   12      @Produces(MediaType.TEXT_PLAIN)
	   13      public String hello() {
	-> 14          if (System.currentTimeMillis()!=0) throw new RuntimeException();
	   15          return "Hello from Quarkus REST";
	   16      }


	at org.acme.GreetingResource$quarkusrestinvoker$hello_e747664148511e1e5212d3e0f4b40d45c56ab8a1.invoke(Unknown Source)
	at org.jboss.resteasy.reactive.server.handlers.InvocationHandler.handle(InvocationHandler.java:29)
	at io.quarkus.resteasy.reac

also - should we have original stacktrace link provide the true original without the injected locations?

@maxandersen
Copy link
Member

for future - would be great to have those lines with know locations clickable to open in IDE :)

Copy link
Member

@maxandersen maxandersen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 on this; if the ansi code is known issue already we should merge this as its really helpful IMO.

the other suggestions are improvements on what is already nice.

@phillip-kruger phillip-kruger force-pushed the #37002 branch 2 times, most recently from a56eff5 to b043849 Compare July 18, 2024 02:59
@phillip-kruger
Copy link
Member

@maxandersen I can not recreate the encoding issue ...
I added a config to disable this in Dev Mode, or enable this in other modes: quarkus.http.decorate-stacktraces=false

@phillip-kruger
Copy link
Member

Making the lines clickable to open an IDE is a bigger piece of work. The error page is not a Dev UI page, so it's not that easy. However, looking at the log in Dev UI, you can click on it:
clickable

This comment has been minimized.

Copy link
Member

@maxandersen maxandersen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as we don't have options for disabling the reversal of stacktraces and there are security issues for prod - I suggest to make this pr have this feature enabled always in devmode and only add a option to disable in dev and enable in prod as a follow up if we get feedback/usecase on it.

Signed-off-by: Phillip Kruger <[email protected]>
Copy link

quarkus-bot bot commented Jul 21, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 5348b71.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.


Flaky tests - Develocity

⚙️ JVM Tests - JDK 17 Windows

📦 integration-tests/grpc-hibernate

com.example.grpc.hibernate.VertxBlockingRawTest.shouldAdd - History

  • Condition with Lambda expression in com.example.grpc.hibernate.BlockingRawTestBase was not fulfilled within 30 seconds. - org.awaitility.core.ConditionTimeoutException
org.awaitility.core.ConditionTimeoutException: Condition with Lambda expression in com.example.grpc.hibernate.BlockingRawTestBase was not fulfilled within 30 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:26)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1006)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:975)
	at com.example.grpc.hibernate.BlockingRawTestBase.shouldAdd(BlockingRawTestBase.java:59)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)

Copy link
Member

@maxandersen maxandersen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm

@geoand geoand merged commit 86d7b85 into quarkusio:main Jul 26, 2024
52 checks passed
@quarkus-bot quarkus-bot bot added the kind/enhancement New feature or request label Jul 26, 2024
@quarkus-bot quarkus-bot bot added this to the 3.14 - main milestone Jul 26, 2024
@gsmet gsmet changed the title Decorate stacktraces in dev-mode error page Decorate stacktraces in dev mode error page Aug 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

provide source file context for exception message
3 participants