From cdd1c2876c10b1cae740c6b64d82eb59347f412b Mon Sep 17 00:00:00 2001 From: TSFenwick Date: Wed, 10 Nov 2021 17:22:04 -0800 Subject: [PATCH] catch throwable because calcite is throwing an error not exception (#11892) * catch throwable because calcite is throwing an error not exception * add test case --- .../apache/druid/sql/http/SqlResource.java | 5 ++- .../druid/sql/http/SqlResourceTest.java | 38 +++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/sql/src/main/java/org/apache/druid/sql/http/SqlResource.java b/sql/src/main/java/org/apache/druid/sql/http/SqlResource.java index 68152924bd2..3e766960f5c 100644 --- a/sql/src/main/java/org/apache/druid/sql/http/SqlResource.java +++ b/sql/src/main/java/org/apache/druid/sql/http/SqlResource.java @@ -194,11 +194,12 @@ public class SqlResource endLifecycleWithoutEmittingMetrics(sqlQueryId, lifecycle); throw (ForbiddenException) serverConfig.getErrorResponseTransformStrategy().transformIfNeeded(e); // let ForbiddenExceptionMapper handle this } - catch (Exception e) { + // calcite throws a java.lang.AssertionError which is type error not exception. using throwable will catch all + catch (Throwable e) { log.warn(e, "Failed to handle query: %s", sqlQuery); endLifecycle(sqlQueryId, lifecycle, e, remoteAddr, -1); - final Exception exceptionToReport; + final Throwable exceptionToReport; if (e instanceof RelOptPlanner.CannotPlanException) { exceptionToReport = new ISE("Cannot build plan for query: %s", sqlQuery.getQuery()); diff --git a/sql/src/test/java/org/apache/druid/sql/http/SqlResourceTest.java b/sql/src/test/java/org/apache/druid/sql/http/SqlResourceTest.java index 6da9ccbfec5..8ab76d2f16e 100644 --- a/sql/src/test/java/org/apache/druid/sql/http/SqlResourceTest.java +++ b/sql/src/test/java/org/apache/druid/sql/http/SqlResourceTest.java @@ -1048,6 +1048,44 @@ public class SqlResourceTest extends CalciteTestBase Assert.assertTrue(lifecycleManager.getAll("id").isEmpty()); } + @Test + public void testAssertionErrorThrowsErrorWithFilterResponse() throws Exception + { + resource = new SqlResource( + JSON_MAPPER, + CalciteTests.TEST_AUTHORIZER_MAPPER, + sqlLifecycleFactory, + lifecycleManager, + new ServerConfig() { + @Override + public boolean isShowDetailedJettyErrors() + { + return true; + } + @Override + public ErrorResponseTransformStrategy getErrorResponseTransformStrategy() + { + return new AllowedRegexErrorResponseTransformStrategy(ImmutableList.of()); + } + } + ); + + String errorMessage = "could not assert"; + SqlQuery badQuery = EasyMock.createMock(SqlQuery.class); + EasyMock.expect(badQuery.getQuery()).andReturn("SELECT ANSWER TO LIFE"); + EasyMock.expect(badQuery.getContext()).andReturn(ImmutableMap.of("sqlQueryId", "id")); + EasyMock.expect(badQuery.getParameterList()).andThrow(new Error(errorMessage)); + EasyMock.replay(badQuery); + final QueryException exception = doPost(badQuery).lhs; + + Assert.assertNotNull(exception); + Assert.assertNull(exception.getMessage()); + Assert.assertNull(exception.getHost()); + Assert.assertEquals(exception.getErrorCode(), QueryInterruptedException.UNKNOWN_EXCEPTION); + Assert.assertNull(exception.getErrorClass()); + Assert.assertTrue(lifecycleManager.getAll("id").isEmpty()); + } + @Test public void testTooManyRequests() throws Exception {