Fix #436: Add cause message to RestClientException by banterCZ · Pull Request #437 · wultra/java-core

banterCZ · 2026-02-19T08:58:03Z

No description provided.

Copilot

Pull request overview

This PR addresses issue #436 by enriching RestClientException messages thrown by DefaultRestClient so they include the underlying exception message, and it adds/adjusts tests to validate the new message behavior.

Changes:

Append the caught exception message to RestClientException messages for GET/POST/PUT/DELETE/PATCH/HEAD failures.
Add new tests asserting the updated failure message includes DNS resolution details.
Refactor an existing error-response test to use assertThrows and add an assertion for the exception message.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
rest-client-base/src/main/java/com/wultra/core/rest/client/base/DefaultRestClient.java	Updates thrown `RestClientException` messages to include the underlying exception message.
rest-client-base/src/test/java/com/wultra/core/rest/client/base/DefaultRestClientTest.java	Adds message assertions for request-failure scenarios and improves one error-response test structure.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-19T09:01:15Z

rest-client-base/src/main/java/com/wultra/core/rest/client/base/DefaultRestClient.java

                throw (RestClientException) ex.getCause();
            }
-            throw new RestClientException("HTTP GET request failed", ex);
+            throw new RestClientException("HTTP GET request failed; " + ex.getMessage(), ex);


The new exception message concatenates ex.getMessage() unconditionally. For some exception types getMessage() can be null, resulting in messages like HTTP GET request failed; null, which is not helpful. Consider appending the cause message only when present/non-blank (or falling back to ex.getClass().getSimpleName()/ex.toString()), and apply the same pattern across the other HTTP methods updated in this PR.

IMHO there should be always some message for exception and we should just put it into RestClientException, no matter it is null, empty string ... So the implementation in this PR is correct

Copilot · 2026-02-19T09:01:15Z

rest-client-base/src/test/java/com/wultra/core/rest/client/base/DefaultRestClientTest.java

+    void testGet_causeMessage() throws Exception {
+        final DefaultRestClient localRestClient = new DefaultRestClient("http://nonexistent.invalid");
+        final RestClientException exception = assertThrows(RestClientException.class, () ->
+                localRestClient.get("/", new ParameterizedTypeReference<>() {}));
+
+        assertTrue(
+                exception.getMessage().startsWith("HTTP GET request failed; Failed to resolve 'nonexistent.invalid'"),
+                () -> "Unexpected message: " + exception.getMessage());


These tests make real network calls and assert a very specific DNS-resolution error message. This is likely to be flaky/slow in CI (network may be blocked) and the message text varies across OS/JDK/Reactor Netty versions. To make the test stable, avoid external DNS and instead inject/mock the WebClient/connector to throw a deterministic exception, or assert more loosely (e.g., that the message starts with HTTP <METHOD> request failed; and that the cause message is non-empty/contains the hostname).

I agree that mocking webClient would be better. On the other hand it would require quite a lot of stubbing and these tests would be hard to read. So usage real localRestClient is acceptable for this use-case.

Copilot · 2026-02-19T09:01:15Z

rest-client-base/src/test/java/com/wultra/core/rest/client/base/DefaultRestClientTest.java

+    void testGet_causeMessage() throws Exception {
+        final DefaultRestClient localRestClient = new DefaultRestClient("http://nonexistent.invalid");
+        final RestClientException exception = assertThrows(RestClientException.class, () ->
+                localRestClient.get("/", new ParameterizedTypeReference<>() {}));
+
+        assertTrue(
+                exception.getMessage().startsWith("HTTP GET request failed; Failed to resolve 'nonexistent.invalid'"),
+                () -> "Unexpected message: " + exception.getMessage());
+    }
+
+    @Test
+    void testHead_causeMessage() throws Exception {
+        final DefaultRestClient localRestClient = new DefaultRestClient("http://nonexistent.invalid");
+        final RestClientException exception = assertThrows(RestClientException.class, () ->
+                localRestClient.head("/", new ParameterizedTypeReference<>() {}));
+
+        assertTrue(
+                exception.getMessage().startsWith("HTTP HEAD request failed; Failed to resolve 'nonexistent.invalid'"),
+                () -> "Unexpected message: " + exception.getMessage());
+    }
+
+    @Test
+    void testDelete_causeMessage() throws Exception {
+        final DefaultRestClient localRestClient = new DefaultRestClient("http://nonexistent.invalid");
+        final RestClientException exception = assertThrows(RestClientException.class, () ->
+                localRestClient.delete("/", new ParameterizedTypeReference<>() {}));
+
+        assertTrue(
+                exception.getMessage().startsWith("HTTP DELETE request failed; Failed to resolve 'nonexistent.invalid'"),
+                () -> "Unexpected message: " + exception.getMessage());
+    }
+
+    @Test
+    void testPut_causeMessage() throws Exception {
+        final DefaultRestClient localRestClient = new DefaultRestClient("http://nonexistent.invalid");
+        final RestClientException exception = assertThrows(RestClientException.class, () ->
+                localRestClient.put("/", null, new ParameterizedTypeReference<>() {}));
+
+        assertTrue(
+                exception.getMessage().startsWith("HTTP PUT request failed; Failed to resolve 'nonexistent.invalid'"),
+                () -> "Unexpected message: " + exception.getMessage());
+    }
+
+    @Test
+    void testPost_causeMessage() throws Exception {
+        final DefaultRestClient localRestClient = new DefaultRestClient("http://nonexistent.invalid");
+        final RestClientException exception = assertThrows(RestClientException.class, () ->
+                localRestClient.post("/", null, new ParameterizedTypeReference<>() {}));
+
+        assertTrue(
+                exception.getMessage().startsWith("HTTP POST request failed; Failed to resolve 'nonexistent.invalid'"),
+                () -> "Unexpected message: " + exception.getMessage());
+    }


The five *_causeMessage tests are nearly identical aside from the HTTP method. Consider converting them to a single parameterized test (e.g., method name + lambda) to reduce duplication and make future message changes easier to update consistently.

michal-rozehnal-w

Nice PR, it will definitely help us in any future investigations

michal-rozehnal-w · 2026-02-19T09:11:36Z

rest-client-base/src/main/java/com/wultra/core/rest/client/base/DefaultRestClient.java

                throw (RestClientException) ex.getCause();
            }
-            throw new RestClientException("HTTP GET request failed", ex);
+            throw new RestClientException("HTTP GET request failed; " + ex.getMessage(), ex);


IMHO there should be always some message for exception and we should just put it into RestClientException, no matter it is null, empty string ... So the implementation in this PR is correct

michal-rozehnal-w · 2026-02-19T09:21:04Z

rest-client-base/src/test/java/com/wultra/core/rest/client/base/DefaultRestClientTest.java

+    void testGet_causeMessage() throws Exception {
+        final DefaultRestClient localRestClient = new DefaultRestClient("http://nonexistent.invalid");
+        final RestClientException exception = assertThrows(RestClientException.class, () ->
+                localRestClient.get("/", new ParameterizedTypeReference<>() {}));
+
+        assertTrue(
+                exception.getMessage().startsWith("HTTP GET request failed; Failed to resolve 'nonexistent.invalid'"),
+                () -> "Unexpected message: " + exception.getMessage());


I agree that mocking webClient would be better. On the other hand it would require quite a lot of stubbing and these tests would be hard to read. So usage real localRestClient is acceptable for this use-case.

romanstrobl

Looks OK, I just don't like the ; in the error message.

rest-client-base/src/main/java/com/wultra/core/rest/client/base/DefaultRestClient.java

Fix #436: Add cause message to RestClientException

7e16b4b

banterCZ requested review from Copilot, michal-rozehnal-w, romanstrobl and zcgandcomp February 19, 2026 08:58

Copilot started reviewing on behalf of banterCZ February 19, 2026 08:58 View session

Copilot AI reviewed Feb 19, 2026

View reviewed changes

michal-rozehnal-w approved these changes Feb 19, 2026

View reviewed changes

romanstrobl approved these changes Feb 20, 2026

View reviewed changes

rest-client-base/src/main/java/com/wultra/core/rest/client/base/DefaultRestClient.java Outdated Show resolved Hide resolved

Improve message format

49d1239

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

Fix #436: Add cause message to RestClientException#437

Fix #436: Add cause message to RestClientException#437
banterCZ wants to merge 2 commits intodevelopfrom
issues/436-cause-message

banterCZ commented Feb 19, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

michal-rozehnal-w Feb 19, 2026

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

michal-rozehnal-w Feb 19, 2026

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

michal-rozehnal-w left a comment

Uh oh!

michal-rozehnal-w Feb 19, 2026

Uh oh!

michal-rozehnal-w Feb 19, 2026

Uh oh!

romanstrobl left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Comments

Conversation

banterCZ commented Feb 19, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

michal-rozehnal-w Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

michal-rozehnal-w Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

michal-rozehnal-w left a comment

Choose a reason for hiding this comment

Uh oh!

michal-rozehnal-w Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

michal-rozehnal-w Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

romanstrobl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants