You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by "jono-morris (via GitHub)" <gi...@apache.org> on 2023/09/18 13:08:31 UTC

[GitHub] [camel] jono-morris opened a new pull request, #11449: CAMEL-17173 Mute server errors by default

jono-morris opened a new pull request, #11449:
URL: https://github.com/apache/camel/pull/11449

   
   Updated code so that by default we don't include stack traces in the response body when we returning HTTP 500.  
   
   Also added a new option `logException` to log exceptions when a muted response, one not containing a stack trace, is returned.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [camel] jono-morris commented on pull request #11449: CAMEL-17173 Mute server errors by default

Posted by "jono-morris (via GitHub)" <gi...@apache.org>.
jono-morris commented on PR #11449:
URL: https://github.com/apache/camel/pull/11449#issuecomment-1727540884

   > @jono-morris is all okay, did you fix those tests ?
   
   @davsclaus, yes I've fixed all the tests.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [camel] jono-morris commented on a diff in pull request #11449: CAMEL-17173 Mute server errors by default

Posted by "jono-morris (via GitHub)" <gi...@apache.org>.
jono-morris commented on code in PR #11449:
URL: https://github.com/apache/camel/pull/11449#discussion_r1328808694


##########
components/camel-servlet/src/test/java/org/apache/camel/component/servlet/ServletMuteExceptionTest.java:
##########
@@ -25,6 +25,18 @@
 
 public class ServletMuteExceptionTest extends ServletCamelRouterTestSupport {
 
+    @Test
+    public void testMuteDefaultTrue() throws Exception {
+        WebRequest req = new PostMethodWebRequest(
+                contextUrl + "/services/muteDefault",
+                new ByteArrayInputStream("".getBytes()), "text/plain");
+        WebResponse response = query(req, false);
+
+        assertEquals(500, response.getResponseCode());
+        assertEquals("text/plain", response.getContentType());
+        assertEquals("", response.getText());

Review Comment:
   Done, thanks!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [camel] github-actions[bot] commented on pull request #11449: CAMEL-17173 Mute server errors by default

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #11449:
URL: https://github.com/apache/camel/pull/11449#issuecomment-1723376248

   :star2: Thank you for your contribution to the Apache Camel project! :star2: 
   
   :robot: CI automation will test this PR automatically.
   
   :camel: Apache Camel Committers, please review the following items:
   
   * First-time contributors **require MANUAL approval** for the GitHub Actions to run
   
   * You can use the command `/component-test (camel-)component-name1 (camel-)component-name2..` to request a test from the test bot.
   
   * You can label PRs using `build-all`, `build-dependents`, `skip-tests` and `test-dependents` to fine-tune the checks executed by this PR.
   
   * Build and test logs are available in the Summary page. **Only** [Apache Camel committers](https://camel.apache.org/community/team/#committers) have access to the summary. 
   
   * :warning: Be careful when sharing logs. Review their contents before sharing them publicly.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [camel] davsclaus commented on pull request #11449: CAMEL-17173 Mute server errors by default

Posted by "davsclaus (via GitHub)" <gi...@apache.org>.
davsclaus commented on PR #11449:
URL: https://github.com/apache/camel/pull/11449#issuecomment-1727466656

   @jono-morris is all okay, did you fix those tests ?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [camel] jono-morris commented on pull request #11449: CAMEL-17173 Mute server errors by default

Posted by "jono-morris (via GitHub)" <gi...@apache.org>.
jono-morris commented on PR #11449:
URL: https://github.com/apache/camel/pull/11449#issuecomment-1723526185

   Also noticed that I need to fix up a couple of the unit tests in camel-jetty that expect a response body with HTTP 500.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [camel] davsclaus merged pull request #11449: CAMEL-17173 Mute server errors by default

Posted by "davsclaus (via GitHub)" <gi...@apache.org>.
davsclaus merged PR #11449:
URL: https://github.com/apache/camel/pull/11449


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [camel] davsclaus commented on pull request #11449: CAMEL-17173 Mute server errors by default

Posted by "davsclaus (via GitHub)" <gi...@apache.org>.
davsclaus commented on PR #11449:
URL: https://github.com/apache/camel/pull/11449#issuecomment-1725268438

   Such a change needs to be documented in the 4.1 upgrade guide in the docs folder


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [camel] orpiske commented on a diff in pull request #11449: CAMEL-17173 Mute server errors by default

Posted by "orpiske (via GitHub)" <gi...@apache.org>.
orpiske commented on code in PR #11449:
URL: https://github.com/apache/camel/pull/11449#discussion_r1328717403


##########
components/camel-jetty/src/test/java/org/apache/camel/component/jetty/JettyMuteExceptionTest.java:
##########
@@ -41,13 +41,29 @@ public void testMuteException() throws Exception {
         }
     }
 
+    @Test
+    public void testDefaultMuteException() throws Exception {
+        HttpGet get = new HttpGet("http://localhost:" + getPort() + "/fooDefault");
+        get.addHeader("Accept", "application/text");
+        try (CloseableHttpClient client = HttpClients.createDefault();
+             CloseableHttpResponse response = client.execute(get)) {
+
+            String responseString = EntityUtils.toString(response.getEntity(), "UTF-8");
+            assertEquals("", responseString);

Review Comment:
   Maybe use `assertTrue(responseString.isEmpty())` ?



##########
components/camel-jetty/src/test/java/org/apache/camel/component/jetty/JettyMuteExceptionTest.java:
##########
@@ -41,13 +41,29 @@ public void testMuteException() throws Exception {
         }
     }
 
+    @Test
+    public void testDefaultMuteException() throws Exception {
+        HttpGet get = new HttpGet("http://localhost:" + getPort() + "/fooDefault");
+        get.addHeader("Accept", "application/text");
+        try (CloseableHttpClient client = HttpClients.createDefault();
+             CloseableHttpResponse response = client.execute(get)) {
+
+            String responseString = EntityUtils.toString(response.getEntity(), "UTF-8");

Review Comment:
   Use `StandardCharsets.UTF_8` 
   
   Obs.: I know the other test in there uses "UTF-8" (this is an old practice - we still have a few places doing this).



##########
components/camel-servlet/src/test/java/org/apache/camel/component/servlet/ServletMuteExceptionTest.java:
##########
@@ -25,6 +25,18 @@
 
 public class ServletMuteExceptionTest extends ServletCamelRouterTestSupport {
 
+    @Test
+    public void testMuteDefaultTrue() throws Exception {
+        WebRequest req = new PostMethodWebRequest(
+                contextUrl + "/services/muteDefault",
+                new ByteArrayInputStream("".getBytes()), "text/plain");
+        WebResponse response = query(req, false);
+
+        assertEquals(500, response.getResponseCode());
+        assertEquals("text/plain", response.getContentType());
+        assertEquals("", response.getText());

Review Comment:
   Same note I raised on another text.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [camel] jono-morris commented on a diff in pull request #11449: CAMEL-17173 Mute server errors by default

Posted by "jono-morris (via GitHub)" <gi...@apache.org>.
jono-morris commented on code in PR #11449:
URL: https://github.com/apache/camel/pull/11449#discussion_r1328808389


##########
components/camel-jetty/src/test/java/org/apache/camel/component/jetty/JettyMuteExceptionTest.java:
##########
@@ -41,13 +41,29 @@ public void testMuteException() throws Exception {
         }
     }
 
+    @Test
+    public void testDefaultMuteException() throws Exception {
+        HttpGet get = new HttpGet("http://localhost:" + getPort() + "/fooDefault");
+        get.addHeader("Accept", "application/text");
+        try (CloseableHttpClient client = HttpClients.createDefault();
+             CloseableHttpResponse response = client.execute(get)) {
+
+            String responseString = EntityUtils.toString(response.getEntity(), "UTF-8");
+            assertEquals("", responseString);

Review Comment:
   Done, thanks for that.



##########
components/camel-jetty/src/test/java/org/apache/camel/component/jetty/JettyMuteExceptionTest.java:
##########
@@ -41,13 +41,29 @@ public void testMuteException() throws Exception {
         }
     }
 
+    @Test
+    public void testDefaultMuteException() throws Exception {
+        HttpGet get = new HttpGet("http://localhost:" + getPort() + "/fooDefault");
+        get.addHeader("Accept", "application/text");
+        try (CloseableHttpClient client = HttpClients.createDefault();
+             CloseableHttpResponse response = client.execute(get)) {
+
+            String responseString = EntityUtils.toString(response.getEntity(), "UTF-8");

Review Comment:
   Done.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org