Suppress rest exceptions by default and log them instead

Today we are very verbose when rendering exceptions on the rest layer.
Yet, this isn't necessarily very easy to read and way too much infromation most
of the time. This change suppresses the stacktrace rendering by default but instead
adds a `rest.suppressed` logger that logs the suppressed stacktrace or rather the entire
exception on the node that renderes the exception
The log message looks like this:

```
[2015-08-19 16:21:58,427][INFO ][rest.suppressed          ] /test/_search/ Params: {index=test}
[test] IndexNotFoundException[no such index]
	at org.elasticsearch.cluster.metadata.IndexNameExpressionResolver$WildcardExpressionResolver.resolve(IndexNameExpressionResolver.java:551)
```

core/src/main/java/org/elasticsearch/ElasticsearchException.java

@@ -45,8 +45,8 @@ public class ElasticsearchException extends RuntimeException implements ToXConte
 
     public static final String REST_EXCEPTION_SKIP_CAUSE = "rest.exception.cause.skip";
     public static final String REST_EXCEPTION_SKIP_STACK_TRACE = "rest.exception.stacktrace.skip";
-    private static final boolean REST_EXCEPTION_SKIP_STACK_TRACE_DEFAULT = false;
+    public static final boolean REST_EXCEPTION_SKIP_STACK_TRACE_DEFAULT = true;
-    private static final boolean REST_EXCEPTION_SKIP_CAUSE_DEFAULT = false;
+    public static final boolean REST_EXCEPTION_SKIP_CAUSE_DEFAULT = false;
     private static final String INDEX_HEADER_KEY = "es.index";
     private static final String SHARD_HEADER_KEY = "es.shard";
     private static final String RESOURCE_HEADER_TYPE_KEY = "es.resource.type";

core/src/main/java/org/elasticsearch/rest/BytesRestResponse.java

@@ -25,6 +25,8 @@ import org.elasticsearch.bootstrap.Elasticsearch;
 import org.elasticsearch.common.bytes.BytesArray;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.collect.Tuple;
+import org.elasticsearch.common.logging.ESLogger;
+import org.elasticsearch.common.logging.ESLoggerFactory;
 import org.elasticsearch.common.xcontent.ToXContent;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 
@@ -115,11 +117,20 @@ public class BytesRestResponse extends RestResponse {
         return this.status;
     }
 
+    private static final ESLogger SUPPRESSED_ERROR_LOGGER = ESLoggerFactory.getLogger("rest.suppressed");
+
     private static XContentBuilder convert(RestChannel channel, RestStatus status, Throwable t) throws IOException {
         XContentBuilder builder = channel.newErrorBuilder().startObject();
         if (t == null) {
             builder.field("error", "unknown");
         } else if (channel.detailedErrorsEnabled()) {
+            final ToXContent.Params params;
+            if (channel.request().paramAsBoolean("error_trace", !ElasticsearchException.REST_EXCEPTION_SKIP_STACK_TRACE_DEFAULT)) {
+                params =  new ToXContent.DelegatingMapParams(Collections.singletonMap(ElasticsearchException.REST_EXCEPTION_SKIP_STACK_TRACE, "false"), channel.request());
+            } else {
+                SUPPRESSED_ERROR_LOGGER.info("{} Params: {}", t, channel.request().path(), channel.request().params());
+                params = channel.request();
+            }
             builder.field("error");
             builder.startObject();
             final ElasticsearchException[] rootCauses = ElasticsearchException.guessRootCauses(t);
@@ -127,16 +138,13 @@ public class BytesRestResponse extends RestResponse {
             builder.startArray();
             for (ElasticsearchException rootCause : rootCauses){
                 builder.startObject();
-                rootCause.toXContent(builder, new ToXContent.DelegatingMapParams(Collections.singletonMap(ElasticsearchException.REST_EXCEPTION_SKIP_CAUSE, "true"), channel.request()));
+                rootCause.toXContent(builder, new ToXContent.DelegatingMapParams(Collections.singletonMap(ElasticsearchException.REST_EXCEPTION_SKIP_CAUSE, "true"), params));
                 builder.endObject();
             }
             builder.endArray();
 
-            ElasticsearchException.toXContent(builder, channel.request(), t);
+            ElasticsearchException.toXContent(builder, params, t);
             builder.endObject();
-            if (channel.request().paramAsBoolean("error_trace", false)) {
-                buildErrorTrace(t, builder);
-            }
         } else {
             builder.field("error", simpleMessage(t));
         }
@@ -145,45 +153,6 @@ public class BytesRestResponse extends RestResponse {
         return builder;
     }
 
-
-    private static void buildErrorTrace(Throwable t, XContentBuilder builder) throws IOException {
-        builder.startObject("error_trace");
-        boolean first = true;
-        int counter = 0;
-        while (t != null) {
-            // bail if there are more than 10 levels, becomes useless really...
-            if (counter++ > 10) {
-                break;
-            }
-            if (!first) {
-                builder.startObject("cause");
-            }
-            buildThrowable(t, builder);
-            if (!first) {
-                builder.endObject();
-            }
-            t = t.getCause();
-            first = false;
-        }
-        builder.endObject();
-    }
-
-    private static void buildThrowable(Throwable t, XContentBuilder builder) throws IOException {
-        builder.field("message", t.getMessage());
-        for (StackTraceElement stElement : t.getStackTrace()) {
-            builder.startObject("at")
-                    .field("class", stElement.getClassName())
-                    .field("method", stElement.getMethodName());
-            if (stElement.getFileName() != null) {
-                builder.field("file", stElement.getFileName());
-            }
-            if (stElement.getLineNumber() >= 0) {
-                builder.field("line", stElement.getLineNumber());
-            }
-            builder.endObject();
-        }
-    }
-
     /*
      * Builds a simple error string from the message of the first ElasticsearchException
      */

core/src/test/java/org/elasticsearch/ESExceptionTests.java

@@ -57,7 +57,7 @@ import java.util.Collections;
 import static org.hamcrest.Matchers.equalTo;
 
 public class ESExceptionTests extends ESTestCase {
-    private static final ToXContent.Params PARAMS = new ToXContent.MapParams(Collections.singletonMap(ElasticsearchException.REST_EXCEPTION_SKIP_STACK_TRACE, "true"));
+    private static final ToXContent.Params PARAMS = ToXContent.EMPTY_PARAMS;
 
     @Test
     public void testStatus() {

core/src/test/java/org/elasticsearch/ExceptionSerializationTests.java

@@ -526,7 +526,7 @@ public class ExceptionSerializationTests extends ESTestCase {
         try {
             XContentBuilder builder = XContentFactory.jsonBuilder();
             builder.startObject();
-            x.toXContent(builder, new ToXContent.MapParams(Collections.singletonMap(ElasticsearchException.REST_EXCEPTION_SKIP_STACK_TRACE, "true")));
+            x.toXContent(builder, ToXContent.EMPTY_PARAMS);
             builder.endObject();
             return builder.string();
         } catch (IOException e) {

core/src/test/java/org/elasticsearch/action/search/MultiSearchRequestTests.java

@@ -125,7 +125,7 @@ public class MultiSearchRequestTests extends ESTestCase {
     public void testResponseErrorToXContent() throws IOException {
         MultiSearchResponse response = new MultiSearchResponse(new MultiSearchResponse.Item[]{new MultiSearchResponse.Item(null, new IllegalStateException("foobar")), new MultiSearchResponse.Item(null, new IllegalStateException("baaaaaazzzz"))});
         XContentBuilder builder = XContentFactory.jsonBuilder();
-        response.toXContent(builder, new ToXContent.MapParams(Collections.singletonMap(ElasticsearchException.REST_EXCEPTION_SKIP_STACK_TRACE, "true")));
+        response.toXContent(builder, ToXContent.EMPTY_PARAMS);
         assertEquals("\"responses\"[{\"error\":{\"root_cause\":[{\"type\":\"illegal_state_exception\",\"reason\":\"foobar\"}],\"type\":\"illegal_state_exception\",\"reason\":\"foobar\"}},{\"error\":{\"root_cause\":[{\"type\":\"illegal_state_exception\",\"reason\":\"baaaaaazzzz\"}],\"type\":\"illegal_state_exception\",\"reason\":\"baaaaaazzzz\"}}]",
                 builder.string());
     }

core/src/test/java/org/elasticsearch/options/detailederrors/DetailedErrorsEnabledIT.java

@@ -32,6 +32,7 @@ import org.elasticsearch.test.rest.client.http.HttpResponse;
 import org.junit.Test;
 
 import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.not;
 
 /**
  * Tests that by default the error_trace parameter can be used to show stacktraces
@@ -59,6 +60,16 @@ public class DetailedErrorsEnabledIT extends ESIntegTestCase {
                 .execute();
 
         assertThat(response.getHeaders().get("Content-Type"), containsString("application/json"));
-        assertThat(response.getBody(), containsString("\"error_trace\":{\"message\":\"Validation Failed"));
+        assertThat(response.getBody(), containsString("\"stack_trace\":\"[Validation Failed: 1: index / indices is missing;]; nested: ActionRequestValidationException[Validation Failed: 1:"));
+
+        // Make the HTTP request
+        response = new HttpRequestBuilder(HttpClients.createDefault())
+                .httpTransport(internalCluster().getDataNodeInstance(HttpServerTransport.class))
+                .path("/")
+                .method(HttpDeleteWithEntity.METHOD_NAME)
+                .execute();
+
+        assertThat(response.getHeaders().get("Content-Type"), containsString("application/json"));
+        assertThat(response.getBody(), not(containsString("\"stack_trace\":\"[Validation Failed: 1: index / indices is missing;]; nested: ActionRequestValidationException[Validation Failed: 1:")));
     }
 }

core/src/test/java/org/elasticsearch/rest/BytesRestResponseTests.java

@@ -105,8 +105,8 @@ public class BytesRestResponseTests extends ESTestCase {
         BytesRestResponse response = new BytesRestResponse(channel, t);
         String text = response.content().toUtf8();
         assertThat(text, containsString("\"type\":\"throwable\",\"reason\":\"an error occurred reading data\""));
-        assertThat(text, containsString("{\"type\":\"file_not_found_exception\",\"reason\":\"/foo/bar\"}"));
+        assertThat(text, containsString("{\"type\":\"file_not_found_exception\""));
-        assertThat(text, containsString("\"error_trace\":{\"message\":\"an error occurred reading data\""));
+        assertThat(text, containsString("\"stack_trace\":\"[an error occurred reading data]"));
     }
 
     public void testGuessRootCause() throws IOException {
@@ -176,7 +176,6 @@ public class BytesRestResponseTests extends ESTestCase {
 
         DetailedExceptionRestChannel(RestRequest request) {
             super(request, true);
-            request.params().put(ElasticsearchException.REST_EXCEPTION_SKIP_STACK_TRACE, "true");
         }
 
         @Override