From c2faa0e3bf47abc5702aa8371e188dab00457b75 Mon Sep 17 00:00:00 2001 From: Nate Jensen Date: Wed, 12 Jun 2013 16:16:11 -0500 Subject: [PATCH] Issue #2102 improve error handling on thrift or http errors Change-Id: I6b2cad4c2a73eb436354012838423d8ba02f2c6b Former-commit-id: c38fb601bf4a360ab171e4d04f242e99f91710f2 --- .../alertviz.product | 3 +- .../awips.product | 3 +- .../developer.product | 3 +- edexOsgi/build.edex/esb/conf/wrapper.conf | 4 ++ .../raytheon/uf/common/comm/HttpClient.java | 38 ++++++++++++++++--- .../stream/DynamicSerializeStreamHandler.java | 2 +- .../thrift/SelfDescribingBinaryProtocol.java | 27 ++++++++++++- pythonPackages/pypies/pypies/handlers.py | 6 ++- 8 files changed, 72 insertions(+), 14 deletions(-) diff --git a/cave/com.raytheon.uf.viz.product.alertviz/alertviz.product b/cave/com.raytheon.uf.viz.product.alertviz/alertviz.product index f27bbd425f..b41ebb9baa 100644 --- a/cave/com.raytheon.uf.viz.product.alertviz/alertviz.product +++ b/cave/com.raytheon.uf.viz.product.alertviz/alertviz.product @@ -31,7 +31,8 @@ Developed on the Raytheon Visualization Environment (viz) -Dqpid.dest_syntax=BURL -Dcom.sun.management.jmxremote.authenticate=false -Dcom.sun.management.jmxremote.ssl=false --Dlog4j.configuration=log4j-alertviz.xml +-Dlog4j.configuration=log4j-alertviz.xml +-Dthrift.stream.maxsize=200 diff --git a/cave/com.raytheon.viz.product.awips/awips.product b/cave/com.raytheon.viz.product.awips/awips.product index 35a026f04c..3f1ae88f85 100644 --- a/cave/com.raytheon.viz.product.awips/awips.product +++ b/cave/com.raytheon.viz.product.awips/awips.product @@ -28,7 +28,8 @@ -Dcom.sun.management.jmxremote.authenticate=false -Dcom.sun.management.jmxremote.ssl=false -XX:OnOutOfMemoryError="capture -t no -p $pid &" --Dlog4j.configuration=log4j-viz-core.xml +-Dlog4j.configuration=log4j-viz-core.xml +-Dthrift.stream.maxsize=200 -Xmx1280M -Dfile.encoding=UTF-8 -Xmx768M diff --git a/cave/com.raytheon.viz.product.awips/developer.product b/cave/com.raytheon.viz.product.awips/developer.product index ff80a15d6c..92446e1e6b 100644 --- a/cave/com.raytheon.viz.product.awips/developer.product +++ b/cave/com.raytheon.viz.product.awips/developer.product @@ -24,7 +24,8 @@ -Dcom.sun.management.jmxremote -Dcom.sun.management.jmxremote.authenticate=false -Dcom.sun.management.jmxremote.ssl=false --Dlog4j.configuration=log4j-viz-core-developer.xml +-Dlog4j.configuration=log4j-viz-core-developer.xml +-Dthrift.stream.maxsize=200 -Dfile.encoding=UTF-8 diff --git a/edexOsgi/build.edex/esb/conf/wrapper.conf b/edexOsgi/build.edex/esb/conf/wrapper.conf index 7473c7169c..d2f298fa3e 100644 --- a/edexOsgi/build.edex/esb/conf/wrapper.conf +++ b/edexOsgi/build.edex/esb/conf/wrapper.conf @@ -127,6 +127,10 @@ wrapper.java.additional.web.2=-Dconfidential.port=8443 # is removed from SerializationManager wrapper.java.additional.misc.1=-DinitializeHibernatables=true +# the max size in MB of any stream sent to thrift, this prevents the OutOfMemory +# errors reported by thrift sometimes when the stream is corrupt/incorrect +wrapper.java.additional.thrift.maxStreamSize=-Dthrift.stream.maxsize=200 + # enables yourkit profiling, determined by flag to start.sh wrapper.java.additional.profile.1=${PROFILER_PARAM_1} diff --git a/edexOsgi/com.raytheon.uf.common.comm/src/com/raytheon/uf/common/comm/HttpClient.java b/edexOsgi/com.raytheon.uf.common.comm/src/com/raytheon/uf/common/comm/HttpClient.java index cbb6d2d44f..c516a9e0b4 100644 --- a/edexOsgi/com.raytheon.uf.common.comm/src/com/raytheon/uf/common/comm/HttpClient.java +++ b/edexOsgi/com.raytheon.uf.common.comm/src/com/raytheon/uf/common/comm/HttpClient.java @@ -93,15 +93,17 @@ import com.raytheon.uf.common.util.ByteArrayOutputStreamPool.ByteArrayOutputStre * Date Ticket# Engineer Description * ------------ ---------- ----------- -------------------------- * 7/1/06 #1088 chammack Initial Creation. - * 5/17/10 #5901 njensen Moved to common + * 5/17/10 #5901 njensen Moved to common * 03/02/11 #8045 rferrel Add connect reestablished message. - * 07/17/12 #911 njensen Refactored significantly - * 08/09/12 15307 snaples Added putEntitiy in postStreamingEntity. - * 01/07/13 DR 15294 D. Friedman Added streaming requests. - * Jan 24, 2013 1526 njensen Added postDynamicSerialize() + * 07/17/12 #911 njensen Refactored significantly + * 08/09/12 15307 snaples Added putEntitiy in postStreamingEntity. + * 01/07/13 DR 15294 D. Friedman Added streaming requests. + * Jan 24, 2013 1526 njensen Added postDynamicSerialize() * Feb 20, 2013 1628 bsteffen clean up Inflaters used by - * HttpClient. + * HttpClient. * Mar 11, 2013 1786 mpduff Add https capability. + * Jun 12, 2013 2102 njensen Better error handling when using + * DynamicSerializeStreamHandler * * * @@ -641,6 +643,30 @@ public class HttpClient { } } + if (resp.getStatusLine().getStatusCode() != SUCCESS_CODE + && handlerCallback instanceof DynamicSerializeStreamHandler) { + // the status code can be returned and/or processed + // depending on which post method and handlerCallback is used, + // so we only want to error off here if we're using a + // DynamicSerializeStreamHandler because deserializing will fail + // badly + String exceptionMsg = "HTTP server returned error code: " + + resp.getStatusLine().getStatusCode(); + DefaultInternalStreamHandler errorHandler = new DefaultInternalStreamHandler(); + String serverErrorMsg = null; + try { + errorHandler.handleStream(resp.getEntity().getContent()); + serverErrorMsg = new String(errorHandler.byteResult); + } catch (IOException e) { + statusHandler + .warn("Error reading the server's error message"); + } + if (serverErrorMsg != null) { + exceptionMsg += "\n" + serverErrorMsg; + } + throw new CommunicationException(exceptionMsg); + } + // should only be able to get here if we didn't encounter the // exceptions above on the most recent try processResponse(resp, handlerCallback); diff --git a/edexOsgi/com.raytheon.uf.common.comm/src/com/raytheon/uf/common/comm/stream/DynamicSerializeStreamHandler.java b/edexOsgi/com.raytheon.uf.common.comm/src/com/raytheon/uf/common/comm/stream/DynamicSerializeStreamHandler.java index e745af04e9..4737d992c0 100644 --- a/edexOsgi/com.raytheon.uf.common.comm/src/com/raytheon/uf/common/comm/stream/DynamicSerializeStreamHandler.java +++ b/edexOsgi/com.raytheon.uf.common.comm/src/com/raytheon/uf/common/comm/stream/DynamicSerializeStreamHandler.java @@ -63,7 +63,7 @@ public class DynamicSerializeStreamHandler implements IStreamHandler { SerializationType.Thrift).deserialize(is); } catch (SerializationException e) { throw new CommunicationException( - "Error deserializing streamed response"); + "Error deserializing streamed response", e); } } diff --git a/edexOsgi/com.raytheon.uf.common.serialization/src/com/raytheon/uf/common/serialization/thrift/SelfDescribingBinaryProtocol.java b/edexOsgi/com.raytheon.uf.common.serialization/src/com/raytheon/uf/common/serialization/thrift/SelfDescribingBinaryProtocol.java index 213909adb3..871fb976d6 100644 --- a/edexOsgi/com.raytheon.uf.common.serialization/src/com/raytheon/uf/common/serialization/thrift/SelfDescribingBinaryProtocol.java +++ b/edexOsgi/com.raytheon.uf.common.serialization/src/com/raytheon/uf/common/serialization/thrift/SelfDescribingBinaryProtocol.java @@ -54,7 +54,9 @@ import com.facebook.thrift.transport.TTransport; * Date Ticket# Engineer Description * ------------ ---------- ----------- -------------------------- * Aug 7, 2008 chammack Initial creation - * Jun 17, 2010 #5091 njensen Added primitive list methods + * Jun 17, 2010 #5091 njensen Added primitive list methods + * Jun 12, 2013 2102 njensen Added max read length to prevent out + * of memory errors due to bad stream * * * @@ -66,13 +68,34 @@ public class SelfDescribingBinaryProtocol extends TBinaryProtocol { public static final byte FLOAT = 64; + /** + * This is to ensure a safety check because if the stream has bad bytes at + * the start, thrift may try to allocate something huge, such as GBs of + * data, and then the JVM will blow up about OutOfMemory errors. + **/ + private static int MAX_READ_LENGTH; + + static { + try { + int sizeInMB = Integer.parseInt(System + .getProperty("thrift.stream.maxsize")); + MAX_READ_LENGTH = sizeInMB * 1024 * 1024; + } catch (Throwable t) { + System.err + .println("Error reading property thrift.stream.maxsize - falling back to default of 200 MB"); + t.printStackTrace(); + MAX_READ_LENGTH = 200 * 1024 * 1024; + } + } + public SelfDescribingBinaryProtocol(TTransport trans) { - super(trans); + this(trans, false, true); } public SelfDescribingBinaryProtocol(TTransport trans, boolean strictRead, boolean strictWrite) { super(trans, strictRead, strictWrite); + this.setReadLength(MAX_READ_LENGTH); } /* diff --git a/pythonPackages/pypies/pypies/handlers.py b/pythonPackages/pypies/pypies/handlers.py index b9e3a780d1..c7df9857d4 100644 --- a/pythonPackages/pypies/pypies/handlers.py +++ b/pythonPackages/pypies/pypies/handlers.py @@ -30,7 +30,8 @@ # ------------ ---------- ----------- -------------------------- # 08/17/10 njensen Initial Creation. # 01/11/13 bkowal Pypies will now read the hdf5 root from configuration -# 01/17/13 1490 bkowal Relocated the configuration of pypies +# 01/17/13 1490 bkowal Relocated the configuration of pypies +# 06/12/13 2102 njensen Raise uncaught exceptions to force http code 500 # # @@ -111,7 +112,8 @@ def pypies_response(request): except: # Absolutely should not reach this, if we do, need to fix code logger.error("Uncaught exception! " + IDataStore._exc()) - return Response("Very bad uncaught exception, check pypies log") + # want to re-raise the error as that will cause PyPIES to return http error code 500 + raise def __prepareResponse(resp):