[GitHub] [druid] paul-rogers commented on issue #13123: Enhanced Druid Exceptions for improved user error messages

[GitBox <gi...@apache.org>]

paul-rogers commented on issue #13123:
URL: https://github.com/apache/druid/issues/13123#issuecomment-1261656839

   @gianm, thank you for the suggestion to look at the MSQ errors. The example you showed is very much in line with the idea outlined in the proposal. The format of the example is somewhat MSQ-specific (with row counts, query IDs, etc.) We can reconcile it with the proposal by viewing the MSQ-specific items as the "context" suggested in the proposal.
   
   @FrankChen021, this example may clarify the meaning of "context": it is additional information relevant to the error in question. Different errors have different "extra" context (line number for an SQL error, record number for an ingest error, segment ID for other things, etc.) The context provides a generic way of handling situation-specific information useful to the user. 
   
   Making up a quick straw-man example:
   
   ```java
     throw DruidException
         .userError("TooManyColumns")
         .error(DruidException.RUNTIME_ERROR)
         .message("Too many output columns (requested = %d, max = %d)", colCount, COL_LIMIT)
         .context( "numColumns", colCount)
         .context("maxColumns", 2000)
         .build();
   ```
   
   One could image this being serialized to JSON as:
   
   ```json
     "error": {
       "code": "TooManyColumns",
       "kind": "Runtime Error",
       "message": "Too many output columns (requested = 2003, max = 2000)"
       "context": {
         "numColumns": 2003,
         "maxColumns": 2000
       }
     },
   ```
   
   Using your `TooManyColumns` example, I hunted down how things are handled in MSQ today. Looks like there are multiple `Fault` classes, with JSON-serializable fields to capture the error-specific information. This works, but may lead to a large number of classes. The `DruidException`, by contrast, tries to be a one-size-fits-all error.
   
   This raises the question, what if we want to catch certain exceptions? Generally, a `DruidException` would occur when an operation-fatal error occurs that will bubble back up to the user. If we have an "internal" exception that we will handle elsewhere, we'd continue to use "normal" exceptions. And, of course, if it makes sense to have subclasses for different use cases, we could do so.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org