[incubator-pinot] branch master updated: [TE] Fix exception handling - Propagate and display the error message/exception on frontind (#4419)

[ak...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

akshayrai09 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git


The following commit(s) were added to refs/heads/master by this push:
     new 0993974  [TE] Fix exception handling - Propagate and display the error message/exception on frontind (#4419)
0993974 is described below

commit 09939744b401e140c4724b0af9277a5822860f3d
Author: Akshay Rai <ak...@gmail.com>
AuthorDate: Thu Jul 11 11:09:29 2019 -0700

    [TE] Fix exception handling - Propagate and display the error message/exception on frontind (#4419)
---
 .../thirdeye/detection/DetectionPipeline.java      | 35 ++++++++++++----------
 .../detection/DetectionPipelineLoader.java         | 10 +++++--
 .../validators/DetectionConfigValidator.java       | 23 ++++++--------
 .../thirdeye/detection/yaml/YamlResource.java      | 24 ++++++++-------
 4 files changed, 49 insertions(+), 43 deletions(-)

diff --git a/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/DetectionPipeline.java b/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/DetectionPipeline.java
index 425a246..d10a7f4 100644
--- a/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/DetectionPipeline.java
+++ b/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/DetectionPipeline.java
@@ -66,11 +66,7 @@ public abstract class DetectionPipeline {
     this.config = config;
     this.startTime = startTime;
     this.endTime = endTime;
-    try {
-      this.initComponents();
-    } catch (Exception e) {
-      throw new IllegalArgumentException("Initialize components failed. Please check rule parameters. " + e.getMessage());
-    }
+    this.initComponents();
   }
 
   /**
@@ -83,9 +79,8 @@ public abstract class DetectionPipeline {
 
   /**
    * Initialize all components in the pipeline
-   * @throws Exception
    */
-  private void initComponents() throws Exception {
+  private void initComponents() {
     InputDataFetcher dataFetcher = new DefaultInputDataFetcher(this.provider, this.config.getId());
     Map<String, BaseComponent> instancesMap = config.getComponents();
     Map<String, Object> componentSpecs = config.getComponentSpecs();
@@ -104,22 +99,32 @@ public abstract class DetectionPipeline {
             componentSpec.put(entry.getKey(), instancesMap.get(DetectionUtils.getComponentKey(entry.getValue().toString())));
           }
         }
+        // Initialize the components
         instancesMap.get(componentKey).init(getComponentSpec(componentSpec), dataFetcher);
       }
     }
     config.setComponents(instancesMap);
   }
 
-  private BaseComponent createComponent(Map<String, Object> componentSpec)
-      throws Exception {
-    Class<BaseComponent> clazz = (Class<BaseComponent>) Class.forName(MapUtils.getString(componentSpec, PROP_CLASS_NAME));
-    return clazz.newInstance();
+  private BaseComponent createComponent(Map<String, Object> componentSpec) {
+    String className = MapUtils.getString(componentSpec, PROP_CLASS_NAME);
+    try {
+      Class<BaseComponent> clazz = (Class<BaseComponent>) Class.forName(className);
+      return clazz.newInstance();
+    } catch (Exception e) {
+      throw new IllegalArgumentException("Failed to create component for " + className, e.getCause());
+    }
   }
 
-  private AbstractSpec getComponentSpec(Map<String, Object> componentSpec) throws Exception {
-    Class clazz = Class.forName(MapUtils.getString(componentSpec, PROP_CLASS_NAME));
-    Class<AbstractSpec> specClazz = (Class<AbstractSpec>) Class.forName(getSpecClassName(clazz));
-    return AbstractSpec.fromProperties(componentSpec, specClazz);
+  private AbstractSpec getComponentSpec(Map<String, Object> componentSpec) {
+    String className = MapUtils.getString(componentSpec, PROP_CLASS_NAME);
+    try {
+      Class clazz = Class.forName(className);
+      Class<AbstractSpec> specClazz = (Class<AbstractSpec>) Class.forName(getSpecClassName(clazz));
+      return AbstractSpec.fromProperties(componentSpec, specClazz);
+    } catch (Exception e) {
+      throw new IllegalArgumentException("Failed to get component spec for " + className, e);
+    }
   }
 
   /**
diff --git a/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/DetectionPipelineLoader.java b/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/DetectionPipelineLoader.java
index 53ba49f..dff23c8 100644
--- a/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/DetectionPipelineLoader.java
+++ b/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/DetectionPipelineLoader.java
@@ -26,9 +26,13 @@ import java.lang.reflect.Constructor;
 public class DetectionPipelineLoader {
   private static final String PROP_CLASS_NAME = "className";
 
-  public DetectionPipeline from(DataProvider provider, DetectionConfigDTO config, long start, long end) throws Exception {
+  public DetectionPipeline from(DataProvider provider, DetectionConfigDTO config, long start, long end) {
     String className = config.getProperties().get(PROP_CLASS_NAME).toString();
-    Constructor<?> constructor = Class.forName(className).getConstructor(DataProvider.class, DetectionConfigDTO.class, long.class, long.class);
-    return (DetectionPipeline) constructor.newInstance(provider, config, start, end);
+    try {
+      Constructor<?> constructor = Class.forName(className).getConstructor(DataProvider.class, DetectionConfigDTO.class, long.class, long.class);
+      return (DetectionPipeline) constructor.newInstance(provider, config, start, end);
+    } catch (Exception e) {
+      throw new IllegalArgumentException("Failed to initialize the detection pipeline.", e.getCause());
+    }
   }
 }
diff --git a/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/validators/DetectionConfigValidator.java b/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/validators/DetectionConfigValidator.java
index 9fa0cdd..b37c7c4 100644
--- a/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/validators/DetectionConfigValidator.java
+++ b/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/validators/DetectionConfigValidator.java
@@ -72,20 +72,15 @@ public class DetectionConfigValidator implements ConfigValidator<DetectionConfig
    * Validate the pipeline by loading and initializing components
    */
   private void semanticValidation(DetectionConfigDTO detectionConfig) {
-    try {
-      // backup and swap out id
-      Long id = detectionConfig.getId();
-      detectionConfig.setId(-1L);
-
-      // try to load the detection pipeline and init all the components
-      this.loader.from(provider, detectionConfig, 0, 0);
-
-      // set id back
-      detectionConfig.setId(id);
-    } catch (Exception e){
-      // exception thrown in validate pipeline via reflection
-      throw new IllegalArgumentException("Semantic error: " + e.getCause().getMessage());
-    }
+    // backup and swap out id
+    Long id = detectionConfig.getId();
+    detectionConfig.setId(-1L);
+
+    // try to load the detection pipeline and init all the components
+    this.loader.from(provider, detectionConfig, 0, 0);
+
+    // set id back
+    detectionConfig.setId(id);
   }
 
   /**
diff --git a/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/yaml/YamlResource.java b/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/yaml/YamlResource.java
index 7515251..d880fb2 100644
--- a/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/yaml/YamlResource.java
+++ b/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/yaml/YamlResource.java
@@ -26,6 +26,8 @@ import io.dropwizard.auth.Auth;
 import io.swagger.annotations.Api;
 import io.swagger.annotations.ApiOperation;
 import io.swagger.annotations.ApiParam;
+import java.io.PrintWriter;
+import java.io.StringWriter;
 import java.lang.reflect.InvocationTargetException;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -231,23 +233,27 @@ public class YamlResource {
   private Response processBadRequestResponse(String type, String operation, String payload, IllegalArgumentException e) {
     Map<String, String> responseMessage = new HashMap<>();
     LOG.warn("Validation error while {} {} with payload {}", operation, type, payload, e);
-    responseMessage.put(type + "Msg", "Validation Error! " + e.getMessage());
+    responseMessage.put("message", "Validation Error in " + type + "! " + e.getMessage());
+    StringWriter sw = new StringWriter();
+    PrintWriter pw = new PrintWriter(sw);
+    e.getCause().printStackTrace(pw);
+    responseMessage.put("more-info", "Error = " + sw.toString());
     return Response.status(Response.Status.BAD_REQUEST).entity(responseMessage).build();
   }
 
   private Response processServerErrorResponse(String type, String operation, String payload, Exception e) {
     Map<String, String> responseMessage = new HashMap<>();
     LOG.error("Error {} {} with payload {}", operation, type, payload, e);
-    responseMessage.put(type + "Msg", "Failed to create the " + type + ". Reach out to the ThirdEye team.");
-    responseMessage.put(type + "Msg-moreInfo", "Error = " + e.getMessage());
+    responseMessage.put("message", "Failed to create the " + type + ". Reach out to the ThirdEye team.");
+    responseMessage.put("more-info", "Error = " + e.getMessage());
     return Response.serverError().entity(responseMessage).build();
   }
 
   private Response processBadAuthorizationResponse(String type, String operation, String payload, NotAuthorizedException e) {
     Map<String, String> responseMessage = new HashMap<>();
     LOG.warn("Authorization error while {} {} with payload {}", operation, type, payload, e);
-    responseMessage.put(type + "Msg", "Authorization Error!");
-    responseMessage.put(type + "Msg-moreInfo", "Configure owners property in " + type + " config");
+    responseMessage.put("message", "Authorization error! You do not have permissions to " + operation + " this " + type + " config");
+    responseMessage.put("more-info", "Configure owners property in " + type + " config");
     return Response.status(Response.Status.UNAUTHORIZED).entity(responseMessage).build();
   }
 
@@ -265,7 +271,6 @@ public class YamlResource {
     Map<String, String> yamls;
 
     // Detection
-    Map<String, String> responseMessage = new HashMap<>();
     long detectionConfigId;
     try {
       yamls = OBJECT_MAPPER.readValue(payload, Map.class);
@@ -374,8 +379,8 @@ public class YamlResource {
     }
 
     LOG.info("Detection created with id " + detectionConfigId + " using payload " + payload);
-    responseMessage.put("detectionMsg", "Alert was created successfully.");
-    responseMessage.put("detectionMsg-moreInfo", "Record saved with id " + detectionConfigId);
+    responseMessage.put("message", "Alert was created successfully.");
+    responseMessage.put("more-info", "Record saved with id " + detectionConfigId);
     return Response.ok().entity(responseMessage).build();
   }
 
@@ -741,9 +746,6 @@ public class YamlResource {
       return Response.ok(result).build();
     } catch (IllegalArgumentException e) {
       return processBadRequestResponse(YamlOperations.PREVIEW.name(), YamlOperations.RUNNING.name(), payload, e);
-    } catch (InvocationTargetException e) {
-      responseMessage.put("message", "Failed to run the preview due to " + e.getTargetException().getMessage());
-      return Response.serverError().entity(responseMessage).build();
     } catch (TimeoutException e) {
       responseMessage.put("message", "Preview has timed out");
       return Response.serverError().entity(responseMessage).build();


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