You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by el...@apache.org on 2016/05/09 14:08:38 UTC
calcite git commit: [CALCITE-1218] Lift try/catch to Jetty Handler to
prevent exceptions propagating to Jetty [Forced Update!]
Repository: calcite
Updated Branches:
refs/heads/master fa87c0621 -> 9deff987b (forced update)
[CALCITE-1218] Lift try/catch to Jetty Handler to prevent exceptions propagating to Jetty
We need to make sure that we always send back an ErrorResponse to the
client when we can't execute their Request (for whatever reason).
I tried to write a test to exercise this, but ran into tons of issues
trying to do this because of the relocation of protobuf classes. This
will be alleviated by CALCITE-1224 in the future.
Project: http://git-wip-us.apache.org/repos/asf/calcite/repo
Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/9deff987
Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/9deff987
Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/9deff987
Branch: refs/heads/master
Commit: 9deff987ba177a6eb1ded2813e770a9233f3f2f4
Parents: ebb68da
Author: Josh Elser <el...@apache.org>
Authored: Sun May 8 23:26:07 2016 -0400
Committer: Josh Elser <el...@apache.org>
Committed: Mon May 9 10:08:19 2016 -0400
----------------------------------------------------------------------
.../calcite/avatica/remote/AbstractHandler.java | 2 +-
.../avatica/remote/AbstractHandlerTest.java | 2 ++
.../calcite/avatica/server/AvaticaJsonHandler.java | 15 ++++++++-------
.../avatica/server/AvaticaProtobufHandler.java | 17 +++++++++--------
4 files changed, 20 insertions(+), 16 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/calcite/blob/9deff987/avatica/core/src/main/java/org/apache/calcite/avatica/remote/AbstractHandler.java
----------------------------------------------------------------------
diff --git a/avatica/core/src/main/java/org/apache/calcite/avatica/remote/AbstractHandler.java b/avatica/core/src/main/java/org/apache/calcite/avatica/remote/AbstractHandler.java
index 0f8770b..c37e063 100644
--- a/avatica/core/src/main/java/org/apache/calcite/avatica/remote/AbstractHandler.java
+++ b/avatica/core/src/main/java/org/apache/calcite/avatica/remote/AbstractHandler.java
@@ -106,7 +106,7 @@ public abstract class AbstractHandler<T> implements Handler<T> {
* @param e The exception to convert.
* @return A HandlerResponse instance.
*/
- private HandlerResponse<T> convertToErrorResponse(Exception e) {
+ public HandlerResponse<T> convertToErrorResponse(Exception e) {
ErrorResponse errorResp = unwrapException(e);
try {
http://git-wip-us.apache.org/repos/asf/calcite/blob/9deff987/avatica/core/src/test/java/org/apache/calcite/avatica/remote/AbstractHandlerTest.java
----------------------------------------------------------------------
diff --git a/avatica/core/src/test/java/org/apache/calcite/avatica/remote/AbstractHandlerTest.java b/avatica/core/src/test/java/org/apache/calcite/avatica/remote/AbstractHandlerTest.java
index 16402d8..5460f93 100644
--- a/avatica/core/src/test/java/org/apache/calcite/avatica/remote/AbstractHandlerTest.java
+++ b/avatica/core/src/test/java/org/apache/calcite/avatica/remote/AbstractHandlerTest.java
@@ -107,6 +107,7 @@ public class AbstractHandlerTest {
Mockito.when(request.accept(Mockito.any(Service.class))).thenReturn(response);
// Throw an IOException when serializing the Response.
Mockito.when(handler.encode(response)).thenThrow(exception);
+ Mockito.when(handler.convertToErrorResponse(exception)).thenCallRealMethod();
// Convert the IOException into an ErrorResponse
Mockito.when(handler.unwrapException(exception)).thenReturn(errorResponse);
Mockito.when(handler.encode(errorResponse)).thenReturn(serializedErrorResponse);
@@ -155,6 +156,7 @@ public class AbstractHandlerTest {
Mockito.when(handler.apply(Mockito.anyString())).thenCallRealMethod();
// Throw an Exception trying to convert it back into a POJO
Mockito.when(handler.decode(Mockito.anyString())).thenThrow(exception);
+ Mockito.when(handler.convertToErrorResponse(exception)).thenCallRealMethod();
Mockito.when(handler.unwrapException(exception)).thenReturn(errorResponse);
Mockito.when(handler.encode(errorResponse)).thenReturn(serializedErrorResponse);
http://git-wip-us.apache.org/repos/asf/calcite/blob/9deff987/avatica/server/src/main/java/org/apache/calcite/avatica/server/AvaticaJsonHandler.java
----------------------------------------------------------------------
diff --git a/avatica/server/src/main/java/org/apache/calcite/avatica/server/AvaticaJsonHandler.java b/avatica/server/src/main/java/org/apache/calcite/avatica/server/AvaticaJsonHandler.java
index f5c939c..b322d66 100644
--- a/avatica/server/src/main/java/org/apache/calcite/avatica/server/AvaticaJsonHandler.java
+++ b/avatica/server/src/main/java/org/apache/calcite/avatica/server/AvaticaJsonHandler.java
@@ -116,21 +116,22 @@ public class AvaticaJsonHandler extends AbstractAvaticaHandler {
new String(rawRequest.getBytes("ISO-8859-1"), "UTF-8");
LOG.trace("request: {}", jsonRequest);
- final HandlerResponse<String> jsonResponse;
- if (null != serverConfig && serverConfig.supportsImpersonation()) {
- try {
+ HandlerResponse<String> jsonResponse;
+ try {
+ if (null != serverConfig && serverConfig.supportsImpersonation()) {
jsonResponse = serverConfig.doAsRemoteUser(request.getRemoteUser(),
request.getRemoteAddr(), new Callable<HandlerResponse<String>>() {
@Override public HandlerResponse<String> call() {
return jsonHandler.apply(jsonRequest);
}
});
- } catch (Exception e) {
- throw new RuntimeException(e);
+ } else {
+ jsonResponse = jsonHandler.apply(jsonRequest);
}
- } else {
- jsonResponse = jsonHandler.apply(jsonRequest);
+ } catch (Exception e) {
+ jsonResponse = jsonHandler.convertToErrorResponse(e);
}
+
LOG.trace("response: {}", jsonResponse);
baseRequest.setHandled(true);
// Set the status code and write out the response.
http://git-wip-us.apache.org/repos/asf/calcite/blob/9deff987/avatica/server/src/main/java/org/apache/calcite/avatica/server/AvaticaProtobufHandler.java
----------------------------------------------------------------------
diff --git a/avatica/server/src/main/java/org/apache/calcite/avatica/server/AvaticaProtobufHandler.java b/avatica/server/src/main/java/org/apache/calcite/avatica/server/AvaticaProtobufHandler.java
index a465ba1..7b08b0b 100644
--- a/avatica/server/src/main/java/org/apache/calcite/avatica/server/AvaticaProtobufHandler.java
+++ b/avatica/server/src/main/java/org/apache/calcite/avatica/server/AvaticaProtobufHandler.java
@@ -110,21 +110,22 @@ public class AvaticaProtobufHandler extends AbstractAvaticaHandler {
buffer.reset();
}
- final HandlerResponse<byte[]> handlerResponse;
- if (null != serverConfig && serverConfig.supportsImpersonation()) {
- // Invoke the ProtobufHandler inside as doAs for the remote user.
- try {
+ HandlerResponse<byte[]> handlerResponse;
+ try {
+ if (null != serverConfig && serverConfig.supportsImpersonation()) {
+ // Invoke the ProtobufHandler inside as doAs for the remote user.
handlerResponse = serverConfig.doAsRemoteUser(request.getRemoteUser(),
request.getRemoteAddr(), new Callable<HandlerResponse<byte[]>>() {
@Override public HandlerResponse<byte[]> call() {
return pbHandler.apply(requestBytes);
}
});
- } catch (Exception e) {
- throw new RuntimeException(e);
+ } else {
+ handlerResponse = pbHandler.apply(requestBytes);
}
- } else {
- handlerResponse = pbHandler.apply(requestBytes);
+ } catch (Exception e) {
+ // Catch at the highest level of exceptions
+ handlerResponse = pbHandler.convertToErrorResponse(e);
}
baseRequest.setHandled(true);