-----------------------------------------------------------
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
