From c238c0867906eba5ef7644cd376ac1a221170127 Mon Sep 17 00:00:00 2001
From: Justin Sweeney <justinsweeney@fullstory.com>
Date: Thu, 7 Sep 2023 08:52:35 -0600
Subject: [PATCH] Handling BAD_REQUEST exceptions as failures even with
 shard.tolerant as true (#139)

* Handling BAD_REQUEST exceptions as failures even with shard.tolerant as true

* Fixing forbiddenapi use

* Updating the set of error codes to just be a Set
---
 .../solr/handler/component/SearchHandler.java       |  7 ++++++-
 .../solr/handler/component/ShardResponse.java       |  4 ++++
 .../solr/handler/component/SearchHandlerTest.java   | 13 +++++++++++++
 3 files changed, 23 insertions(+), 1 deletion(-)

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 89316672cba..e7093c44b58 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
@@ -92,6 +92,9 @@ public class SearchHandler extends RequestHandlerBase
 
   protected static final String SHARD_HANDLER_SUFFIX = "[shard]";
 
+  private static final Set<SolrException.ErrorCode> NONTOLERANT_ERROR_CODES =
+      Set.of(SolrException.ErrorCode.BAD_REQUEST);
+
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
   /** A counter to ensure that no RID is equal, even if they fall in the same millisecond */
@@ -572,7 +575,9 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw
             if (srsp.getException() != null) {
               log.warn("Shard request failed : {}", srsp);
               // If things are not tolerant, abort everything and rethrow
-              if (!tolerant) {
+              if (!tolerant
+                  || NONTOLERANT_ERROR_CODES.contains(
+                      SolrException.ErrorCode.getErrorCode(srsp.getRspCode()))) {
                 shardHandler1.cancelAll();
                 if (srsp.getException() instanceof SolrException) {
                   throw (SolrException) srsp.getException();
diff --git a/solr/core/src/java/org/apache/solr/handler/component/ShardResponse.java b/solr/core/src/java/org/apache/solr/handler/component/ShardResponse.java
index dee5fcdb803..9cc33328134 100644
--- a/solr/core/src/java/org/apache/solr/handler/component/ShardResponse.java
+++ b/solr/core/src/java/org/apache/solr/handler/component/ShardResponse.java
@@ -81,6 +81,10 @@ void setResponseCode(int rspCode) {
     this.rspCode = rspCode;
   }
 
+  public int getRspCode() {
+    return rspCode;
+  }
+
   void setNodeName(String nodeName) {
     this.nodeName = nodeName;
   }
diff --git a/solr/core/src/test/org/apache/solr/handler/component/SearchHandlerTest.java b/solr/core/src/test/org/apache/solr/handler/component/SearchHandlerTest.java
index da861b79d99..c7bce23e0ac 100644
--- a/solr/core/src/test/org/apache/solr/handler/component/SearchHandlerTest.java
+++ b/solr/core/src/test/org/apache/solr/handler/component/SearchHandlerTest.java
@@ -360,6 +360,19 @@ public void testLuceneIOExceptionHandling() throws Exception {
       assertTrue(
           e.getMessage()
               .contains("Query contains too many nested clauses; maxClauseCount is set to 1"));
+
+      solrQuery = new SolrQuery("{!surround maxBasicQueries=1000 df=title}10W(tes*,test*)");
+      solrQuery.add(ShardParams.SHARDS_TOLERANT, "true");
+      final QueryRequest req4 = new QueryRequest(solrQuery);
+      req4.setMethod(SolrRequest.METHOD.POST);
+      e =
+          assertThrows(
+              BaseHttpSolrClient.RemoteSolrException.class,
+              () -> req4.process(cloudSolrClient, collectionName));
+      assertEquals(400, e.code());
+      assertTrue(
+          e.getMessage()
+              .contains("Query contains too many nested clauses; maxClauseCount is set to 1"));
     } finally {
       if (initialMaxBooleanClauses != null) {
         System.setProperty("solr.max.booleanClauses", initialMaxBooleanClauses);