Michael Blow has submitted this change and it was merged. Change subject: Do not include Exception class in error message ......................................................................
Do not include Exception class in error message Change-Id: I33c76222dabfe6ede67dbcd7f0992cbb047265ef Reviewed-on: https://asterix-gerrit.ics.uci.edu/1757 Sonar-Qube: Jenkins <[email protected]> Tested-by: Jenkins <[email protected]> BAD: Jenkins <[email protected]> Integration-Tests: Jenkins <[email protected]> Reviewed-by: Michael Blow <[email protected]> --- M asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/QueryServiceServlet.java M asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/ResultUtil.java M asterixdb/asterix-app/src/main/java/org/apache/asterix/app/message/ExecuteStatementRequestMessage.java 3 files changed, 27 insertions(+), 18 deletions(-) Approvals: Michael Blow: Looks good to me, approved Jenkins: Verified; No violations found; No violations found; Verified diff --git a/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/QueryServiceServlet.java b/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/QueryServiceServlet.java index c45f24a..479c8b0 100644 --- a/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/QueryServiceServlet.java +++ b/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/QueryServiceServlet.java @@ -48,7 +48,9 @@ import org.apache.asterix.translator.IStatementExecutorFactory; import org.apache.asterix.translator.SessionConfig; import org.apache.asterix.translator.SessionOutput; +import org.apache.hyracks.algebricks.common.exceptions.AlgebricksException; import org.apache.hyracks.api.application.IServiceContext; +import org.apache.hyracks.api.exceptions.HyracksException; import org.apache.hyracks.http.api.IServletRequest; import org.apache.hyracks.http.api.IServletResponse; import org.apache.hyracks.http.server.utils.HttpUtil; @@ -415,13 +417,18 @@ if (ResultDelivery.IMMEDIATE == delivery || ResultDelivery.DEFERRED == delivery) { ResultUtil.printStatus(sessionOutput, ResultStatus.SUCCESS); } - } catch (AsterixException | TokenMgrError | org.apache.asterix.aqlplus.parser.TokenMgrError pe) { - GlobalConfig.ASTERIX_LOGGER.log(Level.SEVERE, pe.getMessage(), pe); + } catch (AlgebricksException | TokenMgrError | org.apache.asterix.aqlplus.parser.TokenMgrError pe) { + GlobalConfig.ASTERIX_LOGGER.log(Level.INFO, pe.getMessage(), pe); ResultUtil.printError(resultWriter, pe); ResultUtil.printStatus(sessionOutput, ResultStatus.FATAL); status = HttpResponseStatus.BAD_REQUEST; + } catch (HyracksException pe) { + GlobalConfig.ASTERIX_LOGGER.log(Level.WARNING, pe.getMessage(), pe); + ResultUtil.printError(resultWriter, pe); + ResultUtil.printStatus(sessionOutput, ResultStatus.FATAL); + status = HttpResponseStatus.INTERNAL_SERVER_ERROR; } catch (Exception e) { - GlobalConfig.ASTERIX_LOGGER.log(Level.SEVERE, e.toString(), e); + GlobalConfig.ASTERIX_LOGGER.log(Level.SEVERE, "Unexpected exception", e); ResultUtil.printError(resultWriter, e); ResultUtil.printStatus(sessionOutput, ResultStatus.FATAL); status = HttpResponseStatus.INTERNAL_SERVER_ERROR; diff --git a/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/ResultUtil.java b/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/ResultUtil.java index 3bcd670..f004446 100644 --- a/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/ResultUtil.java +++ b/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/ResultUtil.java @@ -37,6 +37,7 @@ import org.apache.asterix.app.result.ResultPrinter; import org.apache.asterix.app.result.ResultReader; import org.apache.asterix.common.api.IApplicationContext; +import org.apache.asterix.lang.aql.parser.TokenMgrError; import org.apache.asterix.om.types.ARecordType; import org.apache.asterix.translator.IStatementExecutor.Stats; import org.apache.asterix.translator.SessionOutput; @@ -44,6 +45,7 @@ import org.apache.hyracks.algebricks.common.exceptions.AlgebricksException; import org.apache.hyracks.algebricks.core.algebra.prettyprint.AlgebricksAppendable; import org.apache.hyracks.api.exceptions.HyracksDataException; +import org.apache.hyracks.api.exceptions.HyracksException; import org.apache.hyracks.util.JSONUtil; import org.apache.log4j.Logger; @@ -115,17 +117,18 @@ public static void printError(PrintWriter pw, Throwable e, boolean comma) { Throwable rootCause = getRootCause(e); - if (rootCause == null) { - rootCause = e; - } final boolean addStack = false; pw.print("\t\""); pw.print(AbstractQueryApiServlet.ResultFields.ERRORS.str()); pw.print("\": [{ \n"); printField(pw, QueryServiceServlet.ErrorField.CODE.str(), "1"); - final String msg = rootCause.getMessage(); - printField(pw, QueryServiceServlet.ErrorField.MSG.str(), - JSONUtil.escape(msg != null ? msg : rootCause.getClass().getSimpleName()), addStack); + String msg = rootCause.getMessage(); + if (!(rootCause instanceof AlgebricksException || rootCause instanceof HyracksException + || rootCause instanceof TokenMgrError + || rootCause instanceof org.apache.asterix.aqlplus.parser.TokenMgrError)) { + msg = rootCause.getClass().getSimpleName() + (msg == null ? "" : ": " + msg); + } + printField(pw, QueryServiceServlet.ErrorField.MSG.str(), JSONUtil.escape(msg), addStack); pw.print(comma ? "\t}],\n" : "\t}]\n"); } diff --git a/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/message/ExecuteStatementRequestMessage.java b/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/message/ExecuteStatementRequestMessage.java index defa180..fc0c1ff 100644 --- a/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/message/ExecuteStatementRequestMessage.java +++ b/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/message/ExecuteStatementRequestMessage.java @@ -32,7 +32,6 @@ import org.apache.asterix.common.config.GlobalConfig; import org.apache.asterix.common.context.IStorageComponentProvider; import org.apache.asterix.common.dataflow.ICcApplicationContext; -import org.apache.asterix.common.exceptions.AsterixException; import org.apache.asterix.common.messaging.api.ICcAddressedMessage; import org.apache.asterix.compiler.provider.ILangCompilationProvider; import org.apache.asterix.hyracks.bootstrap.CCApplication; @@ -47,8 +46,10 @@ import org.apache.asterix.translator.IStatementExecutorFactory; import org.apache.asterix.translator.SessionConfig; import org.apache.asterix.translator.SessionOutput; +import org.apache.hyracks.algebricks.common.exceptions.AlgebricksException; import org.apache.hyracks.api.application.ICCServiceContext; import org.apache.hyracks.api.exceptions.HyracksDataException; +import org.apache.hyracks.api.exceptions.HyracksException; import org.apache.hyracks.control.cc.ClusterControllerService; public final class ExecuteStatementRequestMessage implements ICcAddressedMessage { @@ -129,16 +130,14 @@ outPrinter.close(); responseMsg.setResult(outWriter.toString()); responseMsg.setMetadata(outMetadata); - } catch (TokenMgrError | org.apache.asterix.aqlplus.parser.TokenMgrError pe) { - GlobalConfig.ASTERIX_LOGGER.log(Level.SEVERE, pe.getMessage(), pe); + } catch (AlgebricksException | HyracksException | TokenMgrError + | org.apache.asterix.aqlplus.parser.TokenMgrError pe) { + // we trust that "our" exceptions are serializable and have a comprehensible error message + GlobalConfig.ASTERIX_LOGGER.log(Level.WARNING, pe.getMessage(), pe); responseMsg.setError(pe); - } catch (AsterixException pe) { - GlobalConfig.ASTERIX_LOGGER.log(Level.SEVERE, pe.getMessage(), pe); - responseMsg.setError(new AsterixException(pe.getMessage())); } catch (Exception e) { - String estr = e.toString(); - GlobalConfig.ASTERIX_LOGGER.log(Level.SEVERE, estr, e); - responseMsg.setError(new Exception(estr)); + GlobalConfig.ASTERIX_LOGGER.log(Level.SEVERE, "Unexpected exception", e); + responseMsg.setError(new Exception(e.toString())); } try { -- To view, visit https://asterix-gerrit.ics.uci.edu/1757 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: merged Gerrit-Change-Id: I33c76222dabfe6ede67dbcd7f0992cbb047265ef Gerrit-PatchSet: 4 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Till Westmann <[email protected]> Gerrit-Reviewer: Dmitry Lychagin <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Michael Blow <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]>
