From 6cd7878f362c8ac7a6a745e2abf287c2bf227771 Mon Sep 17 00:00:00 2001 From: JackieTien97 Date: Sat, 14 Nov 2026 18:43:14 +0800 Subject: [PATCH] Avoid stack traces for expected REST query errors --- .../rest/v1/handler/ExceptionHandler.java | 3 +- .../rest/v2/handler/ExceptionHandler.java | 3 +- .../iotdb/db/utils/ErrorHandlingUtils.java | 58 ++++--- .../rest/handler/ExceptionHandlerTest.java | 151 ++++++++++++++++++ 4 files changed, 187 insertions(+), 28 deletions(-) create mode 100644 iotdb-core/datanode/src/test/java/org/apache/iotdb/db/protocol/rest/handler/ExceptionHandlerTest.java diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/protocol/rest/v1/handler/ExceptionHandler.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/protocol/rest/v1/handler/ExceptionHandler.java index acc705181bef4..7ae230b7261e8 100644 --- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/protocol/rest/v1/handler/ExceptionHandler.java +++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/protocol/rest/v1/handler/ExceptionHandler.java @@ -28,6 +28,7 @@ import org.apache.iotdb.db.exception.sql.StatementAnalyzeException; import org.apache.iotdb.db.protocol.rest.exception.RequestLimitExceededException; import org.apache.iotdb.db.protocol.rest.v1.model.ExecutionStatus; +import org.apache.iotdb.db.utils.ErrorHandlingUtils; import org.apache.iotdb.rpc.TSStatusCode; import org.antlr.v4.runtime.misc.ParseCancellationException; @@ -85,7 +86,7 @@ public static ExecutionStatus tryCatchException(Exception e) { responseResult.setMessage(e.getMessage()); responseResult.setCode(TSStatusCode.INTERNAL_SERVER_ERROR.getStatusCode()); } - LOGGER.warn(e.getMessage(), e); + ErrorHandlingUtils.logQueryException(LOGGER, e.getMessage(), responseResult.getCode(), e); return responseResult; } diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/protocol/rest/v2/handler/ExceptionHandler.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/protocol/rest/v2/handler/ExceptionHandler.java index 1b989e46220d0..be366786089ad 100644 --- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/protocol/rest/v2/handler/ExceptionHandler.java +++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/protocol/rest/v2/handler/ExceptionHandler.java @@ -28,6 +28,7 @@ import org.apache.iotdb.db.exception.sql.StatementAnalyzeException; import org.apache.iotdb.db.protocol.rest.exception.RequestLimitExceededException; import org.apache.iotdb.db.protocol.rest.model.ExecutionStatus; +import org.apache.iotdb.db.utils.ErrorHandlingUtils; import org.apache.iotdb.rpc.TSStatusCode; import org.antlr.v4.runtime.misc.ParseCancellationException; @@ -85,7 +86,7 @@ public static ExecutionStatus tryCatchException(Exception e) { responseResult.setMessage(e.getMessage()); responseResult.setCode(TSStatusCode.INTERNAL_SERVER_ERROR.getStatusCode()); } - LOGGER.warn(e.getMessage(), e); + ErrorHandlingUtils.logQueryException(LOGGER, e.getMessage(), responseResult.getCode(), e); return responseResult; } diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/utils/ErrorHandlingUtils.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/utils/ErrorHandlingUtils.java index 8da705ea6dab9..5813ab0205a71 100644 --- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/utils/ErrorHandlingUtils.java +++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/utils/ErrorHandlingUtils.java @@ -109,32 +109,11 @@ public static TSStatus onThriftFrameOversizeException(Throwable t) { public static TSStatus onQueryException(Exception e, String operation, TSStatusCode statusCode) { TSStatus status = tryCatchQueryException(e); if (status != null) { - // ignore logging sg not ready exception - if (status.getCode() != TSStatusCode.STORAGE_ENGINE_NOT_READY.getStatusCode()) { - String message = - String.format( - "Status code: %s, Query Statement: %s failed because %s", - status.getCode(), operation, status.getMessage()); - if (status.getCode() == TSStatusCode.SQL_PARSE_ERROR.getStatusCode() - || status.getCode() == TSStatusCode.SEMANTIC_ERROR.getStatusCode() - || status.getCode() == TSStatusCode.NO_PERMISSION.getStatusCode() - || status.getCode() == TSStatusCode.ILLEGAL_PATH.getStatusCode() - || status.getCode() == TSStatusCode.NUMERIC_VALUE_OUT_OF_RANGE.getStatusCode() - || status.getCode() == TSStatusCode.DIVISION_BY_ZERO.getStatusCode() - || status.getCode() == TSStatusCode.DATE_OUT_OF_RANGE.getStatusCode() - || status.getCode() == TSStatusCode.UDF_LOAD_CLASS_ERROR.getStatusCode() - || status.getCode() == TSStatusCode.PLAN_FAILED_NETWORK_PARTITION.getStatusCode() - || status.getCode() == TSStatusCode.SYNC_CONNECTION_ERROR.getStatusCode() - || status.getCode() == TSStatusCode.CANNOT_FETCH_FI_STATE.getStatusCode() - || status.getCode() == TSStatusCode.TEMPLATE_INCOMPATIBLE.getStatusCode() - || status.getCode() == TSStatusCode.PATH_ALREADY_EXIST.getStatusCode() - || status.getCode() == TSStatusCode.PIPE_NOT_EXIST_ERROR.getStatusCode() - || status.getCode() == TSStatusCode.QUERY_TIMEOUT.getStatusCode()) { - LOGGER.info(message); - } else { - LOGGER.warn(message, e); - } - } + String message = + String.format( + "Status code: %s, Query Statement: %s failed because %s", + status.getCode(), operation, status.getMessage()); + logQueryException(LOGGER, message, status.getCode(), e); status.setNeedRetry(needRetry(status)); return status; } else { @@ -142,6 +121,33 @@ public static TSStatus onQueryException(Exception e, String operation, TSStatusC } } + /** Logs expected query errors without stack traces and retains traces for unexpected errors. */ + public static void logQueryException(Logger logger, String message, int statusCode, Exception e) { + // ignore logging sg not ready exception + if (statusCode == TSStatusCode.STORAGE_ENGINE_NOT_READY.getStatusCode()) { + return; + } + if (statusCode == TSStatusCode.SQL_PARSE_ERROR.getStatusCode() + || statusCode == TSStatusCode.SEMANTIC_ERROR.getStatusCode() + || statusCode == TSStatusCode.NO_PERMISSION.getStatusCode() + || statusCode == TSStatusCode.ILLEGAL_PATH.getStatusCode() + || statusCode == TSStatusCode.NUMERIC_VALUE_OUT_OF_RANGE.getStatusCode() + || statusCode == TSStatusCode.DIVISION_BY_ZERO.getStatusCode() + || statusCode == TSStatusCode.DATE_OUT_OF_RANGE.getStatusCode() + || statusCode == TSStatusCode.UDF_LOAD_CLASS_ERROR.getStatusCode() + || statusCode == TSStatusCode.PLAN_FAILED_NETWORK_PARTITION.getStatusCode() + || statusCode == TSStatusCode.SYNC_CONNECTION_ERROR.getStatusCode() + || statusCode == TSStatusCode.CANNOT_FETCH_FI_STATE.getStatusCode() + || statusCode == TSStatusCode.TEMPLATE_INCOMPATIBLE.getStatusCode() + || statusCode == TSStatusCode.PATH_ALREADY_EXIST.getStatusCode() + || statusCode == TSStatusCode.PIPE_NOT_EXIST_ERROR.getStatusCode() + || statusCode == TSStatusCode.QUERY_TIMEOUT.getStatusCode()) { + logger.info(message); + } else { + logger.warn(message, e); + } + } + public static TSStatus onQueryException(Exception e, String operation) { return onQueryException(e, operation, TSStatusCode.INTERNAL_SERVER_ERROR); } diff --git a/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/protocol/rest/handler/ExceptionHandlerTest.java b/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/protocol/rest/handler/ExceptionHandlerTest.java new file mode 100644 index 0000000000000..13598c0294dae --- /dev/null +++ b/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/protocol/rest/handler/ExceptionHandlerTest.java @@ -0,0 +1,151 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.iotdb.db.protocol.rest.handler; + +import org.apache.iotdb.commons.exception.IoTDBException; +import org.apache.iotdb.db.exception.sql.SemanticException; +import org.apache.iotdb.rpc.TSStatusCode; + +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; +import org.antlr.v4.runtime.misc.ParseCancellationException; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.slf4j.LoggerFactory; + +import java.util.Arrays; +import java.util.Collection; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; + +@RunWith(Parameterized.class) +public class ExceptionHandlerTest { + @Parameterized.Parameters(name = "REST v{0}") + public static Collection versions() { + return Arrays.asList(new Object[][] {{1}, {2}}); + } + + private final int version; + + public ExceptionHandlerTest(int version) { + this.version = version; + } + + @Test + public void testParseAndSemanticErrorsDoNotLogStackTraces() { + assertException( + new ParseCancellationException("invalid SQL"), TSStatusCode.SQL_PARSE_ERROR, Level.INFO); + assertException( + new SemanticException("invalid query"), TSStatusCode.SEMANTIC_ERROR, Level.INFO); + } + + @Test + public void testExpectedStatusCodesDoNotLogStackTraces() { + for (TSStatusCode code : + new TSStatusCode[] { + TSStatusCode.SQL_PARSE_ERROR, + TSStatusCode.SEMANTIC_ERROR, + TSStatusCode.NO_PERMISSION, + TSStatusCode.ILLEGAL_PATH, + TSStatusCode.NUMERIC_VALUE_OUT_OF_RANGE, + TSStatusCode.DIVISION_BY_ZERO, + TSStatusCode.DATE_OUT_OF_RANGE, + TSStatusCode.UDF_LOAD_CLASS_ERROR, + TSStatusCode.PLAN_FAILED_NETWORK_PARTITION, + TSStatusCode.SYNC_CONNECTION_ERROR, + TSStatusCode.CANNOT_FETCH_FI_STATE, + TSStatusCode.TEMPLATE_INCOMPATIBLE, + TSStatusCode.PATH_ALREADY_EXIST, + TSStatusCode.PIPE_NOT_EXIST_ERROR, + TSStatusCode.QUERY_TIMEOUT + }) { + assertException( + new IoTDBException("expected failure", code.getStatusCode()), code, Level.INFO); + } + } + + @Test + public void testUnexpectedErrorsKeepStackTraces() { + assertException( + new NullPointerException("unexpected failure"), + TSStatusCode.INTERNAL_SERVER_ERROR, + Level.WARN); + assertException( + new IoTDBException("query failure", TSStatusCode.QUERY_PROCESS_ERROR.getStatusCode()), + TSStatusCode.QUERY_PROCESS_ERROR, + Level.WARN); + } + + @Test + public void testStorageEngineNotReadyIsNotLogged() { + assertException( + new IoTDBException("not ready", TSStatusCode.STORAGE_ENGINE_NOT_READY.getStatusCode()), + TSStatusCode.STORAGE_ENGINE_NOT_READY, + null); + } + + private void assertException(Exception exception, TSStatusCode code, Level expectedLevel) { + Logger logger = + (Logger) + LoggerFactory.getLogger( + version == 1 + ? org.apache.iotdb.db.protocol.rest.v1.handler.ExceptionHandler.class + : org.apache.iotdb.db.protocol.rest.v2.handler.ExceptionHandler.class); + Level originalLevel = logger.getLevel(); + ListAppender appender = new ListAppender<>(); + appender.start(); + logger.addAppender(appender); + logger.setLevel(Level.INFO); + try { + if (version == 1) { + org.apache.iotdb.db.protocol.rest.v1.model.ExecutionStatus response = + org.apache.iotdb.db.protocol.rest.v1.handler.ExceptionHandler.tryCatchException( + exception); + assertEquals(code.getStatusCode(), response.getCode().intValue()); + assertEquals(exception.getMessage(), response.getMessage()); + } else { + org.apache.iotdb.db.protocol.rest.model.ExecutionStatus response = + org.apache.iotdb.db.protocol.rest.v2.handler.ExceptionHandler.tryCatchException( + exception); + assertEquals(code.getStatusCode(), response.getCode().intValue()); + assertEquals(exception.getMessage(), response.getMessage()); + } + assertEquals(expectedLevel == null ? 0 : 1, appender.list.size()); + if (expectedLevel != null) { + ILoggingEvent event = appender.list.get(0); + assertEquals(expectedLevel, event.getLevel()); + assertEquals(exception.getMessage(), event.getFormattedMessage()); + if (expectedLevel == Level.INFO) { + assertNull(event.getThrowableProxy()); + } else { + assertNotNull(event.getThrowableProxy()); + assertEquals(exception.getClass().getName(), event.getThrowableProxy().getClassName()); + } + } + } finally { + logger.detachAppender(appender); + logger.setLevel(originalLevel); + appender.stop(); + } + } +}