Browse Source

[HTTP] add option to only return simple exception messages

Adds a setting to disable detailed error messages and full exception stack traces
in HTTP responses. When set to false, the error_trace request parameter will result
in a HTTP 400 response. When the error_trace parameter is not present, the message
of the first ElasticsearchException will be output and no nested exception messages
will be output.
jaymode 10 years ago
parent
commit
105bdd486a

+ 5 - 0
docs/reference/modules/http.asciidoc

@@ -72,6 +72,11 @@ be cached for. Defaults to `1728000` (20 days)
 header should be returned. Note: This header is only returned, when the setting is
 set to `true`. Defaults to `false`
 
+|`http.detailed_errors.enabled` |Enables or disables the output of detailed error messages
+and stack traces in response output. Note: When set to `false` and the `error_trace` request
+parameter is specified, an error will be returned; when `error_trace` is not specified, a
+simple message will be returned. Defaults to `true`
+
 |`http.pipelining` |Enable or disable HTTP pipelining, defaults to `true`.
 
 |`http.pipelining.max_events` |The maximum number of events to be queued up in memory before a HTTP connection is closed, defaults to `10000`.

+ 2 - 2
src/main/java/org/elasticsearch/http/HttpChannel.java

@@ -27,7 +27,7 @@ import org.elasticsearch.rest.RestRequest;
  */
 public abstract class HttpChannel extends RestChannel {
 
-    protected HttpChannel(RestRequest request) {
-        super(request);
+    protected HttpChannel(RestRequest request, boolean detailedErrorsEnabled) {
+        super(request, detailedErrorsEnabled);
     }
 }

+ 5 - 3
src/main/java/org/elasticsearch/http/netty/HttpRequestHandler.java

@@ -36,11 +36,13 @@ public class HttpRequestHandler extends SimpleChannelUpstreamHandler {
     private final NettyHttpServerTransport serverTransport;
     private final Pattern corsPattern;
     private final boolean httpPipeliningEnabled;
+    private final boolean detailedErrorsEnabled;
 
-    public HttpRequestHandler(NettyHttpServerTransport serverTransport) {
+    public HttpRequestHandler(NettyHttpServerTransport serverTransport, boolean detailedErrorsEnabled) {
         this.serverTransport = serverTransport;
         this.corsPattern = RestUtils.getCorsSettingRegex(serverTransport.settings());
         this.httpPipeliningEnabled = serverTransport.pipelining;
+        this.detailedErrorsEnabled = detailedErrorsEnabled;
     }
 
     @Override
@@ -58,9 +60,9 @@ public class HttpRequestHandler extends SimpleChannelUpstreamHandler {
         // when reading, or using a cumalation buffer
         NettyHttpRequest httpRequest = new NettyHttpRequest(request, e.getChannel());
         if (oue != null) {
-            serverTransport.dispatchRequest(httpRequest, new NettyHttpChannel(serverTransport, httpRequest, corsPattern, oue));
+            serverTransport.dispatchRequest(httpRequest, new NettyHttpChannel(serverTransport, httpRequest, corsPattern, oue, detailedErrorsEnabled));
         } else {
-            serverTransport.dispatchRequest(httpRequest, new NettyHttpChannel(serverTransport, httpRequest, corsPattern));
+            serverTransport.dispatchRequest(httpRequest, new NettyHttpChannel(serverTransport, httpRequest, corsPattern, detailedErrorsEnabled));
         }
         super.messageReceived(ctx, e);
     }

+ 4 - 4
src/main/java/org/elasticsearch/http/netty/NettyHttpChannel.java

@@ -64,16 +64,16 @@ public class NettyHttpChannel extends HttpChannel {
     private OrderedUpstreamMessageEvent orderedUpstreamMessageEvent = null;
     private Pattern corsPattern;
 
-    public NettyHttpChannel(NettyHttpServerTransport transport, NettyHttpRequest request, Pattern corsPattern) {
-        super(request);
+    public NettyHttpChannel(NettyHttpServerTransport transport, NettyHttpRequest request, Pattern corsPattern, boolean detailedErrorsEnabled) {
+        super(request, detailedErrorsEnabled);
         this.transport = transport;
         this.channel = request.getChannel();
         this.nettyRequest = request.request();
         this.corsPattern = corsPattern;
     }
 
-    public NettyHttpChannel(NettyHttpServerTransport transport, NettyHttpRequest request, Pattern corsPattern, OrderedUpstreamMessageEvent orderedUpstreamMessageEvent) {
-        this(transport, request, corsPattern);
+    public NettyHttpChannel(NettyHttpServerTransport transport, NettyHttpRequest request, Pattern corsPattern, OrderedUpstreamMessageEvent orderedUpstreamMessageEvent, boolean detailedErrorsEnabled) {
+        this(transport, request, corsPattern, detailedErrorsEnabled);
         this.orderedUpstreamMessageEvent = orderedUpstreamMessageEvent;
     }
 

+ 7 - 3
src/main/java/org/elasticsearch/http/netty/NettyHttpServerTransport.java

@@ -77,6 +77,7 @@ public class NettyHttpServerTransport extends AbstractLifecycleComponent<HttpSer
     public static final String SETTING_PIPELINING_MAX_EVENTS = "http.pipelining.max_events";
     public static final String SETTING_HTTP_COMPRESSION = "http.compression";
     public static final String SETTING_HTTP_COMPRESSION_LEVEL = "http.compression_level";
+    public static final String SETTING_HTTP_DETAILED_ERRORS_ENABLED = "http.detailed_errors.enabled";
 
     public static final boolean DEFAULT_SETTING_PIPELINING = true;
     public static final int DEFAULT_SETTING_PIPELINING_MAX_EVENTS = 10000;
@@ -109,6 +110,8 @@ public class NettyHttpServerTransport extends AbstractLifecycleComponent<HttpSer
 
     protected final String publishHost;
 
+    protected final boolean detailedErrorsEnabled;
+
     protected int publishPort;
 
     protected final String tcpNoDelay;
@@ -162,6 +165,7 @@ public class NettyHttpServerTransport extends AbstractLifecycleComponent<HttpSer
         this.reuseAddress = settings.getAsBoolean("http.netty.reuse_address", settings.getAsBoolean(TCP_REUSE_ADDRESS, NetworkUtils.defaultReuseAddress()));
         this.tcpSendBufferSize = settings.getAsBytesSize("http.netty.tcp_send_buffer_size", settings.getAsBytesSize(TCP_SEND_BUFFER_SIZE, TCP_DEFAULT_SEND_BUFFER_SIZE));
         this.tcpReceiveBufferSize = settings.getAsBytesSize("http.netty.tcp_receive_buffer_size", settings.getAsBytesSize(TCP_RECEIVE_BUFFER_SIZE, TCP_DEFAULT_RECEIVE_BUFFER_SIZE));
+        this.detailedErrorsEnabled = settings.getAsBoolean(SETTING_HTTP_DETAILED_ERRORS_ENABLED, true);
 
         long defaultReceiverPredictor = 512 * 1024;
         if (JvmInfo.jvmInfo().mem().directMemoryMax().bytes() > 0) {
@@ -349,7 +353,7 @@ public class NettyHttpServerTransport extends AbstractLifecycleComponent<HttpSer
     }
 
     public ChannelPipelineFactory configureServerChannelPipelineFactory() {
-        return new HttpChannelPipelineFactory(this);
+        return new HttpChannelPipelineFactory(this, detailedErrorsEnabled);
     }
 
     protected static class HttpChannelPipelineFactory implements ChannelPipelineFactory {
@@ -357,9 +361,9 @@ public class NettyHttpServerTransport extends AbstractLifecycleComponent<HttpSer
         protected final NettyHttpServerTransport transport;
         protected final HttpRequestHandler requestHandler;
 
-        public HttpChannelPipelineFactory(NettyHttpServerTransport transport) {
+        public HttpChannelPipelineFactory(NettyHttpServerTransport transport, boolean detailedErrorsEnabled) {
             this.transport = transport;
-            this.requestHandler = new HttpRequestHandler(transport);
+            this.requestHandler = new HttpRequestHandler(transport, detailedErrorsEnabled);
         }
 
         @Override

+ 49 - 23
src/main/java/org/elasticsearch/rest/BytesRestResponse.java

@@ -120,34 +120,44 @@ public class BytesRestResponse extends RestResponse {
     }
 
     private static XContentBuilder convert(RestChannel channel, RestStatus status, Throwable t) throws IOException {
-        XContentBuilder builder = channel.newBuilder().startObject()
-                .field("error", detailedMessage(t))
-                .field("status", status.getStatus());
-        if (t != null && channel.request().paramAsBoolean("error_trace", false)) {
-            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;
+        XContentBuilder builder = channel.newBuilder().startObject();
+        if (t == null) {
+            builder.field("error", "Unknown");
+        } else if (channel.detailedErrorsEnabled()) {
+            builder.field("error", detailedMessage(t));
+            if (channel.request().paramAsBoolean("error_trace", false)) {
+                buildErrorTrace(t, builder);
             }
-            builder.endObject();
+        } else {
+            builder.field("error", simpleMessage(t));
         }
+        builder.field("status", status.getStatus());
         builder.endObject();
         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()) {
@@ -163,4 +173,20 @@ public class BytesRestResponse extends RestResponse {
             builder.endObject();
         }
     }
-}
+
+    /*
+     * Builds a simple error string from the message of the first ElasticsearchException
+     */
+    private static String simpleMessage(Throwable t) throws IOException {
+        int counter = 0;
+        Throwable next = t;
+        while (next != null && counter++ < 10) {
+            if (t instanceof ElasticsearchException) {
+                return next.getClass().getSimpleName() + "[" + next.getMessage() + "]";
+            }
+            next = next.getCause();
+        }
+
+        return "No ElasticsearchException found";
+    }
+}

+ 7 - 1
src/main/java/org/elasticsearch/rest/RestChannel.java

@@ -34,11 +34,13 @@ import java.io.IOException;
 public abstract class RestChannel {
 
     protected final RestRequest request;
+    protected final boolean detailedErrorsEnabled;
 
     private BytesStreamOutput bytesOut;
 
-    protected RestChannel(RestRequest request) {
+    protected RestChannel(RestRequest request, boolean detailedErrorsEnabled) {
         this.request = request;
+        this.detailedErrorsEnabled = detailedErrorsEnabled;
     }
 
     public XContentBuilder newBuilder() throws IOException {
@@ -96,5 +98,9 @@ public abstract class RestChannel {
         return this.request;
     }
 
+    public boolean detailedErrorsEnabled() {
+        return detailedErrorsEnabled;
+    }
+
     public abstract void sendResponse(RestResponse response);
 }

+ 40 - 12
src/main/java/org/elasticsearch/rest/RestController.java

@@ -161,20 +161,10 @@ public class RestController extends AbstractLifecycleComponent<RestController> {
     }
 
     public void dispatchRequest(final RestRequest request, final RestChannel channel) {
-        // If JSONP is disabled and someone sends a callback parameter we should bail out before querying
-        if (!settings.getAsBoolean(HTTP_JSON_ENABLE, false) && request.hasParam("callback")){
-            try {
-                XContentBuilder builder = channel.newBuilder();
-                builder.startObject().field("error","JSONP is disabled.").endObject().string();
-                RestResponse response = new BytesRestResponse(FORBIDDEN, builder);
-                response.addHeader("Content-Type", "application/javascript");
-                channel.sendResponse(response);
-            } catch (IOException e) {
-                logger.warn("Failed to send response", e);
-                return;
-            }
+        if (!checkRequestParameters(request, channel)) {
             return;
         }
+
         if (filters.length == 0) {
             try {
                 executeHandler(request, channel);
@@ -191,6 +181,44 @@ public class RestController extends AbstractLifecycleComponent<RestController> {
         }
     }
 
+    /**
+     * Checks the request parameters against enabled settings for JSONP and error trace support
+     * @param request
+     * @param channel
+     * @return true if the request does not have any parameters that conflict with system settings
+     */
+    boolean checkRequestParameters(final RestRequest request, final RestChannel channel) {
+        // If JSONP is disabled and someone sends a callback parameter we should bail out before querying
+        if (!settings.getAsBoolean(HTTP_JSON_ENABLE, false) && request.hasParam("callback")) {
+            try {
+                XContentBuilder builder = channel.newBuilder();
+                builder.startObject().field("error","JSONP is disabled.").endObject().string();
+                RestResponse response = new BytesRestResponse(FORBIDDEN, builder);
+                response.addHeader("Content-Type", "application/javascript");
+                channel.sendResponse(response);
+            } catch (IOException e) {
+                logger.warn("Failed to send response", e);
+            }
+            return false;
+        }
+
+        // error_trace cannot be used when we disable detailed errors
+        if (channel.detailedErrorsEnabled() == false && request.paramAsBoolean("error_trace", false)) {
+            try {
+                XContentBuilder builder = channel.newBuilder();
+                builder.startObject().field("error","error traces in responses are disabled.").endObject().string();
+                RestResponse response = new BytesRestResponse(BAD_REQUEST, builder);
+                response.addHeader("Content-Type", "application/json");
+                channel.sendResponse(response);
+            } catch (IOException e) {
+                logger.warn("Failed to send response", e);
+            }
+            return false;
+        }
+
+        return true;
+    }
+
     void executeHandler(RestRequest request, RestChannel channel) throws Exception {
         final RestHandler handler = getHandler(request);
         if (handler != null) {

+ 1 - 1
src/test/java/org/elasticsearch/http/netty/NettyHttpServerPipeliningTest.java

@@ -147,7 +147,7 @@ public class NettyHttpServerPipeliningTest extends ElasticsearchTestCase {
         private final ExecutorService executorService;
 
         public CustomHttpChannelPipelineFactory(NettyHttpServerTransport transport, ExecutorService executorService) {
-            super(transport);
+            super(transport, randomBoolean());
             this.executorService = executorService;
         }
 

+ 67 - 0
src/test/java/org/elasticsearch/options/detailederrors/DetailedErrorsDisabledTest.java

@@ -0,0 +1,67 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.options.detailederrors;
+
+import org.apache.http.impl.client.HttpClients;
+import org.elasticsearch.common.settings.ImmutableSettings;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.http.HttpServerTransport;
+import org.elasticsearch.http.netty.NettyHttpServerTransport;
+import org.elasticsearch.node.Node;
+import org.elasticsearch.test.ElasticsearchIntegrationTest;
+import org.elasticsearch.test.ElasticsearchIntegrationTest.ClusterScope;
+import org.elasticsearch.test.ElasticsearchIntegrationTest.Scope;
+import org.elasticsearch.test.rest.client.http.HttpDeleteWithEntity;
+import org.elasticsearch.test.rest.client.http.HttpRequestBuilder;
+import org.elasticsearch.test.rest.client.http.HttpResponse;
+import org.junit.Test;
+
+import static org.hamcrest.Matchers.is;
+
+/**
+ * Tests that when disabling detailed errors, a request with the error_trace parameter returns a HTTP 400
+ */
+@ClusterScope(scope = Scope.TEST, numDataNodes = 1)
+public class DetailedErrorsDisabledTest extends ElasticsearchIntegrationTest {
+
+    // Build our cluster settings
+    @Override
+    protected Settings nodeSettings(int nodeOrdinal) {
+        return ImmutableSettings.settingsBuilder()
+                .put(super.nodeSettings(nodeOrdinal))
+                .put(Node.HTTP_ENABLED, true)
+                .put(NettyHttpServerTransport.SETTING_HTTP_DETAILED_ERRORS_ENABLED, false)
+                .build();
+    }
+
+    @Test
+    public void testThatErrorTraceParamReturns400() throws Exception {
+        // Make the HTTP request
+        HttpResponse response = new HttpRequestBuilder(HttpClients.createDefault())
+                .httpTransport(internalCluster().getDataNodeInstance(HttpServerTransport.class))
+                .addParam("error_trace", "true")
+                .method(HttpDeleteWithEntity.METHOD_NAME)
+                .execute();
+
+        assertThat(response.getHeaders().get("Content-Type"), is("application/json"));
+        assertThat(response.getBody(), is("{\"error\":\"error traces in responses are disabled.\"}"));
+        assertThat(response.getStatusCode(), is(400));
+    }
+}

+ 65 - 0
src/test/java/org/elasticsearch/options/detailederrors/DetailedErrorsEnabledTest.java

@@ -0,0 +1,65 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.options.detailederrors;
+
+import org.apache.http.impl.client.HttpClients;
+import org.elasticsearch.common.settings.ImmutableSettings;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.http.HttpServerTransport;
+import org.elasticsearch.node.Node;
+import org.elasticsearch.test.ElasticsearchIntegrationTest;
+import org.elasticsearch.test.ElasticsearchIntegrationTest.ClusterScope;
+import org.elasticsearch.test.ElasticsearchIntegrationTest.Scope;
+import org.elasticsearch.test.rest.client.http.HttpDeleteWithEntity;
+import org.elasticsearch.test.rest.client.http.HttpRequestBuilder;
+import org.elasticsearch.test.rest.client.http.HttpResponse;
+import org.junit.Test;
+
+import static org.hamcrest.Matchers.containsString;
+
+/**
+ * Tests that by default the error_trace parameter can be used to show stacktraces
+ */
+@ClusterScope(scope = Scope.TEST, numDataNodes = 1)
+public class DetailedErrorsEnabledTest extends ElasticsearchIntegrationTest {
+
+    // Build our cluster settings
+    @Override
+    protected Settings nodeSettings(int nodeOrdinal) {
+        return ImmutableSettings.settingsBuilder()
+                .put(super.nodeSettings(nodeOrdinal))
+                .put(Node.HTTP_ENABLED, true)
+                .build();
+    }
+
+    @Test
+    public void testThatErrorTraceWorksByDefault() throws Exception {
+        // Make the HTTP request
+        HttpResponse response = new HttpRequestBuilder(HttpClients.createDefault())
+                .httpTransport(internalCluster().getDataNodeInstance(HttpServerTransport.class))
+                .path("/")
+                .addParam("error_trace", "true")
+                .method(HttpDeleteWithEntity.METHOD_NAME)
+                .execute();
+
+        assertThat(response.getHeaders().get("Content-Type"), containsString("application/json"));
+        assertThat(response.getBody(), containsString("\"error_trace\":{\"message\":\"Validation Failed"));
+    }
+}

+ 92 - 5
src/test/java/org/elasticsearch/rest/BytesRestResponseTests.java

@@ -23,7 +23,11 @@ import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.test.ElasticsearchTestCase;
 import org.junit.Test;
 
+import java.io.FileNotFoundException;
+
 import static org.hamcrest.Matchers.contains;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.not;
 import static org.hamcrest.Matchers.notNullValue;
 
 /**
@@ -34,11 +38,8 @@ public class BytesRestResponseTests extends ElasticsearchTestCase {
     @Test
     public void testWithHeaders() throws Exception {
         RestRequest request = new FakeRestRequest();
-        RestChannel channel = new RestChannel(request) {
-            @Override
-            public void sendResponse(RestResponse response) {
-            }
-        };
+        RestChannel channel = randomBoolean() ? new DetailedExceptionRestChannel(request) : new SimpleExceptionRestChannel(request);
+
         BytesRestResponse response = new BytesRestResponse(channel, new ExceptionWithHeaders());
         assertThat(response.getHeaders().get("n1"), notNullValue());
         assertThat(response.getHeaders().get("n1"), contains("v11", "v12"));
@@ -46,6 +47,70 @@ public class BytesRestResponseTests extends ElasticsearchTestCase {
         assertThat(response.getHeaders().get("n2"), contains("v21", "v22"));
     }
 
+    @Test
+    public void testSimpleExceptionMessage() throws Exception {
+        RestRequest request = new FakeRestRequest();
+        RestChannel channel = new SimpleExceptionRestChannel(request);
+
+        Throwable t = new ElasticsearchException("an error occurred reading data", new FileNotFoundException("/foo/bar"));
+        BytesRestResponse response = new BytesRestResponse(channel, t);
+        String text = response.content().toUtf8();
+        assertThat(text, containsString("ElasticsearchException[an error occurred reading data]"));
+        assertThat(text, not(containsString("FileNotFoundException")));
+        assertThat(text, not(containsString("/foo/bar")));
+        assertThat(text, not(containsString("error_trace")));
+    }
+
+    @Test
+    public void testDetailedExceptionMessage() throws Exception {
+        RestRequest request = new FakeRestRequest();
+        RestChannel channel = new DetailedExceptionRestChannel(request);
+
+        Throwable t = new ElasticsearchException("an error occurred reading data", new FileNotFoundException("/foo/bar"));
+        BytesRestResponse response = new BytesRestResponse(channel, t);
+        String text = response.content().toUtf8();
+        assertThat(text, containsString("ElasticsearchException[an error occurred reading data]"));
+        assertThat(text, containsString("FileNotFoundException[/foo/bar]"));
+    }
+
+    @Test
+    public void testNonElasticsearchExceptionIsNotShownAsSimpleMessage() throws Exception {
+        RestRequest request = new FakeRestRequest();
+        RestChannel channel = new SimpleExceptionRestChannel(request);
+
+        Throwable t = new Throwable("an error occurred reading data", new FileNotFoundException("/foo/bar"));
+        BytesRestResponse response = new BytesRestResponse(channel, t);
+        String text = response.content().toUtf8();
+        assertThat(text, not(containsString("Throwable[an error occurred reading data]")));
+        assertThat(text, not(containsString("FileNotFoundException[/foo/bar]")));
+        assertThat(text, not(containsString("error_trace")));
+        assertThat(text, containsString("\"error\":\"No ElasticsearchException found\""));
+    }
+
+    @Test
+    public void testErrorTrace() throws Exception {
+        RestRequest request = new FakeRestRequest();
+        request.params().put("error_trace", "true");
+        RestChannel channel = new DetailedExceptionRestChannel(request);
+
+        Throwable t = new Throwable("an error occurred reading data", new FileNotFoundException("/foo/bar"));
+        BytesRestResponse response = new BytesRestResponse(channel, t);
+        String text = response.content().toUtf8();
+        assertThat(text, containsString("\"error\":\"Throwable[an error occurred reading data]"));
+        assertThat(text, containsString("FileNotFoundException[/foo/bar]"));
+        assertThat(text, containsString("\"error_trace\":{\"message\":\"an error occurred reading data\""));
+    }
+
+    @Test
+    public void testNullThrowable() throws Exception {
+        RestRequest request = new FakeRestRequest();
+        RestChannel channel = new SimpleExceptionRestChannel(request);
+
+        BytesRestResponse response = new BytesRestResponse(channel, null);
+        String text = response.content().toUtf8();
+        assertThat(text, containsString("\"error\":\"Unknown\""));
+        assertThat(text, not(containsString("error_trace")));
+    }
 
     private static class ExceptionWithHeaders extends ElasticsearchException.WithRestHeaders {
 
@@ -53,4 +118,26 @@ public class BytesRestResponseTests extends ElasticsearchTestCase {
             super("", header("n1", "v11", "v12"), header("n2", "v21", "v22"));
         }
     }
+
+    private static class SimpleExceptionRestChannel extends RestChannel {
+
+        SimpleExceptionRestChannel(RestRequest request) {
+            super(request, false);
+        }
+
+        @Override
+        public void sendResponse(RestResponse response) {
+        }
+    }
+
+    private static class DetailedExceptionRestChannel extends RestChannel {
+
+        DetailedExceptionRestChannel(RestRequest request) {
+            super(request, true);
+        }
+
+        @Override
+        public void sendResponse(RestResponse response) {
+        }
+    }
 }

+ 1 - 1
src/test/java/org/elasticsearch/rest/RestFilterChainTests.java

@@ -157,7 +157,7 @@ public class RestFilterChainTests extends ElasticsearchTestCase {
         AtomicInteger errors = new AtomicInteger();
 
         protected FakeRestChannel(RestRequest request, int responseCount) {
-            super(request);
+            super(request, randomBoolean());
             this.latch = new CountDownLatch(responseCount);
         }