(solr) branch branch_9_9 updated: SOLR-17792: Fix error handling for shard requests (#3429)

[houston]

This is an automated email from the ASF dual-hosted git repository.

houston pushed a commit to branch branch_9_9
in repository https://gitbox.apache.org/repos/asf/solr.git


The following commit(s) were added to refs/heads/branch_9_9 by this push:
     new 498568b5dd6 SOLR-17792: Fix error handling for shard requests (#3429)
498568b5dd6 is described below

commit 498568b5dd6d2d7422742bdebd1ec4a500817be2
Author: Houston Putman <hous...@apache.org>
AuthorDate: Fri Jul 18 11:46:02 2025 -0700

    SOLR-17792: Fix error handling for shard requests (#3429)
    
    (cherry picked from commit 71554944de68207ea39e60aae5387b793a5f2c4b)
---
 .../apache/solr/handler/component/HttpShardHandler.java   | 14 ++++++++++++--
 .../org/apache/solr/handler/component/SearchHandler.java  | 15 ++++++++++++---
 2 files changed, 24 insertions(+), 5 deletions(-)

diff --git 
a/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java 
b/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java
index 4a0e9b46d73..36c62aae972 100644
--- a/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java
@@ -267,7 +267,7 @@ public class HttpShardHandler extends ShardHandler {
     // Synchronize on canceled, so that we know precisely whether to add it to 
the responseFutureMap
     // or not.
     synchronized (canceled) {
-      if (canceled.get()) {
+      if (canceled.get() && !future.isDone()) {
         future.cancel(true);
         return;
       } else {
@@ -335,7 +335,17 @@ public class HttpShardHandler extends ShardHandler {
           responses.clear();
 
           // We want to return the last response we received, if possible
-          return previousResponse;
+          if (previousResponse == null) {
+            return null;
+          } else {
+            // If we have been canceled, return a response from this request 
that has an exception,
+            // since that is the contract of the method.
+            // Otherwise return the last response that we processed.
+            return previousResponse.getShardRequest().responses.stream()
+                .filter(sr -> sr.getException() != null)
+                .findFirst()
+                .orElse(previousResponse);
+          }
         } else {
           responseFutureMap.remove(rsp);
 
diff --git 
a/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java 
b/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java
index 91abd23447e..17b0957aace 100644
--- a/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java
@@ -37,6 +37,8 @@ import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicLong;
@@ -630,10 +632,17 @@ public class SearchHandler extends RequestHandlerBase
               rsp.setPartialResults(rb.req);
             }
             // Was there an exception?
-            if (srsp.getException() != null) {
+            // In the case of tolerant search, we need to check all responses 
to see if there was an
+            // exception.
+            Optional<Throwable> shardException =
+                srsp.getShardRequest().responses.stream()
+                    .map(ShardResponse::getException)
+                    .filter(Objects::nonNull)
+                    .findFirst();
+            if (shardException.isPresent()) {
               // If things are not tolerant, abort everything and rethrow
               if (!tolerant) {
-                throwSolrException(srsp.getException());
+                throwSolrException(shardException.get());
               } else {
                 // Check if the purpose includes 'PURPOSE_GET_TOP_IDS'
                 boolean includesTopIdsPurpose =
@@ -646,7 +655,7 @@ public class SearchHandler extends RequestHandlerBase
                 boolean allShardsFailed = includesTopIdsPurpose && 
allResponsesHaveExceptions;
                 // if all shards fail, fail the request despite shards.tolerant
                 if (allShardsFailed) {
-                  throwSolrException(srsp.getException());
+                  throwSolrException(shardException.get());
                 } else {
                   rsp.setPartialResults(rb.req);
                 }