THRIFT-3686 Java processor should report internal error on uncaught exception

Client: java

This closes #904
This closes #1428

2 files changed, 27 insertions, 18 deletions

diff --git a/lib/java/src/org/apache/thrift/ProcessFunction.java b/lib/java/src/org/apache/thrift/ProcessFunction.java
index 5c039fed7..340e30128 100644
--- a/lib/java/src/org/apache/thrift/ProcessFunction.java
+++ b/lib/java/src/org/apache/thrift/ProcessFunction.java
@@ -4,6 +4,7 @@ import org.apache.thrift.protocol.TMessage;
 import org.apache.thrift.protocol.TMessageType;
 import org.apache.thrift.protocol.TProtocol;
 import org.apache.thrift.protocol.TProtocolException;
+import org.apache.thrift.transport.TTransportException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -30,24 +31,29 @@ public abstract class ProcessFunction<I, T extends TBase> {
       return;
     }
     iprot.readMessageEnd();
-    TBase result = null;
+    TSerializable result = null;
+    byte msgType = TMessageType.REPLY;
 
     try {
       result = getResult(iface, args);
-    } catch(TException tex) {
-      LOGGER.error("Internal error processing " + getMethodName(), tex);
-      handleException(seqid, oprot);
-      return;
-    } catch(RuntimeException rex) {
-      LOGGER.error("Internal error processing " + getMethodName(), rex);
-      if (handleRuntimeExceptions()) {
-        handleException(seqid, oprot);
+    } catch (TTransportException ex) {
+      LOGGER.error("Transport error while processing " + getMethodName(), ex);
+      throw ex;
+    } catch (TApplicationException ex) {
+      LOGGER.error("Internal application error processing " + getMethodName(), ex);
+      result = ex;
+      msgType = TMessageType.EXCEPTION;
+    } catch (Exception ex) {
+      LOGGER.error("Internal error processing " + getMethodName(), ex);
+      if(!isOneway()) {
+        result = new TApplicationException(TApplicationException.INTERNAL_ERROR,
+            "Internal error processing " + getMethodName());
+        msgType = TMessageType.EXCEPTION;
       }
-      return;
     }
 
     if(!isOneway()) {
-      oprot.writeMessageBegin(new TMessage(getMethodName(), TMessageType.REPLY, seqid));
+      oprot.writeMessageBegin(new TMessage(getMethodName(), msgType, seqid));
       result.write(oprot);
       oprot.writeMessageEnd();
       oprot.getTransport().flush();
diff --git a/lib/java/test/org/apache/thrift/server/ServerTestBase.java b/lib/java/test/org/apache/thrift/server/ServerTestBase.java
index e3e4288e0..e245963d6 100644
--- a/lib/java/test/org/apache/thrift/server/ServerTestBase.java
+++ b/lib/java/test/org/apache/thrift/server/ServerTestBase.java
@@ -234,7 +234,8 @@ public abstract class ServerTestBase extends TestCase {
         x.message = arg;
         throw x;
       } else if ("TException".equals(arg)) {
-        throw new TException(arg);
+        // Unspecified exception should yield a TApplicationException on client side
+        throw new RuntimeException(arg);
       } else {
         Xtruct result = new Xtruct();
         result.string_thing = arg;
@@ -681,13 +682,15 @@ public abstract class ServerTestBase extends TestCase {
         x.errorCode = 1001;
         x.message = arg;
         // throw and onError yield the same result.
-        // resultHandler.onError(x);
-        // return;
-        throw x;
-      } else if ("TException".equals(arg)) {
-        // throw new TException(arg);
-        resultHandler.onError(new TException(arg));
+        // throw x;
+        resultHandler.onError(x);
         return;
+      } else if ("TException".equals(arg)) {
+        // throw and onError yield the same result.
+        // resultHandler.onError(new TException(arg));
+        // return;
+        // Unspecified exception should yield a TApplicationException on client side
+        throw new RuntimeException(arg);
       }
       resultHandler.onComplete(null);
     }