Some general improvements to integrate reports better

Signed-off-by: Simeon Widdis <sawiddis@amazon.com>

Author: Swiddis

common/src/main/java/org/opensearch/sql/common/error/ErrorReport.java

@@ -9,6 +9,7 @@
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import lombok.Getter;
 
 /**
  * Error report that wraps exceptions and accumulates contextual information as errors bubble up
@@ -37,12 +38,12 @@
 public class ErrorReport extends RuntimeException {
 
   private final Throwable cause;
-  private final ErrorCode code;
-  private final QueryProcessingStage stage;
+  @Getter private final ErrorCode code;
+  @Getter private final QueryProcessingStage stage;
   private final List<String> locationChain;
   private final Map<String, Object> context;
-  private final String suggestion;
-  private final String details;
+  @Getter private final String suggestion;
+  @Getter private final String details;
 
   private ErrorReport(Builder builder) {
     super(builder.cause.getMessage(), builder.cause);
@@ -63,8 +64,7 @@ private ErrorReport(Builder builder) {
    * @return A builder for constructing the error report
    */
   public static Builder wrap(Throwable cause) {
-    if (cause instanceof ErrorReport) {
-      ErrorReport existing = (ErrorReport) cause;
+    if (cause instanceof ErrorReport existing) {
       return new Builder(existing.cause)
           .code(existing.code)
           .stage(existing.stage)
@@ -76,14 +76,6 @@ public static Builder wrap(Throwable cause) {
     return new Builder(cause);
   }
 
-  public ErrorCode getCode() {
-    return code;
-  }
-
-  public QueryProcessingStage getStage() {
-    return stage;
-  }
-
   public List<String> getLocationChain() {
     return new ArrayList<>(locationChain);
   }
@@ -92,14 +84,6 @@ public Map<String, Object> getContext() {
     return new LinkedHashMap<>(context);
   }
 
-  public String getSuggestion() {
-    return suggestion;
-  }
-
-  public String getDetails() {
-    return details;
-  }
-
   /** Get the original exception type name. */
   public String getExceptionType() {
     return cause.getClass().getSimpleName();
@@ -155,7 +139,7 @@ public Map<String, Object> toJsonMap() {
 
     json.put("type", getExceptionType());
 
-    if (code != null && code != ErrorCode.UNKNOWN) {
+    if (code != null) {
       json.put("code", code.name());
     }
 
@@ -168,12 +152,12 @@ public Map<String, Object> toJsonMap() {
     }
 
     // Build context with stage information included
-    if (!context.isEmpty() || stage != null) {
-      Map<String, Object> contextMap = new LinkedHashMap<>(context);
-      if (stage != null) {
-        contextMap.put("stage", stage.toJsonKey());
-        contextMap.put("stage_description", stage.getDisplayName());
-      }
+    Map<String, Object> contextMap = new LinkedHashMap<>(context);
+    if (stage != null) {
+      contextMap.put("stage", stage.toJsonKey());
+      contextMap.put("stage_description", stage.getDisplayName());
+    }
+    if (!contextMap.isEmpty()) {
       json.put("context", contextMap);
     }
 

common/src/main/java/org/opensearch/sql/common/error/ErrorReportStatusCodeHelper.java

@@ -0,0 +1,102 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package org.opensearch.sql.common.error;
+
+import java.util.function.Predicate;
+
+/**
+ * Helper class for determining HTTP status codes from ErrorReport instances. Provides utilities to
+ * check if an exception (potentially wrapped in ErrorReport) should result in a client error (4xx)
+ * or server error (5xx) status code.
+ */
+public class ErrorReportStatusCodeHelper {
+
+  /**
+   * Determines if an exception should be treated as a client error (400 BAD_REQUEST) based on:
+   *
+   * <ol>
+   *   <li>ErrorCode (if wrapped in ErrorReport)
+   *   <li>Underlying exception type (checked via provided predicate)
+   * </ol>
+   *
+   * @param e the exception to check
+   * @param exceptionTypeChecker predicate that checks if a given exception type is a client error
+   * @return true if this should result in a 4xx status code, false for 5xx
+   */
+  public static boolean isClientError(Exception e, Predicate<Exception> exceptionTypeChecker) {
+    // If wrapped in ErrorReport, check both the ErrorCode and the underlying cause
+    if (e instanceof ErrorReport) {
+      ErrorReport report = (ErrorReport) e;
+
+      // Check if the ErrorCode indicates a client error
+      if (isClientErrorCode(report.getCode())) {
+        return true;
+      }
+
+      // Fall through to check the underlying exception type
+      Throwable cause = e.getCause();
+      if (cause instanceof Exception) {
+        return exceptionTypeChecker.test((Exception) cause);
+      }
+    }
+
+    // Check the exception type directly
+    return exceptionTypeChecker.test(e);
+  }
+
+  /**
+   * Maps ErrorCode values to client error (400) vs server error (500) status codes.
+   *
+   * <p>Client errors (400):
+   *
+   * <ul>
+   *   <li>FIELD_NOT_FOUND - invalid field reference
+   *   <li>SYNTAX_ERROR - query parsing failed
+   *   <li>AMBIGUOUS_FIELD - multiple fields with same name
+   *   <li>SEMANTIC_ERROR - semantic validation failed
+   *   <li>TYPE_ERROR - type mismatch
+   *   <li>UNSUPPORTED_OPERATION - feature not supported
+   *   <li>INDEX_NOT_FOUND - index doesn't exist
+   * </ul>
+   *
+   * <p>Server errors (500):
+   *
+   * <ul>
+   *   <li>EVALUATION_ERROR - runtime evaluation failed
+   *   <li>PLANNING_ERROR - query planning failed
+   *   <li>EXECUTION_ERROR - query execution failed
+   *   <li>RESOURCE_LIMIT_EXCEEDED - system resource limit
+   *   <li>UNKNOWN - unclassified error
+   * </ul>
+   *
+   * @param code the ErrorCode to check
+   * @return true if this ErrorCode indicates a client error (400), false for server error (500)
+   */
+  private static boolean isClientErrorCode(ErrorCode code) {
+    if (code == null) {
+      return false;
+    }
+
+    switch (code) {
+      case FIELD_NOT_FOUND:
+      case SYNTAX_ERROR:
+      case AMBIGUOUS_FIELD:
+      case SEMANTIC_ERROR:
+      case TYPE_ERROR:
+      case UNSUPPORTED_OPERATION:
+      case INDEX_NOT_FOUND:
+        return true;
+
+      case EVALUATION_ERROR:
+      case PLANNING_ERROR:
+      case EXECUTION_ERROR:
+      case RESOURCE_LIMIT_EXCEEDED:
+      case UNKNOWN:
+      default:
+        return false;
+    }
+  }
+}

core/src/main/java/org/opensearch/sql/calcite/QualifiedNameResolver.java

@@ -16,6 +16,8 @@
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.opensearch.sql.ast.expression.QualifiedName;
+import org.opensearch.sql.common.error.ErrorCode;
+import org.opensearch.sql.common.error.ErrorReport;
 import org.opensearch.sql.expression.function.BuiltinFunctionName;
 import org.opensearch.sql.expression.function.PPLFuncImpTable;
 
@@ -322,7 +324,10 @@ private static Optional<RexNode> replaceWithNullLiteralInCoalesce(CalcitePlanCon
     return Optional.empty();
   }
 
-  private static RuntimeException getNotFoundException(QualifiedName node) {
-    return new IllegalArgumentException(String.format("Field [%s] not found.", node.toString()));
+  private static ErrorReport getNotFoundException(QualifiedName node) {
+    return ErrorReport.wrap(
+            new IllegalArgumentException(String.format("Field [%s] not found.", node.toString())))
+        .code(ErrorCode.FIELD_NOT_FOUND)
+        .build();
   }
 }

integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteErrorReportStageIT.java

@@ -49,7 +49,8 @@ public void testFieldNotFoundErrorIncludesStage() throws IOException {
     String stageDescription = context.getString("stage_description");
     assertTrue(
         "Stage description should be user-friendly",
-        stageDescription.contains("Checking") || stageDescription.contains("Query"));
+        stageDescription.toLowerCase().contains("checking")
+            || stageDescription.toLowerCase().contains("query"));
 
     // Verify error has location chain
     assertTrue("Error should have location", error.has("location"));