calcite-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From els...@apache.org
Subject calcite git commit: CALCITE-1218 Lift try/catch to Jetty Handler to prevent exceptions propagating to Jetty
Date Mon, 09 May 2016 03:30:04 GMT
Repository: calcite
Updated Branches:
  refs/heads/master ebb68dadf -> fa87c0621


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/fa87c062
Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/fa87c062
Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/fa87c062

Branch: refs/heads/master
Commit: fa87c0621fde841d2b1a40b8fb0048ed70ea576a
Parents: ebb68da
Author: Josh Elser <elserj@apache.org>
Authored: Sun May 8 23:26:07 2016 -0400
Committer: Josh Elser <elserj@apache.org>
Committed: Sun May 8 23:26:07 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/fa87c062/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/fa87c062/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/fa87c062/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/fa87c062/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);


Mime
View raw message