You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by abdelhakim deneche <ad...@gmail.com> on 2015/04/08 22:04:51 UTC
Review Request 32987: DRILL-2675: Implement a subset of User
Exceptions to improve how errors are reported to the user
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32987/
-----------------------------------------------------------
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
Re: Review Request 32987: DRILL-2675: Implement a subset of User
Exceptions to improve how errors are reported to the user
Posted by abdelhakim deneche <ad...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32987/#review79426
-----------------------------------------------------------
common/src/main/java/org/apache/drill/common/exceptions/DrillUserException.java
<https://reviews.apache.org/r/32987/#comment128774>
I should probably change this into an abstract method.
common/src/main/java/org/apache/drill/common/exceptions/ErrorHelper.java
<https://reviews.apache.org/r/32987/#comment128775>
I will probably split the methods of this utility class into 2 utility classes:
- one class will deal with message generation, this is where we will make sure we have well formatted error messages for the clients
- the other class will handle the protobuf error objects specific methods. Most developers won't need to change this class.
common/src/main/java/org/apache/drill/common/exceptions/ErrorHelper.java
<https://reviews.apache.org/r/32987/#comment128776>
I am tempted to move this logic to the subclasses of DrillUserException, but I will need to convert the protobuf error object back to it's corresponding user exception. This doesn't seem to be needed elsewhere and I will still a similar switch in DrillUserException to handle the conversion
- 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
>
>
Re: Review Request 32987: DRILL-2675: Implement a subset of User
Exceptions to improve how errors are reported to the user
Posted by abdelhakim deneche <ad...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32987/
-----------------------------------------------------------
(Updated April 9, 2015, 5:20 p.m.)
Review request for drill and Jacques Nadeau.
Changes
-------
This is an improved patch. Error propagation has been improved, and it should be easier for developer to send meaningful error messages to the client without having the message being lost or mangled before reaching the client.
**Changes since the last patch**
- DrillSystemException will display the root error message of the wrapped exception
. from the user perspective, this will keep Drill behaviour the same when reporting errors that don't have **yet** a proper user error message
- DrillUserException.getMessage() generates the error message that will be sent to the client
. we don't need ErrorHelper to build the message from protobuf error object
. logging a DrillUserException will display the proper error message in the logs
- added DrillUnsupportedOperationException
- moved some error message generation logic from DrillParseException to DrillSqlWorker
- DrillUserException constructor expects an ErrorType. This allows it to build the final error message itself
- DrillUserException.addContext(String) can be used to add more context to existing user exceptions
. subclasses of DrillUserException don't need to hold specific context fields
. it is easier to add context to an existing user exception without knowing it's specific type
. the error message generation can be centralized in DrillUserException.getMessage()
- the final error message is generated on the server side and sent to the client through a DrillPBError's message field
. we don't need to update the clients each time we add a new context field to user exceptions
- JsonRecordReader first checks if the exception catched is not already a user exception, in this case it will just add more context to it and throw it, otherwise it will create a DrillDataReadException
- reverted my changes in proto objects DrillPBError and QueryResult, the only change left is the ErrorType enum
I still need to do some cleanup and add the remaining user exception types.
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 (updated)
-----
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/DrillUnsupportedOperationException.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/protobuf/UserBitShared.proto 5e44655
Diff: https://reviews.apache.org/r/32987/diff/
Testing
-------
Thanks,
abdelhakim deneche
Re: Review Request 32987: DRILL-2675: Implement a subset of User
Exceptions to improve how errors are reported to the user
Posted by abdelhakim deneche <ad...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32987/#review79450
-----------------------------------------------------------
common/src/main/java/org/apache/drill/common/exceptions/DrillSystemException.java
<https://reviews.apache.org/r/32987/#comment128836>
system exception are meant to wrap any _internal_ exception that we don't want the user to know the details about. It doesn't make sense to have a constructor with just a message string.
- 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
>
>
Re: Review Request 32987: DRILL-2675: Implement a subset of User
Exceptions to improve how errors are reported to the user
Posted by abdelhakim deneche <ad...@gmail.com>.
-----------------------------------------------------------
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
>
>
Re: Review Request 32987: DRILL-2675: Implement a subset of User
Exceptions to improve how errors are reported to the user
Posted by abdelhakim deneche <ad...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32987/#review79437
-----------------------------------------------------------
protocol/src/main/protobuf/UserBitShared.proto
<https://reviews.apache.org/r/32987/#comment128797>
changed from repeated to optional because the current code can't return multiple failures to the client.
- 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
>
>
Re: Review Request 32987: DRILL-2675: Implement a subset of User
Exceptions to improve how errors are reported to the user
Posted by abdelhakim deneche <ad...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32987/#review79448
-----------------------------------------------------------
common/src/main/java/org/apache/drill/common/exceptions/DrillUserException.java
<https://reviews.apache.org/r/32987/#comment128833>
this shouldn't be here!
- 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
>
>
Re: Review Request 32987: DRILL-2675: Implement a subset of User
Exceptions to improve how errors are reported to the user
Posted by abdelhakim deneche <ad...@gmail.com>.
> On April 8, 2015, 11:20 p.m., Parth Chandra wrote:
> > protocol/src/main/protobuf/UserBitShared.proto, line 51
> > <https://reviews.apache.org/r/32987/diff/1/?file=921087#file921087line51>
> >
> > I think this information should also be included in the message itself. If at a later stage we want to include additional information (for example, add the field name where a data read error occurred), then we will not need to change the protobuf mesage and there will be no impact on the clients.
generating the message on the server side has some additional benefits: I can move the generation logic into the exception classes and remove the ugly switch code from ErrorHelper
- abdelhakim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32987/#review79447
-----------------------------------------------------------
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
>
>
Re: Review Request 32987: DRILL-2675: Implement a subset of User
Exceptions to improve how errors are reported to the user
Posted by abdelhakim deneche <ad...@gmail.com>.
> On April 8, 2015, 11:20 p.m., Parth Chandra wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JSONRecordReader.java, line 94
> > <https://reviews.apache.org/r/32987/diff/1/?file=921076#file921076line94>
> >
> > It would help if we have a hierarchy of exception 'contexts' that we can use to build the message. For instance, in this code, JSONRecord reader will catch an exception thrown by JSONReader which in turn will have caught an exception thrown by SingleListWriter. The SingleListWriter has no idea of the file name, line number, etc that the erroneous data came from. JSONReader knows the line number and column but not the file name. JSONRecordReader knows the file name. If each level adds the information about the context, the DrillUserException created can construct a message that contains the error as well as the context (in this case the precise location) of where the error occurred.
DrillUserException exposes an addContext(String) that can be used to add more context to existing exceptions. One example of this being used is inside JsonRecordReader.handleAndRaise()
- abdelhakim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32987/#review79447
-----------------------------------------------------------
On April 9, 2015, 5:42 p.m., abdelhakim deneche wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32987/
> -----------------------------------------------------------
>
> (Updated April 9, 2015, 5:42 p.m.)
>
>
> Review request for drill, Jacques Nadeau, Jason Altekruse, and Parth Chandra.
>
>
> 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/DrillUnsupportedOperationException.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/protobuf/UserBitShared.proto 5e44655
>
> Diff: https://reviews.apache.org/r/32987/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> abdelhakim deneche
>
>
Re: Review Request 32987: DRILL-2675: Implement a subset of User
Exceptions to improve how errors are reported to the user
Posted by Parth Chandra <pc...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32987/#review79447
-----------------------------------------------------------
We'll need a DrillUnsupportedOperationException
exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JSONRecordReader.java
<https://reviews.apache.org/r/32987/#comment128835>
It would help if we have a hierarchy of exception 'contexts' that we can use to build the message. For instance, in this code, JSONRecord reader will catch an exception thrown by JSONReader which in turn will have caught an exception thrown by SingleListWriter. The SingleListWriter has no idea of the file name, line number, etc that the erroneous data came from. JSONReader knows the line number and column but not the file name. JSONRecordReader knows the file name. If each level adds the information about the context, the DrillUserException created can construct a message that contains the error as well as the context (in this case the precise location) of where the error occurred.
protocol/src/main/protobuf/UserBitShared.proto
<https://reviews.apache.org/r/32987/#comment128832>
I think this information should also be included in the message itself. If at a later stage we want to include additional information (for example, add the field name where a data read error occurred), then we will not need to change the protobuf mesage and there will be no impact on the clients.
- Parth Chandra
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
>
>