From a991b3d9679492bafe7e44baf4cf8c91e7568858 Mon Sep 17 00:00:00 2001 From: ihoffmann-dot Date: Wed, 9 Sep 2026 23:07:52 -0300 Subject: [PATCH] fix(content-drive): fail request instead of silently returning partial results on ES sub-query failure --- .../com/dotcms/browser/BrowserAPIImpl.java | 73 +++++++++++++------ 1 file changed, 50 insertions(+), 23 deletions(-) diff --git a/dotCMS/src/main/java/com/dotcms/browser/BrowserAPIImpl.java b/dotCMS/src/main/java/com/dotcms/browser/BrowserAPIImpl.java index cec5b06df168..808aa82cf15d 100644 --- a/dotCMS/src/main/java/com/dotcms/browser/BrowserAPIImpl.java +++ b/dotCMS/src/main/java/com/dotcms/browser/BrowserAPIImpl.java @@ -770,8 +770,12 @@ static class ContentUnderParent { * @param browserQuery The {@link BrowserQuery} containing search criteria (filter, fileName) * @param inodes The set of inodes to filter through Elasticsearch text search * @return A filtered set of inodes that match the text search criteria + * @throws DotDataException if a sub-query fails or times out. A failure here is deliberately + * NOT swallowed into an empty result: silently dropping a failed + * sub-query's share of matches would return an incomplete page as a + * successful (HTTP 200) response, which is worse than a visible error. */ - Set processESDirectly(BrowserQuery browserQuery, Set inodes) { + Set processESDirectly(BrowserQuery browserQuery, Set inodes) throws DotDataException { if (inodes == null || inodes.isEmpty()) { return new LinkedHashSet<>(); } @@ -866,8 +870,14 @@ private int countApproximateClausesInQuery(String query) { * @param startTime The time when this method was called, for performance analysis purposes. * * @return A set of Inodes matching the query. - */ - private Set processSingleESQuery(final BrowserQuery browserQuery, final Set inodes, final long startTime) { + * @throws DotDataException if the underlying ES search fails. Previously this method caught + * every exception, logged it, and returned an empty set — which let a + * failed sub-query silently drop its share of matches from the overall + * response instead of failing the request. The exception is now + * propagated so the caller can fail the request instead of returning + * an incomplete page as if it were successful. + */ + private Set processSingleESQuery(final BrowserQuery browserQuery, final Set inodes, final long startTime) throws DotDataException { final boolean live = !browserQuery.showWorking; final SearchAPI searchAPI = APILocator.getSearchAPI(); final List collectedInodes = new ArrayList<>(); @@ -891,7 +901,9 @@ private Set processSingleESQuery(final BrowserQuery browserQuery, final inodes.size(), collectedInodes.size(), duration)); } catch (final Exception e) { - Logger.error(this, String.format("Single ES query failed for %d inodes: %s", inodes.size(), getErrorMessage(e)), e); + final String errorMsg = String.format("Single ES query failed for %d inodes: %s", inodes.size(), getErrorMessage(e)); + Logger.error(this, errorMsg, e); + throw new DotDataException(errorMsg, e); } return new LinkedHashSet<>(collectedInodes); @@ -900,9 +912,20 @@ private Set processSingleESQuery(final BrowserQuery browserQuery, final /** * Processes multiple ES queries when inode count exceeds the limit. * Uses parallel processing for better performance. + *

+ * A failed or timed-out sub-query aborts the whole call with a {@link DotDataException} rather + * than silently substituting an empty result for that sub-batch. Previously each future's + * {@code .exceptionally(...)} fallback swallowed the failure and returned an empty set, and the + * outer {@code allFutures.get(...)} timeout/execution errors were only logged — so a single + * flaky sub-query quietly removed its share of matches and the caller still got back a + * "successful" (but incomplete) page. With a fan-out of many sub-queries per request, that + * silent-partial-result risk is no longer negligible. + *

+ * + * @throws DotDataException if any sub-query fails, times out, or the overall wait times out */ private Set processMultipleESQueries(BrowserQuery browserQuery, Set inodes, - int maxInodesPerQuery, long startTime) { + int maxInodesPerQuery, long startTime) throws DotDataException { final Set allResults = Collections.synchronizedSet(new LinkedHashSet<>()); final List inodesList = new ArrayList<>(inodes); final int totalInodes = inodesList.size(); @@ -930,29 +953,26 @@ private Set processMultipleESQueries(BrowserQuery browserQuery, Set { Logger.debug(BrowserAPIImpl.this, String.format("Processing ES sub-query %d/%d: %d inodes", batchIndex, batchCount, batch.size())); - return processSingleESQuery(browserQuery, new LinkedHashSet<>(batch), System.currentTimeMillis()); + try { + return processSingleESQuery(browserQuery, new LinkedHashSet<>(batch), System.currentTimeMillis()); + } catch (final DotDataException e) { + // Rethrow as unchecked so it surfaces through the future's exceptional + // completion instead of being caught here and papered over. + throw new DotRuntimeException(e.getMessage(), e); + } }, submitter) - .orTimeout(60, TimeUnit.SECONDS) - .exceptionally(throwable -> { - Logger.error(BrowserAPIImpl.this, String.format("ES sub-query %d failed: %s", - batchIndex, throwable.getMessage()), throwable); - return new LinkedHashSet<>(); - }); + .orTimeout(60, TimeUnit.SECONDS); } - // Collect results from all sub-queries + // Collect results from all sub-queries. Any failure (including a timeout) here is + // deliberately allowed to propagate — a visible error is recoverable, a quietly + // incomplete page is not. try { CompletableFuture allFutures = CompletableFuture.allOf(futures); allFutures.get(120, TimeUnit.SECONDS); for (CompletableFuture> future : futures) { - try { - Set batchResults = future.get(); - allResults.addAll(batchResults); - } catch (Exception e) { - Logger.warn(this, "Failed to get result from ES sub-query future: " + e.getMessage()); - Thread.currentThread().interrupt(); - } + allResults.addAll(future.get()); } final long totalDuration = System.currentTimeMillis() - startTime; @@ -960,12 +980,19 @@ private Set processMultipleESQueries(BrowserQuery browserQuery, Set