----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32987/#review79421 -----------------------------------------------------------
common/src/main/java/org/apache/drill/common/exceptions/DrillDataReadException.java <https://reviews.apache.org/r/32987/#comment128762> I should move this to DrillUserException and pass the ErrorType in it's constructor. common/src/main/java/org/apache/drill/common/exceptions/DrillParseException.java <https://reviews.apache.org/r/32987/#comment128763> same as above. common/src/main/java/org/apache/drill/common/exceptions/DrillParseException.java <https://reviews.apache.org/r/32987/#comment128766> I can remove this, it's already done in super.newPBErrorBuilder(). Until DrillParseException has specific context information, we can remove newPBErrorBuilder() from DrillParseException common/src/main/java/org/apache/drill/common/exceptions/DrillRemoteException.java <https://reviews.apache.org/r/32987/#comment128767> The sole purpose of this class is to wrap a DrillPBError so it can be passed around, and thrown if necessary. It replaces RemoteRpcException. This class should extend DrillUserException instead of DrillSystemException common/src/main/java/org/apache/drill/common/exceptions/DrillSystemException.java <https://reviews.apache.org/r/32987/#comment128772> This class represents any internal Drill exception sent to the client. It should not display any error message except from a generic "system exception" along with the error_id and the drillbit identity. common/src/main/java/org/apache/drill/common/exceptions/DrillSystemException.java <https://reviews.apache.org/r/32987/#comment128773> This method is called whenever a class wants to log or store an exception that will be sent to the client. Foreman, FragmentContext and FragmentExecutor are examples of such classes. This method makes sure user exception are not lost when wrapped inside other exceptions. Any exception that is not a user exception is considered to be a system exception (this will most likely make most of Drill error messages disappear behind "system exception" until those messages are properly fixed) - abdelhakim deneche On April 8, 2015, 8:04 p.m., abdelhakim deneche wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/32987/ > ----------------------------------------------------------- > > (Updated April 8, 2015, 8:04 p.m.) > > > Review request for drill and Jacques Nadeau. > > > Bugs: DRILL-2675 > https://issues.apache.org/jira/browse/DRILL-2675 > > > Repository: drill-git > > > Description > ------- > > **INITIAL PATCH** with a working solution. This patch cleans the path for > errors, especially user errors with meaningful messages, to be propagated > properly to the client. > > The patch includes changes to 2 existing use cases where the error message > was successfully improved. > > The general idea is: if a code wants to throw an exception that contains a > meaningful error message, it throws a DrillUserException. The propagation > code will make sure this exception is propagated to the client. The user > exception object doesn't contain the final error message, but enough > information about the error, the client will use this information to display > a better error message. > Any exception that is not a DrillUserException (or one of it's subclasses) > will be considered as a system exception. For those exceptions the client > will only display the error id and drillbit identity in case the user wants > to check the logs for more informations about the error. > Error objects sent to the client will still contain a stack trace that can be > used to display more information if the client has enabled the error verbose > mode. > > > Diffs > ----- > > > common/src/main/java/org/apache/drill/common/exceptions/DrillDataReadException.java > PRE-CREATION > > common/src/main/java/org/apache/drill/common/exceptions/DrillParseException.java > PRE-CREATION > > common/src/main/java/org/apache/drill/common/exceptions/DrillRemoteException.java > PRE-CREATION > > common/src/main/java/org/apache/drill/common/exceptions/DrillSystemException.java > PRE-CREATION > > common/src/main/java/org/apache/drill/common/exceptions/DrillUserException.java > PRE-CREATION > common/src/main/java/org/apache/drill/common/exceptions/ErrorHelper.java > PRE-CREATION > exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java > 18b93e9 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScreenCreator.java > 8038527 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/broadcastsender/BroadcastSenderRootExec.java > d17fdd4 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/StatusHandler.java > 5e21878 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlWorker.java > b98778d > > exec/java-exec/src/main/java/org/apache/drill/exec/rpc/CoordinationQueue.java > 0016d6a > > exec/java-exec/src/main/java/org/apache/drill/exec/rpc/RemoteRpcException.java > 14ea873 > exec/java-exec/src/main/java/org/apache/drill/exec/rpc/RpcBus.java b974963 > > exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/QueryResultHandler.java > a1be83b > > exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JSONRecordReader.java > cc7cb83 > exec/java-exec/src/main/java/org/apache/drill/exec/work/ErrorHelper.java > 0773d6c > > exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java > 23ef0d3 > > exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryManager.java > 8626d5b > > exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/AbstractStatusReporter.java > 1b0885d > > exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java > a7e6c46 > > exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/StatusReporter.java > 26b5d68 > protocol/src/main/java/org/apache/drill/exec/proto/SchemaUserBitShared.java > f72d5e1 > protocol/src/main/java/org/apache/drill/exec/proto/UserBitShared.java > 9a9d196 > protocol/src/main/java/org/apache/drill/exec/proto/beans/DrillPBError.java > ac9cef5 > protocol/src/main/java/org/apache/drill/exec/proto/beans/QueryResult.java > 474e330 > protocol/src/main/protobuf/UserBitShared.proto 5e44655 > > Diff: https://reviews.apache.org/r/32987/diff/ > > > Testing > ------- > > > Thanks, > > abdelhakim deneche > >
