Provide context for Reactive Rest Client exceptions #22103

geoand · 2021-12-10T10:31:36Z

We now add the exact method that caused the exception
in order to make it easier for users to identity where
the exception is coming from

Fixes: #22090

michalszynkiewicz · 2021-12-10T10:56:29Z

.../runtime/src/main/java/org/jboss/resteasy/reactive/client/impl/RestClientRequestContext.java

+            var webApplicationException = (WebApplicationException) res;
+            var message = webApplicationException.getMessage();
+            var invokedMethodObject = properties.get(MP_INVOKED_METHOD_PROP);
+            if ((invokedMethodObject instanceof Method)) {


double brackets :)

Oops! Fixed

michalszynkiewicz · 2021-12-10T10:57:07Z

that's a really useful thing, thanks @geoand !

michalszynkiewicz · 2021-12-10T11:01:45Z

I think we coudl backport it, WDYT?

geoand · 2021-12-10T11:06:46Z

Yup, very good idea

geoand · 2021-12-10T12:26:15Z

@michalszynkiewicz I had to exclude some of the TCK tests because it was asserting the exact exception message

geoand · 2021-12-10T14:56:37Z

I introduced a property for reverting to the spec behavior simply so we can pass the stupid tests...

We now add the exact method that caused the exception in order to make it easier for users to identity where the exception is coming from Fixes: quarkusio#22090

quarkus-bot · 2021-12-10T18:11:33Z

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building `903eb34`

Status	Name	Step	Failures	Logs	Raw logs
✔️	JVM Tests - JDK 11
✖	JVM Tests - JDK 11 Windows	`Build`	Failures	Logs	Raw logs
✔️	JVM Tests - JDK 17

Full information is available in the Build summary check run.

Failures

⚙️ JVM Tests - JDK 11 Windows #

- Failing: integration-tests/rest-client

📦 integration-tests/rest-client

✖ io.quarkus.it.rest.client.selfsigned.ExternalSelfSignedTestCase.should_accept_self_signed_certs line 20 - More details - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code <200> but was <500>.

✖ io.quarkus.it.rest.client.selfsigned.ExternalSelfSignedTestCase.should_accept_self_signed_certs_java_url line 29 - More details - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code <200> but was <500>.

✖ io.quarkus.it.rest.client.trustall.ExternalTlsTrustAllTestCase.restClient line 20 - More details - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code <200> but was <500>.

✖ io.quarkus.it.rest.client.wronghost.ExternalWrongHostTestCase.restClient line 22 - More details - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code <200> but was <500>.

quarkus-bot · 2021-12-11T06:45:59Z

Failing Jobs - Building `903eb34`

Status	Name	Step	Failures	Logs	Raw logs
✔️	JVM Tests - JDK 11
✖	JVM Tests - JDK 11 Windows	`Build`	Failures	Logs	Raw logs
✔️	JVM Tests - JDK 17

Full information is available in the Build summary check run.

Failures

⚙️ JVM Tests - JDK 11 Windows #

- Failing: extensions/opentelemetry/opentelemetry/deployment 
! Skipped: extensions/opentelemetry/opentelemetry-exporter-jaeger/deployment extensions/opentelemetry/opentelemetry-exporter-otlp/deployment integration-tests/opentelemetry and 2 more

📦 extensions/opentelemetry/opentelemetry/deployment

✖ io.quarkus.opentelemetry.deployment.RestClientOpenTelemetryTest.client line 61 - More details - Source on GitHub

org.awaitility.core.ConditionTimeoutException: Assertion condition defined as a lambda expression in io.quarkus.opentelemetry.deployment.TestSpanExporter expected: <2> but was: <1> within 30 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:164)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:119)

geoand · 2021-12-11T07:05:00Z

@radcortez RestClientOpenTemetry test seems to be pretty flaky. I've seen it fail a few times the last few days

radcortez · 2021-12-12T23:48:22Z

Yes, I've already noticed it too. I'll have a look.

geoand · 2021-12-13T04:34:47Z

Thanks

gwenneg · 2022-01-20T13:24:25Z

Is there a way to get the old exception message back? I tried quarkus.rest-client.disable-contextual-error-messages=true but it's not working from our app 😢

gwenneg · 2022-01-20T13:25:37Z

I'm asking because I need the exception message to be unchanged, otherwise it will break something (which could be fixed with an ugly workaround) in our app.

geoand · 2022-01-20T13:37:30Z

For the time being, quarkus.rest-client.disable-contextual-error-messages only works as a system property

gwenneg · 2022-01-20T13:51:34Z

Thanks! I can disable the new behavior with a System.setProperty instruction at application startup indeed.

It would be great if quarkus.rest-client.disable-contextual-error-messages could become a full-fledged configuration key in a future release.

geoand · 2022-01-20T13:52:37Z

~~There is #2277 exactly that if someone wants to take it up (maybe @TomasHofman ? ) :)~~

I thought there was an issue that that tracks the request, but in any case yeah, it should be done.

gwenneg · 2022-01-20T13:59:46Z

Isn't #22777 tracking it as you initially commented?

geoand · 2022-01-20T14:08:12Z

Right, I got mixed up reading too many issues at the same time...

TomasHofman · 2022-01-20T14:11:31Z

Now I'm bit confused by the exchange, but I take it nobody is working on making the quarkus.rest-client.disable-contextual-error-messages a configuration property yet. I will try to add it to the config root.

geoand · 2022-01-20T14:17:05Z

Yeah, exactly that @TomasHofman. Sorry for the mixup

TomasHofman · 2022-01-20T14:17:51Z

No problem, thanks for confirming :).

geoand requested a review from michalszynkiewicz December 10, 2021 10:31

michalszynkiewicz reviewed Dec 10, 2021

View reviewed changes

michalszynkiewicz approved these changes Dec 10, 2021

View reviewed changes

michalszynkiewicz added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Dec 10, 2021

geoand force-pushed the #22090 branch from 0b46531 to c16ee7d Compare December 10, 2021 11:06

geoand added triage/backport-2.5? and removed triage/waiting-for-ci Ready to merge when CI successfully finishes labels Dec 10, 2021

geoand force-pushed the #22090 branch from c16ee7d to b3a18fa Compare December 10, 2021 12:25

geoand force-pushed the #22090 branch 2 times, most recently from 11cdafa to 599429c Compare December 10, 2021 14:56

Provide context for Reactive Rest Client exceptions

903eb34

We now add the exact method that caused the exception in order to make it easier for users to identity where the exception is coming from Fixes: quarkusio#22090

geoand force-pushed the #22090 branch from 599429c to 903eb34 Compare December 10, 2021 14:57

geoand added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Dec 10, 2021

geoand merged commit ea1a4f6 into quarkusio:main Dec 11, 2021

quarkus-bot bot added this to the 2.7 - main milestone Dec 11, 2021

quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Dec 11, 2021

gsmet modified the milestones: 2.7 - main, 2.6.0.Final Dec 13, 2021

gsmet removed the triage/backport? label Dec 13, 2021

gsmet modified the milestones: 2.6.0.Final, 2.5.3.Final Dec 14, 2021

gsmet removed the triage/backport-2.5? label Dec 14, 2021

geoand deleted the #22090 branch January 10, 2022 12:50

TomasHofman mentioned this pull request Jan 20, 2022

Rest-Client-Reactive: Allow disabling contextual error messages #22777

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Provide context for Reactive Rest Client exceptions #22103

Provide context for Reactive Rest Client exceptions #22103

geoand commented Dec 10, 2021

michalszynkiewicz Dec 10, 2021

geoand Dec 10, 2021

michalszynkiewicz commented Dec 10, 2021

michalszynkiewicz commented Dec 10, 2021

geoand commented Dec 10, 2021

geoand commented Dec 10, 2021 •

edited

Loading

geoand commented Dec 10, 2021

quarkus-bot bot commented Dec 10, 2021 •

edited

Loading

Failing Jobs - Building `903eb34`

Failures

⚙️ JVM Tests - JDK 11 Windows #

📦 integration-tests/rest-client

quarkus-bot bot commented Dec 11, 2021

geoand commented Dec 11, 2021

radcortez commented Dec 12, 2021

geoand commented Dec 13, 2021

gwenneg commented Jan 20, 2022

gwenneg commented Jan 20, 2022

geoand commented Jan 20, 2022

gwenneg commented Jan 20, 2022 •

edited

Loading

geoand commented Jan 20, 2022 •

edited

Loading

gwenneg commented Jan 20, 2022

geoand commented Jan 20, 2022

TomasHofman commented Jan 20, 2022

geoand commented Jan 20, 2022

TomasHofman commented Jan 20, 2022

Provide context for Reactive Rest Client exceptions #22103

Provide context for Reactive Rest Client exceptions #22103

Conversation

geoand commented Dec 10, 2021

michalszynkiewicz Dec 10, 2021

Choose a reason for hiding this comment

geoand Dec 10, 2021

Choose a reason for hiding this comment

michalszynkiewicz commented Dec 10, 2021

michalszynkiewicz commented Dec 10, 2021

geoand commented Dec 10, 2021

geoand commented Dec 10, 2021 • edited Loading

geoand commented Dec 10, 2021

quarkus-bot bot commented Dec 10, 2021 • edited Loading

Failing Jobs - Building 903eb34

Failures

⚙️ JVM Tests - JDK 11 Windows #

📦 integration-tests/rest-client

quarkus-bot bot commented Dec 11, 2021

Failing Jobs - Building 903eb34

Failures

⚙️ JVM Tests - JDK 11 Windows #

📦 extensions/opentelemetry/opentelemetry/deployment

geoand commented Dec 11, 2021

radcortez commented Dec 12, 2021

geoand commented Dec 13, 2021

gwenneg commented Jan 20, 2022

gwenneg commented Jan 20, 2022

geoand commented Jan 20, 2022

gwenneg commented Jan 20, 2022 • edited Loading

geoand commented Jan 20, 2022 • edited Loading

gwenneg commented Jan 20, 2022

geoand commented Jan 20, 2022

TomasHofman commented Jan 20, 2022

geoand commented Jan 20, 2022

TomasHofman commented Jan 20, 2022

geoand commented Dec 10, 2021 •

edited

Loading

quarkus-bot bot commented Dec 10, 2021 •

edited

Loading

Failing Jobs - Building `903eb34`

Failing Jobs - Building `903eb34`

gwenneg commented Jan 20, 2022 •

edited

Loading

geoand commented Jan 20, 2022 •

edited

Loading