fix(content-drive): fail request instead of silently returning partial results on ES sub-query failure (#37488)

[ihoffmann-dot]

Summary

• processSingleESQuery / processMultipleESQueries in BrowserAPIImpl used to swallow ES sub-query exceptions/timeouts and quietly return an empty result, so a failed or slow sub-query silently dropped its share of matches and the caller got HTTP 200 with a short/incomplete page.
• Now a sub-query failure propagates as a DotDataException/DotRuntimeException, so the request surfaces a visible error instead of a silently incomplete page.

Fixes #37488. Follow-up from #37395 (single-pass field filter resolution) review — flagged as pre-existing behavior made more likely to matter once that PR widens the ES fan-out per request.

Caller audit

Traced every caller of processSingleESQuery/processMultipleESQueries: the only path is processESDirectly → getChunkFiltered → getContentByChunks → doHybridSingleChunkedQueryES → doElasticSearchTextFiltering → getContentUnderParentFromDB, which already declares/wraps DotDataException and rethrows as DotRuntimeException. No caller depended on the old empty-result-on-failure fallback.

Test plan

• ./mvnw compile -pl :dotcms-core — clean
• Integration test forcing a sub-query failure/timeout and asserting the request surfaces an error (flagged as needing DB/ES-level mocking beyond this unit test file's current scaffolding — see PR discussion)

🤖 Generated with Claude Code

[github-actions]

❌ Linked Issue Needs Team Label

This PR is linked to issue #37488, but that issue has no Team : * label. Every linked issue must be owned by a team for tracking and triage.

How to fix this:

Apply a Team : * label to the linked issue (e.g., Team : Scout, Team : Platform, Team : Falcon, Team : Maintenance). Then push a new commit or edit the PR description to re-run this check.

---

This comment was automatically generated by the issue linking workflow

[claude]

Claude finished @ihoffmann-dot's task in 1m 35s —— View job

---

Code Review

• Read the diff (BrowserAPIImpl.java)
• Trace caller chain for exception propagation
• Verify imports & concurrency semantics
• Check for test breakage from signature changes

The change is correct and does what it claims. The throws DotDataException propagates cleanly through the whole chain — processESDirectly → getChunkFiltered → getContentByChunks → doElasticSearchTextFiltering, all of which already declare DotDataException (and doElasticSearchTextFiltering rethrows DotSecurityException as DotRuntimeException). No caller relied on the old empty-result fallback. No existing unit test references the private methods, so no test breakage from the signature changes.

Concurrency semantics check out: since CompletableFuture.allOf(futures) completes exceptionally when any component future does, allFutures.get(120, ...) (line 972) throws first on any sub-query failure/timeout, so the subsequent future.get() loop (line 975) never blocks and never throws on a completed-normally future. The old per-future catch that spuriously called Thread.currentThread().interrupt() on an ExecutionException is correctly gone.

New Issues

• 🟡 Medium: dotCMS/src/main/java/com/dotcms/browser/BrowserAPIImpl.java:970-996 — On the first sub-query failure, the method now aborts (throws) while the sibling futures are left running. Nothing cancels them — neither allOf, orTimeout, nor the new throw path calls future.cancel(...) or cancels the DotSubmitter tasks. The orphaned ES sub-queries keep occupying shared submitter threads until they finish on their own, even though their results are discarded. Pre-existing code also didn't cancel, but the fail-fast throw makes the wasted work more visible (request errors out immediately while N-1 batches still churn). Worth a for (var f : futures) f.cancel(true); in the failure branches. Non-blocking.

Notes (non-blocking, not introduced by this PR)

• dotCMS/src/main/java/com/dotcms/browser/BrowserAPIImpl.java:992 — The outer TimeoutException catch on allFutures.get(120, SECONDS) is effectively unreachable: every future carries orTimeout(60, SECONDS), so allOf always completes (normally or exceptionally) within ~60s and a sub-query timeout surfaces as an ExecutionException wrapping TimeoutException, not as the outer 120s timeout. Harmless, but the 120s branch is dead. (The orTimeout predates this PR.)

The behavior change itself is intentional per #37488: transient ES hiccups on a browse/Content Drive request will now surface as an error rather than a silently short page. That's the stated goal.

The unchecked integration-test box in the PR body is the real gap — a test forcing a sub-query failure and asserting the request surfaces an error would lock in this behavior. Understandable that it needs ES/DB-level mocking beyond the current unit scaffolding, but worth a follow-up.
· issue-37488-content-drive-es-subquery-failfast