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

Re-implement decorate stacktrace #42200

Merged

Conversation

phillip-kruger
Copy link
Member

@phillip-kruger phillip-kruger commented Jul 29, 2024

The decorate PR (#37034) that was merged a few days ago broke the log help :

[x] - Open last exception (or project) in IDE

This PR re implements the decorate feature in a safer way. (The old implementation clears the stacktraceitems array).

The previous PR (that is not in a release yet) changed the Throwable by setting the stacktrace to an empty array, and override the toString. This broke anyone else downstream that is looking at the stacktrace elements.

This PR does not change the stacktrace item array, but rather change the message (for console only) just before we write to console. For the error page we just include the decoration on the page, before the exception is printed.

Now in the log:
error_console

And then pressing x will open the correct file on the correct place in your IDE

And now in the error page:
error_page

And (this is new) clicking on the link will open the correct file on the correct place in your IDE

Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

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

This PR re implements the decorate feature in a safer way. (The old implementation clears the stacktraceitems array.

Can we also have a description here of what the old way vs the new way is for posterity?

This comment has been minimized.

@phillip-kruger phillip-kruger force-pushed the fix-exception-class-finder branch 2 times, most recently from 782a5b4 to f0ac671 Compare July 30, 2024 00:26
@phillip-kruger phillip-kruger requested a review from geoand July 30, 2024 00:26
@phillip-kruger
Copy link
Member Author

@geoand I updated the description and added the Javadoc. Thanks

This comment has been minimized.

@phillip-kruger phillip-kruger force-pushed the fix-exception-class-finder branch 2 times, most recently from 8e0bbd5 to 8b9b636 Compare July 30, 2024 02:46
Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

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

Excellent!

This comment has been minimized.

@phillip-kruger phillip-kruger force-pushed the fix-exception-class-finder branch from 8b9b636 to 280a54b Compare July 30, 2024 10:22
@phillip-kruger phillip-kruger added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Jul 30, 2024
@phillip-kruger phillip-kruger force-pushed the fix-exception-class-finder branch from 280a54b to 8f4a26b Compare July 30, 2024 11:09

This comment has been minimized.

@phillip-kruger phillip-kruger force-pushed the fix-exception-class-finder branch from 8f4a26b to 8a1d22b Compare July 31, 2024 00:49
@phillip-kruger phillip-kruger force-pushed the fix-exception-class-finder branch 4 times, most recently from 0a0d511 to d7bf4cd Compare July 31, 2024 04:06

This comment has been minimized.

@phillip-kruger phillip-kruger force-pushed the fix-exception-class-finder branch from d7bf4cd to cabc530 Compare July 31, 2024 11:04
Signed-off-by: Phillip Kruger <[email protected]>
@phillip-kruger phillip-kruger force-pushed the fix-exception-class-finder branch from cabc530 to 861c31e Compare July 31, 2024 11:05

This comment has been minimized.

Copy link

quarkus-bot bot commented Aug 1, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 861c31e.

Failing Jobs

Status Name Step Failures Logs Raw logs Build scan
✔️ JVM Tests - JDK 17 Failures Logs Raw logs 🚧
✔️ JVM Tests - JDK 21 Logs Raw logs 🚧

Full information is available in the Build summary check run.

Failures

⚙️ JVM Tests - JDK 17 #

- Failing: devtools/cli 

📦 devtools/cli

io.quarkus.cli.config.SetConfigTest.addEncryptedConfiguration line 74 - History - More details - Source on GitHub

java.lang.RuntimeException: java.lang.NegativeArraySizeException: -98
	at io.smallrye.config.crypto.AESGCMNoPaddingSecretKeysHandler.decode(AESGCMNoPaddingSecretKeysHandler.java:44)
	at io.smallrye.config.ExpressionConfigSourceInterceptor$1.accept(ExpressionConfigSourceInterceptor.java:79)
	at io.smallrye.config.ExpressionConfigSourceInterceptor$1.accept(ExpressionConfigSourceInterceptor.java:71)
	at io.smallrye.common.expression.ExpressionNode.emit(ExpressionNode.java:22)
	at io.smallrye.common.expression.Expression.evaluateException(Expression.java:56)
	at io.smallrye.common.expression.Expression.evaluate(Expression.java:70)
	at io.smallrye.config.ExpressionConfigSourceInterceptor.getValue(ExpressionConfigSourceInterceptor.java:71)

Flaky tests - Develocity

⚙️ JVM Tests - JDK 17

📦 integration-tests/opentelemetry

io.quarkus.it.opentelemetry.OpenTelemetryInjectionsTest.testOTelInjections - History

  • Condition with Lambda expression in io.quarkus.it.opentelemetry.OpenTelemetryInjectionsTest was not fulfilled within 5 seconds. - org.awaitility.core.ConditionTimeoutException
org.awaitility.core.ConditionTimeoutException: Condition with Lambda expression in io.quarkus.it.opentelemetry.OpenTelemetryInjectionsTest was not fulfilled within 5 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 io.quarkus.it.opentelemetry.OpenTelemetryInjectionsTest.reset(OpenTelemetryInjectionsTest.java:26)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)

⚙️ JVM Tests - JDK 21

📦 integration-tests/opentelemetry

io.quarkus.it.opentelemetry.OpenTelemetryInjectionsTest.testOTelInjections - History

  • Condition with Lambda expression in io.quarkus.it.opentelemetry.OpenTelemetryInjectionsTest was not fulfilled within 5 seconds. - org.awaitility.core.ConditionTimeoutException
org.awaitility.core.ConditionTimeoutException: Condition with Lambda expression in io.quarkus.it.opentelemetry.OpenTelemetryInjectionsTest was not fulfilled within 5 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 io.quarkus.it.opentelemetry.OpenTelemetryInjectionsTest.reset(OpenTelemetryInjectionsTest.java:26)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)

⚙️ Native Tests - HTTP

📦 integration-tests/rest-client-reactive

io.quarkus.it.rest.client.BasicTestIT.shouldCreateClientSpans - History

  • expected: <1> but was: <2> - org.opentest4j.AssertionFailedError
org.opentest4j.AssertionFailedError: expected: <1> but was: <2>
	at io.quarkus.it.rest.client.BasicTest.shouldCreateClientSpans(BasicTest.java:216)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at io.quarkus.test.junit.QuarkusTestExtension.interceptTestMethod(QuarkusTestExtension.java:821)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

@phillip-kruger phillip-kruger merged commit cbd0ab2 into quarkusio:main Aug 1, 2024
53 checks passed
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Aug 1, 2024
@quarkus-bot quarkus-bot bot added this to the 3.14 - main milestone Aug 1, 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.

2 participants