From b1fb99d0e0f70c0dd2489d44a03c187a43b7c325 Mon Sep 17 00:00:00 2001 From: Finn Carroll Date: Wed, 17 Jun 2026 21:11:21 -0700 Subject: [PATCH] Redact internal details from unrecognized analytics engine 500s Unrecognized exceptions (those not converted by NativeErrorConverter to 400/429) now return a generic 'Internal error [task_id=X, query_id=Y]' message to the user instead of leaking internal details (stage IDs, shard routing, gRPC metadata, native error messages, planner internals). The full stack trace is logged at ERROR level server-side so operators can still diagnose issues using the task/query ID as a correlator. Exceptions with well-defined HTTP semantics (IllegalArgumentException -> 400, CircuitBreakingException -> 429, RejectedExecution -> 429) are passed through unchanged since their messages are user-facing by design. Signed-off-by: Finn Carroll --- .../analytics/exec/DefaultPlanExecutor.java | 25 ++++++++++++++++++- .../analytics/planner/IndexResolution.java | 4 +-- .../planner/IndexResolutionTests.java | 8 +++--- 3 files changed, 30 insertions(+), 7 deletions(-) diff --git a/sandbox/plugins/analytics-engine/src/main/java/org/opensearch/analytics/exec/DefaultPlanExecutor.java b/sandbox/plugins/analytics-engine/src/main/java/org/opensearch/analytics/exec/DefaultPlanExecutor.java index dd6d0dc9ef403..71d89b408a075 100644 --- a/sandbox/plugins/analytics-engine/src/main/java/org/opensearch/analytics/exec/DefaultPlanExecutor.java +++ b/sandbox/plugins/analytics-engine/src/main/java/org/opensearch/analytics/exec/DefaultPlanExecutor.java @@ -17,6 +17,7 @@ import org.apache.calcite.rel.metadata.RelMetadataQueryBase; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.opensearch.ExceptionsHelper; import org.opensearch.action.support.ActionFilters; import org.opensearch.action.support.HandledTransportAction; import org.opensearch.action.support.TimeoutTaskCancellationUtility; @@ -50,6 +51,7 @@ import org.opensearch.common.inject.Inject; import org.opensearch.common.unit.TimeValue; import org.opensearch.core.action.ActionListener; +import org.opensearch.core.rest.RestStatus; import org.opensearch.core.tasks.TaskId; import org.opensearch.search.SearchService; import org.opensearch.tasks.Task; @@ -409,7 +411,21 @@ protected void doExecute(Task task, AnalyticsQueryRequest request, ActionListene // immediately. The listener is wrapped to convert backend-specific exceptions. ActionListener convertingListener = ActionListener.wrap(listener::onResponse, e -> { Exception converted = e instanceof Exception ex ? contextProvider.convertException(ex) : new RuntimeException(e); - listener.onFailure(converted); + // If convertException returned unrecognized 500 — redact internal details and log the original. + if (converted == e && isInternalError(converted)) { + AnalyticsQueryTask queryTask = (AnalyticsQueryTask) task; + String queryId = queryTask.getQueryId(); + String identifier = "unassigned".equals(queryId) + ? "task_id=" + task.getId() + : "task_id=" + task.getId() + ", query_id=" + queryId; + logger.error( + new org.apache.logging.log4j.message.ParameterizedMessage("[analytics-engine] internal error [{}]", identifier), + converted + ); + listener.onFailure(new RuntimeException("Internal error [" + identifier + "]")); + } else { + listener.onFailure(converted); + } }); ContextAwareExecutor.wrap(searchExecutor, threadPool).execute(() -> { try { @@ -443,6 +459,13 @@ protected void doExecute(Task task, AnalyticsQueryRequest request, ActionListene }); } + /** + * Returns true if the exception would produce a 500 response and should be redacted. + */ + private static boolean isInternalError(Exception e) { + return ExceptionsHelper.status(e) == RestStatus.INTERNAL_SERVER_ERROR; + } + /** * Materializes Arrow batches into row-oriented {@code Object[]}s for the * external query API. The scheduler yields batches (the native wire format); diff --git a/sandbox/plugins/analytics-engine/src/main/java/org/opensearch/analytics/planner/IndexResolution.java b/sandbox/plugins/analytics-engine/src/main/java/org/opensearch/analytics/planner/IndexResolution.java index 4cf9cdaee1b0c..7cdb19d7aec62 100644 --- a/sandbox/plugins/analytics-engine/src/main/java/org/opensearch/analytics/planner/IndexResolution.java +++ b/sandbox/plugins/analytics-engine/src/main/java/org/opensearch/analytics/planner/IndexResolution.java @@ -195,7 +195,7 @@ private static IndexResolution resolveAlias(String aliasName, List IndexResolution.resolve("bank_all", state)); + IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, () -> IndexResolution.resolve("bank_all", state)); assertTrue("error must mention the conflicting field: " + ex.getMessage(), ex.getMessage().contains("age")); assertTrue( "error must mention both indices: " + ex.getMessage(), @@ -103,7 +103,7 @@ public void testAliasRejectsFilterAlias() { IndexMetadata.Builder a = indexBuilder("bank_a", longField("age")).putAlias(filterAlias); ClusterState state = clusterStateOf(a); - IllegalStateException ex = expectThrows(IllegalStateException.class, () -> IndexResolution.resolve("active_only", state)); + IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, () -> IndexResolution.resolve("active_only", state)); assertTrue("error must mention the alias name: " + ex.getMessage(), ex.getMessage().contains("active_only")); assertTrue("error must mention 'filter': " + ex.getMessage(), ex.getMessage().toLowerCase(Locale.ROOT).contains("filter")); } @@ -142,7 +142,7 @@ public void testCommaSeparatedExpressionResolvesToUnion() { public void testWildcardRejectsIncompatibleSchemasAcrossMatches() { ClusterState state = clusterStateOf(indexBuilder("test", longField("age")), indexBuilder("test1", keywordField("age"))); - IllegalStateException ex = expectThrows(IllegalStateException.class, () -> IndexResolution.resolve("test*", state, RESOLVER)); + IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, () -> IndexResolution.resolve("test*", state, RESOLVER)); assertTrue("error must mention the conflicting field: " + ex.getMessage(), ex.getMessage().contains("age")); } @@ -268,7 +268,7 @@ public void testDataStreamRejectsConflictingBackingMappings() { ) .build(); - IllegalStateException ex = expectThrows(IllegalStateException.class, () -> IndexResolution.resolve("logs", state, RESOLVER)); + IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, () -> IndexResolution.resolve("logs", state, RESOLVER)); assertTrue("error must mention the conflicting field: " + ex.getMessage(), ex.getMessage().contains("age")); }