From 401d0ce0d4b8f10f4dd72137e35c0f9ec6c78271 Mon Sep 17 00:00:00 2001
From: developer-rpai <developer-rpai@users.noreply.github.com>
Date: Thu, 24 Sep 2026 21:36:41 -0700
Subject: [PATCH] fix: do not cache empty Achilles drilldown results on SQL
 failure (fixes #2208)

CDMResultsAnalysisRunner.getDrilldown() caught Exception, logged it and
returned the (possibly empty) ObjectNode. The @AchillesCache aspect on
CDMResultsService.getDrilldown then persisted that empty report, and users
saw empty drilldown reports with no way to re-trigger the query short of
manually deleting the cache record.

Propagate the failure instead: the aspect does not cache thrown results
(it would still cache a null), the batch cache-warmer fails honestly, and
the REST endpoint returns an error rather than a cacheable empty 200.

Adds CDMResultsAnalysisRunnerTest regression test.
---
 .../report/CDMResultsAnalysisRunner.java      |  7 ++-
 .../report/CDMResultsAnalysisRunnerTest.java  | 62 +++++++++++++++++++
 2 files changed, 68 insertions(+), 1 deletion(-)
 create mode 100644 src/test/java/org/ohdsi/webapi/report/CDMResultsAnalysisRunnerTest.java

diff --git a/src/main/java/org/ohdsi/webapi/report/CDMResultsAnalysisRunner.java b/src/main/java/org/ohdsi/webapi/report/CDMResultsAnalysisRunner.java
index 95d110c6c2..4465c728e4 100644
--- a/src/main/java/org/ohdsi/webapi/report/CDMResultsAnalysisRunner.java
+++ b/src/main/java/org/ohdsi/webapi/report/CDMResultsAnalysisRunner.java
@@ -284,7 +284,12 @@ public JsonNode getDrilldown(JdbcTemplate jdbcTemplate,
                 }
             }
         } catch (Exception e) {
-            log.error(e.getMessage(), e);
+            log.error("Error running drilldown query for domain {} and conceptId {}: {}", domain, conceptId, e.getMessage(), e);
+            // Do not swallow the failure: an empty (or partial) result returned here would be
+            // persisted in the Achilles cache by the @AchillesCache aspect (which also caches
+            // nulls), leaving users with empty drilldown reports and no way to re-trigger the query.
+            throw new RuntimeException(String.format(
+                    "Failed to run drilldown query for domain '%s' and conceptId '%s'", domain, conceptId), e);
         }
         return objectNode;
     }
diff --git a/src/test/java/org/ohdsi/webapi/report/CDMResultsAnalysisRunnerTest.java b/src/test/java/org/ohdsi/webapi/report/CDMResultsAnalysisRunnerTest.java
new file mode 100644
index 0000000000..90aad7c543
--- /dev/null
+++ b/src/test/java/org/ohdsi/webapi/report/CDMResultsAnalysisRunnerTest.java
@@ -0,0 +1,62 @@
+package org.ohdsi.webapi.report;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.anyString;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.junit.Before;
+import org.junit.Test;
+import org.ohdsi.webapi.source.Source;
+import org.ohdsi.webapi.source.SourceDaimon;
+import org.springframework.dao.DataAccessResourceFailureException;
+import org.springframework.jdbc.core.JdbcTemplate;
+import org.springframework.jdbc.core.RowMapper;
+
+/**
+ * Regression tests for https://github.com/OHDSI/WebAPI/issues/2208:
+ * a SQL failure during an Achilles drilldown used to be swallowed and the empty
+ * result persisted in the CDM cache (via the @AchillesCache aspect), leaving users
+ * with empty drilldown reports that could not be refreshed.
+ */
+public class CDMResultsAnalysisRunnerTest {
+
+    private CDMResultsAnalysisRunner runner;
+    private JdbcTemplate jdbcTemplate;
+    private Source source;
+
+    @Before
+    public void setUp() {
+
+        runner = new CDMResultsAnalysisRunner();
+        runner.init("postgresql", new ObjectMapper());
+
+        jdbcTemplate = mock(JdbcTemplate.class);
+        source = mock(Source.class);
+        when(source.getTableQualifier(SourceDaimon.DaimonType.Results)).thenReturn("results");
+        when(source.getTableQualifier(SourceDaimon.DaimonType.Vocabulary)).thenReturn("vocab");
+        when(source.getTableQualifier(SourceDaimon.DaimonType.CDM)).thenReturn("cdm");
+        when(source.getSourceDialect()).thenReturn("postgresql");
+    }
+
+    @Test
+    public void getDrilldown_propagatesQueryFailureInsteadOfReturningEmptyResult() {
+
+        // Simulate a SQL failure during the drilldown (e.g. unreachable results schema)
+        when(jdbcTemplate.query(anyString(), any(), any(RowMapper.class)))
+                .thenThrow(new DataAccessResourceFailureException("simulated SQL failure"));
+
+        try {
+            JsonNode result = runner.getDrilldown(jdbcTemplate, "condition", null, source);
+            fail("Expected getDrilldown to propagate the SQL failure instead of returning " + result);
+        } catch (RuntimeException expected) {
+            // The failure must reach the caller so the @AchillesCache aspect does not cache an
+            // empty report; the original SQL exception must be preserved as the cause.
+            assertEquals("simulated SQL failure", expected.getCause().getMessage());
+        }
+    }
+}