drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "abdelhakim deneche" <adene...@gmail.com>
Subject Re: Review Request 32987: DRILL-2675: Implement a subset of User Exceptions to improve how errors are reported to the user
Date Sat, 11 Apr 2015 05:07:05 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32987/
-----------------------------------------------------------

(Updated April 11, 2015, 5:07 a.m.)


Review request for drill, Jacques Nadeau, Jason Altekruse, and Parth Chandra.


Changes
-------

- UserResultListener.submissionFailed(DrillUserException) instead of RpcException
- fix unit tests that were expecting RpcException and improve the ones that were extracting
  the exception className and message from the RpcException message
- DrillUserException.Builder is the only way one can create user exceptions
  JsonRecordReader.handleAndRaise() gives a good example of how to use DrillUserException.Builder
- ErrorHelper.wrap(Throwable e) wraps exception inside a system exception if it's not already
a DrillUserException

I still need to improve the exception context to accept DrillbitEndpoint and FragmentContext...


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/DrillRemoteException.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 
  common/src/main/java/org/apache/drill/common/exceptions/UserExceptionContext.java PRE-CREATION

  exec/java-exec/src/main/java/org/apache/drill/exec/client/DrillClient.java 9a948fb 
  exec/java-exec/src/main/java/org/apache/drill/exec/client/PrintingResultsListener.java 98948af

  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/rpc/user/UserResultsListener.java 934a094

  exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/QueryWrapper.java fbbf0b8

  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/foreman/SqlUnsupportedException.java
2299afa 
  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

  exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java 0c2f0e5 
  exec/java-exec/src/test/java/org/apache/drill/SingleRowListener.java 5703bf9 
  exec/java-exec/src/test/java/org/apache/drill/TestBugFixes.java 875fb25 
  exec/java-exec/src/test/java/org/apache/drill/TestDisabledFunctionality.java f62f060 
  exec/java-exec/src/test/java/org/apache/drill/TestStarQueries.java effef9b 
  exec/java-exec/src/test/java/org/apache/drill/TestUnionAll.java 11d83f9 
  exec/java-exec/src/test/java/org/apache/drill/exec/server/TestDrillbitResilience.java e03098a

  exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/ParquetResultListener.java
55f0d75 
  exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/TestParquetPhysicalPlan.java
882cdbd 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillCursor.java 3b38a09 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillResultSet.java 74900bc 
  protocol/src/main/java/org/apache/drill/exec/proto/SchemaUserBitShared.java f72d5e1 
  protocol/src/main/java/org/apache/drill/exec/proto/UserBitShared.java ac1bcbb 
  protocol/src/main/java/org/apache/drill/exec/proto/beans/DrillPBError.java ac9cef5 
  protocol/src/main/protobuf/UserBitShared.proto 2938114 

Diff: https://reviews.apache.org/r/32987/diff/


Testing (updated)
-------

Unit tests are passing


Thanks,

abdelhakim deneche


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message