SOLR-11551: Standardize CoreAdmin API status codes

solr/CHANGES.txt

@@ -74,6 +74,8 @@ Bug Fixes
 * SOLR-12087: Deleting replicas sometimes fails and causes the replicas to exist in the down 
   state (Cao Manh Dat)
 
+* SOLR-11551: Standardize CoreAdmin API success/failure status codes (Jason Gerlowski, Steve Rowe)
+
 Optimizations
 ----------------------
 

solr/core/src/java/org/apache/solr/handler/admin/BackupCoreOp.java

@@ -34,15 +34,9 @@ class BackupCoreOp implements CoreAdminHandler.CoreAdminOp {
   @Override
   public void execute(CoreAdminHandler.CallInfo it) throws Exception {
     final SolrParams params = it.req.getParams();
-    String cname = params.get(CoreAdminParams.CORE);
-    if (cname == null) {
-      throw new IllegalArgumentException(CoreAdminParams.CORE + " is required");
-    }
 
-    String name = params.get(NAME);
+    String cname = params.required().get(CoreAdminParams.CORE);
-    if (name == null) {
+    String name = params.required().get(NAME);
-      throw new IllegalArgumentException(CoreAdminParams.NAME + " is required");
-    }
 
     String repoName = params.get(CoreAdminParams.BACKUP_REPOSITORY);
     BackupRepository repository = it.handler.coreContainer.newBackupRepository(Optional.ofNullable(repoName));

solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java

@@ -406,7 +406,16 @@ public class CoreAdminHandler extends RequestHandlerBase implements PermissionNa
   public interface Invocable {
     Map<String, Object> invoke(SolrQueryRequest req);
   }
+  
   interface CoreAdminOp {
+   /**
+    * @param it request/response object
+    *
+    * If the request is invalid throw a SolrException with SolrException.ErrorCode.BAD_REQUEST ( 400 )
+    * If the execution of the command fails throw a SolrException with SolrException.ErrorCode.SERVER_ERROR ( 500 )
+    * 
+    * Any non-SolrException's are wrapped at a higher level as a SolrException with SolrException.ErrorCode.SERVER_ERROR.
+    */
     void execute(CallInfo it) throws Exception;
   }
 }

solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java

@@ -93,37 +93,33 @@ enum CoreAdminOperation implements CoreAdminOp {
   }),
   UNLOAD_OP(UNLOAD, it -> {
     SolrParams params = it.req.getParams();
-    String cname = params.get(CoreAdminParams.CORE);
+    String cname = params.required().get(CoreAdminParams.CORE);
+
     boolean deleteIndexDir = params.getBool(CoreAdminParams.DELETE_INDEX, false);
     boolean deleteDataDir = params.getBool(CoreAdminParams.DELETE_DATA_DIR, false);
     boolean deleteInstanceDir = params.getBool(CoreAdminParams.DELETE_INSTANCE_DIR, false);
     it.handler.coreContainer.unload(cname, deleteIndexDir, deleteDataDir, deleteInstanceDir);
 
     assert TestInjection.injectNonExistentCoreExceptionAfterUnload(cname);
-
   }),
   RELOAD_OP(RELOAD, it -> {
     SolrParams params = it.req.getParams();
     String cname = params.required().get(CoreAdminParams.CORE);
 
-    try {
     it.handler.coreContainer.reload(cname);
-    } catch (Exception ex) {
-      throw new SolrException(ErrorCode.SERVER_ERROR, "Error handling 'reload' action", ex);
-    }
   }),
   STATUS_OP(STATUS, new StatusOp()),
   SWAP_OP(SWAP, it -> {
     final SolrParams params = it.req.getParams();
-    final String cname = params.get(CoreAdminParams.CORE);
+    final String cname = params.required().get(CoreAdminParams.CORE);
     String other = params.required().get(CoreAdminParams.OTHER);
     it.handler.coreContainer.swap(cname, other);
   }),
 
   RENAME_OP(RENAME, it -> {
     SolrParams params = it.req.getParams();
-    String name = params.get(CoreAdminParams.OTHER);
+    String name = params.required().get(CoreAdminParams.OTHER);
-    String cname = params.get(CoreAdminParams.CORE);
+    String cname = params.required().get(CoreAdminParams.CORE);
 
     if (cname.equals(name)) return;
 
@@ -138,7 +134,7 @@ enum CoreAdminOperation implements CoreAdminOp {
 
   REQUESTRECOVERY_OP(REQUESTRECOVERY, it -> {
     final SolrParams params = it.req.getParams();
-    final String cname = params.get(CoreAdminParams.CORE, "");
+    final String cname = params.required().get(CoreAdminParams.CORE);
     log().info("It has been requested that we recover: core=" + cname);
     
     try (SolrCore core = it.handler.coreContainer.getCore(cname)) {
@@ -154,7 +150,7 @@ enum CoreAdminOperation implements CoreAdminOp {
 
   REQUESTBUFFERUPDATES_OP(REQUESTBUFFERUPDATES, it -> {
     SolrParams params = it.req.getParams();
-    String cname = params.get(CoreAdminParams.NAME, "");
+    String cname = params.required().get(CoreAdminParams.NAME);
     log().info("Starting to buffer updates on core:" + cname);
 
     try (SolrCore core = it.handler.coreContainer.getCore(cname)) {
@@ -180,7 +176,7 @@ enum CoreAdminOperation implements CoreAdminOp {
 
   REQUESTSTATUS_OP(REQUESTSTATUS, it -> {
     SolrParams params = it.req.getParams();
-    String requestId = params.get(CoreAdminParams.REQUESTID);
+    String requestId = params.required().get(CoreAdminParams.REQUESTID);
     log().info("Checking request status for : " + requestId);
 
     if (it.handler.getRequestStatusMap(RUNNING).containsKey(requestId)) {
@@ -225,10 +221,11 @@ enum CoreAdminOperation implements CoreAdminOp {
   CREATESNAPSHOT_OP(CREATESNAPSHOT, new CreateSnapshotOp()),
   DELETESNAPSHOT_OP(DELETESNAPSHOT, new DeleteSnapshotOp()),
   LISTSNAPSHOTS_OP(LISTSNAPSHOTS, it -> {
-    CoreContainer cc = it.handler.getCoreContainer();
     final SolrParams params = it.req.getParams();
-
     String cname = params.required().get(CoreAdminParams.CORE);
+
+    CoreContainer cc = it.handler.getCoreContainer();
+
     try ( SolrCore core = cc.getCore(cname) ) {
       if (core == null) {
         throw new SolrException(ErrorCode.BAD_REQUEST, "Unable to locate core " + cname);
@@ -355,7 +352,13 @@ enum CoreAdminOperation implements CoreAdminOp {
 
   @Override
   public void execute(CallInfo it) throws Exception {
+    try {
       fun.execute(it);
+    } catch (SolrException | InterruptedException e) {
+      // No need to re-wrap; throw as-is.
+      throw e;
+    } catch (Exception e) {
+      throw new SolrException(ErrorCode.SERVER_ERROR, "Error handling '" + action.name() + "' action", e);
+    }
   }
-
 }

solr/core/src/java/org/apache/solr/handler/admin/CreateSnapshotOp.java

@@ -31,11 +31,12 @@ import org.apache.solr.util.RefCounted;
 class CreateSnapshotOp implements CoreAdminHandler.CoreAdminOp {
   @Override
   public void execute(CoreAdminHandler.CallInfo it) throws Exception {
-    CoreContainer cc = it.handler.getCoreContainer();
     final SolrParams params = it.req.getParams();
-
     String commitName = params.required().get(CoreAdminParams.COMMIT_NAME);
     String cname = params.required().get(CoreAdminParams.CORE);
+
+    CoreContainer cc = it.handler.getCoreContainer();
+
     try (SolrCore core = cc.getCore(cname)) {
       if (core == null) {
         throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Unable to locate core " + cname);

solr/core/src/java/org/apache/solr/handler/admin/DeleteSnapshotOp.java

@@ -28,11 +28,11 @@ class DeleteSnapshotOp implements CoreAdminHandler.CoreAdminOp {
 
   @Override
   public void execute(CoreAdminHandler.CallInfo it) throws Exception {
-    CoreContainer cc = it.handler.getCoreContainer();
     final SolrParams params = it.req.getParams();
-
     String commitName = params.required().get(CoreAdminParams.COMMIT_NAME);
     String cname = params.required().get(CoreAdminParams.CORE);
+
+    CoreContainer cc = it.handler.getCoreContainer();
     SolrCore core = cc.getCore(cname);
     if (core == null) {
       throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Unable to locate core " + cname);

solr/core/src/java/org/apache/solr/handler/admin/MergeIndexesOp.java

@@ -54,13 +54,14 @@ class MergeIndexesOp implements CoreAdminHandler.CoreAdminOp {
     String cname = params.required().get(CoreAdminParams.CORE);
     SolrCore core = it.handler.coreContainer.getCore(cname);
     SolrQueryRequest wrappedReq = null;
+    if (core == null) return;
 
     List<SolrCore> sourceCores = Lists.newArrayList();
     List<RefCounted<SolrIndexSearcher>> searchers = Lists.newArrayList();
     // stores readers created from indexDir param values
     List<DirectoryReader> readersToBeClosed = Lists.newArrayList();
     Map<Directory, Boolean> dirsToBeReleased = new HashMap<>();
-    if (core != null) {
+
     try {
       String[] dirNames = params.getParams(CoreAdminParams.INDEX_DIR);
       if (dirNames == null || dirNames.length == 0) {
@@ -139,4 +140,3 @@ class MergeIndexesOp implements CoreAdminHandler.CoreAdminOp {
     }
   }
 }
-}

solr/core/src/java/org/apache/solr/handler/admin/RequestApplyUpdatesOp.java

@@ -31,7 +31,7 @@ class RequestApplyUpdatesOp implements CoreAdminHandler.CoreAdminOp {
   @Override
   public void execute(CoreAdminHandler.CallInfo it) throws Exception {
     SolrParams params = it.req.getParams();
-    String cname = params.get(CoreAdminParams.NAME, "");
+    String cname = params.required().get(CoreAdminParams.NAME);
     CoreAdminOperation.log().info("Applying buffered updates on core: " + cname);
     CoreContainer coreContainer = it.handler.coreContainer;
     try (SolrCore core = coreContainer.getCore(cname)) {

solr/core/src/java/org/apache/solr/handler/admin/RequestSyncShardOp.java

@@ -44,16 +44,14 @@ class RequestSyncShardOp implements CoreAdminHandler.CoreAdminOp {
     final SolrParams params = it.req.getParams();
 
     log.info("I have been requested to sync up my shard");
+
+    String cname = params.required().get(CoreAdminParams.CORE);
+
     ZkController zkController = it.handler.coreContainer.getZkController();
     if (zkController == null) {
       throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Only valid for SolrCloud");
     }
 
-    String cname = params.get(CoreAdminParams.CORE);
-    if (cname == null) {
-      throw new IllegalArgumentException(CoreAdminParams.CORE + " is required");
-    }
-
     SyncStrategy syncStrategy = null;
     try (SolrCore core = it.handler.coreContainer.getCore(cname)) {
 

solr/core/src/java/org/apache/solr/handler/admin/RestoreCoreOp.java

@@ -36,22 +36,15 @@ import static org.apache.solr.common.params.CommonParams.NAME;
 class RestoreCoreOp implements CoreAdminHandler.CoreAdminOp {
   @Override
   public void execute(CoreAdminHandler.CallInfo it) throws Exception {
+    final SolrParams params = it.req.getParams();
+    String cname = params.required().get(CoreAdminParams.CORE);
+    String name = params.required().get(NAME);
+
     ZkController zkController = it.handler.coreContainer.getZkController();
     if (zkController == null) {
       throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Only valid for SolrCloud");
     }
 
-    final SolrParams params = it.req.getParams();
-    String cname = params.get(CoreAdminParams.CORE);
-    if (cname == null) {
-      throw new IllegalArgumentException(CoreAdminParams.CORE + " is required");
-    }
-
-    String name = params.get(NAME);
-    if (name == null) {
-      throw new IllegalArgumentException(CoreAdminParams.NAME + " is required");
-    }
-
     String repoName = params.get(CoreAdminParams.BACKUP_REPOSITORY);
     BackupRepository repository = it.handler.coreContainer.newBackupRepository(Optional.ofNullable(repoName));
 

solr/core/src/java/org/apache/solr/handler/admin/SplitOp.java

@@ -145,11 +145,9 @@ class SplitOp implements CoreAdminHandler.CoreAdminOp {
       }
 
       // After the split has completed, someone (here?) should start the process of replaying the buffered updates.
-
     } catch (Exception e) {
       log.error("ERROR executing split:", e);
-      throw new RuntimeException(e);
+      throw e;
-
     } finally {
       if (req != null) req.close();
       if (core != null) core.close();

solr/core/src/java/org/apache/solr/handler/admin/StatusOp.java

@@ -21,7 +21,6 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 
-import org.apache.solr.common.SolrException;
 import org.apache.solr.common.params.CoreAdminParams;
 import org.apache.solr.common.params.SolrParams;
 import org.apache.solr.common.util.NamedList;
@@ -42,7 +41,6 @@ class StatusOp implements CoreAdminHandler.CoreAdminOp {
     for (Map.Entry<String, CoreContainer.CoreLoadFailure> failure : it.handler.coreContainer.getCoreInitFailures().entrySet()) {
       failures.put(failure.getKey(), failure.getValue().exception);
     }
-    try {
     if (cname == null) {
       for (String name : it.handler.coreContainer.getAllCoreNames()) {
         status.add(name, CoreAdminOperation.getCoreStatus(it.handler.coreContainer, name, isIndexInfoNeeded));
@@ -56,9 +54,5 @@ class StatusOp implements CoreAdminHandler.CoreAdminOp {
           status.add(cname, CoreAdminOperation.getCoreStatus(it.handler.coreContainer, cname, isIndexInfoNeeded));
     }
     it.rsp.add("status", status);
-    } catch (Exception ex) {
-      throw new SolrException(SolrException.ErrorCode.SERVER_ERROR,
-          "Error handling 'status' action ", ex);
-    }
   }
 }

solr/core/src/test/org/apache/solr/handler/admin/CoreAdminHandlerTest.java

@@ -422,8 +422,7 @@ public class CoreAdminHandlerTest extends SolrTestCaseJ4 {
           , resp);
       fail("Was able to successfully reload non-existent-core");
     } catch (Exception e) {
-      String e1 = e.getCause().getMessage();
+      assertEquals("Expected error message for non-existent core.", "No such core: non-existent-core", e.getMessage());
-      assertEquals("Expected error message for non-existent core.", "No such core: non-existent-core", e.getCause().getMessage());
     }
 
     // test null core

solr/core/src/test/org/apache/solr/handler/admin/CoreAdminOperationTest.java

@@ -0,0 +1,681 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.handler.admin;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.util.Map;
+
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SolrException.ErrorCode;
+import org.apache.solr.common.params.MapSolrParams;
+import org.apache.solr.request.SolrQueryRequest;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import com.google.common.collect.Maps;
+
+public class CoreAdminOperationTest extends SolrTestCaseJ4 {
+  
+  private CoreAdminHandler.CallInfo callInfo;
+  private SolrQueryRequest mockRequest;
+
+  @BeforeClass
+  public static void setUpClass() {
+    assumeWorkingMockito();
+  }
+
+  @Before
+  public void setUp() throws Exception{
+    super.setUp();
+    
+    mockRequest = mock(SolrQueryRequest.class);
+    callInfo = new CoreAdminHandler.CallInfo(null, mockRequest, null, null);
+  }
+  
+  @After
+  public void tearDown() throws Exception {
+    super.tearDown();
+  }
+  
+  @Test
+  public void testStatusUnexpectedFailuresResultIn500SolrException() throws Exception {
+    final Throwable cause = new NullPointerException();
+    whenUnexpectedErrorOccursDuringCoreAdminOp(cause);
+
+    try {
+      CoreAdminOperation.STATUS_OP.execute(callInfo);
+      fail("Expected core-status execution to fail with exception");
+    } catch (Exception thrown) {
+      assertSolrExceptionWithCodeAndCause(thrown, ErrorCode.SERVER_ERROR.code, cause);
+    }
+  }
+  
+  @Test
+  public void testUnloadUnexpectedFailuresResultIn500SolrException() throws Exception {
+    final Throwable cause = new NullPointerException();
+    whenUnexpectedErrorOccursDuringCoreAdminOp(cause);
+    
+    try {
+      CoreAdminOperation.UNLOAD_OP.execute(callInfo);
+      fail("Expected core-unload execution to fail with exception.");
+    } catch  (Exception thrown) {
+      assertSolrExceptionWithCodeAndCause(thrown, ErrorCode.SERVER_ERROR.code, cause);
+    }
+  }
+  
+  @Test
+  public void testUnloadMissingCoreNameResultsIn400SolrException() throws Exception {
+    whenCoreAdminOpHasParams(Maps.newHashMap());
+    
+    try {
+      CoreAdminOperation.UNLOAD_OP.execute(callInfo);
+      fail("Expected core-unload execution to fail when no 'core' param present");
+    } catch (Exception thrown) {
+      assertSolrExceptionWithCode(thrown, ErrorCode.BAD_REQUEST.code);
+    }
+  }
+  
+  @Test
+  public void testReloadUnexpectedFailuresResultIn500SolrException() {
+    final Throwable cause = new NullPointerException();
+    whenUnexpectedErrorOccursDuringCoreAdminOp(cause);
+    
+    try {
+      CoreAdminOperation.RELOAD_OP.execute(callInfo);
+      fail("Expected core-reload execution to fail with exception.");
+    } catch  (Exception thrown) {
+      assertSolrExceptionWithCodeAndCause(thrown, ErrorCode.SERVER_ERROR.code, cause);
+    }
+  }
+  
+  @Test
+  public void testReloadMissingCoreNameResultsIn400SolrException() {
+    whenCoreAdminOpHasParams(Maps.newHashMap());
+    
+    try {
+      CoreAdminOperation.RELOAD_OP.execute(callInfo);
+      fail("Expected core-reload execution to fail when no 'core' param present");
+    } catch (Exception thrown) {
+      assertSolrExceptionWithCode(thrown, ErrorCode.BAD_REQUEST.code);
+    }
+  }
+  
+  @Test
+  public void testCreateUnexpectedFailuresResultIn500SolrException() {
+    final Throwable cause = new NullPointerException();
+    whenUnexpectedErrorOccursDuringCoreAdminOp(cause);
+    
+    try {
+      CoreAdminOperation.CREATE_OP.execute(callInfo);
+      fail("Expected core-create execution to fail with exception.");
+    } catch  (Exception thrown) {
+      assertSolrExceptionWithCodeAndCause(thrown, ErrorCode.SERVER_ERROR.code, cause);
+    }
+  }
+  
+  @Test
+  public void testCreateMissingCoreNameResultsIn400SolrException() {
+    whenCoreAdminOpHasParams(Maps.newHashMap());
+    
+    try {
+      CoreAdminOperation.CREATE_OP.execute(callInfo);
+      fail("Expected core-create execution to fail when no 'core' param present");
+    } catch (Exception thrown) {
+      assertSolrExceptionWithCode(thrown, ErrorCode.BAD_REQUEST.code);
+    }
+  }
+  
+  @Test
+  public void testSwapUnexpectedFailuresResultIn500SolrException() {
+    final Throwable cause = new NullPointerException();
+    whenUnexpectedErrorOccursDuringCoreAdminOp(cause);
+    
+    try {
+      CoreAdminOperation.SWAP_OP.execute(callInfo);
+      fail("Expected core-swap execution to fail with exception.");
+    } catch  (Exception thrown) {
+      assertSolrExceptionWithCodeAndCause(thrown, ErrorCode.SERVER_ERROR.code, cause);
+    }
+  }
+  
+  @Test
+  public void testSwapMissingCoreParamResultsIn400SolrException() {
+    final Map<String, String> params = Maps.newHashMap();
+    params.put("other", "some-core-name");
+    whenCoreAdminOpHasParams(params);
+    
+    try {
+      CoreAdminOperation.SWAP_OP.execute(callInfo);
+      fail("Expected core-swap execution to fail when no 'core' param present");
+    } catch (Exception thrown) {
+      assertSolrExceptionWithCode(thrown, ErrorCode.BAD_REQUEST.code);
+    }
+  }
+  
+  @Test
+  public void testSwapMissingOtherParamResultsIn400SolrException() {
+    final Map<String, String> params = Maps.newHashMap();
+    params.put("core", "some-core-name");
+    whenCoreAdminOpHasParams(params);
+    
+    try {
+      CoreAdminOperation.SWAP_OP.execute(callInfo);
+      fail("Expected core-swap execution to fail when no 'other' param present");
+    } catch (Exception thrown) {
+      assertSolrExceptionWithCode(thrown, ErrorCode.BAD_REQUEST.code);
+    }
+  }
+  
+  @Test
+  public void testRenameUnexpectedFailuresResultIn500SolrException() {
+    final Throwable cause = new NullPointerException();
+    whenUnexpectedErrorOccursDuringCoreAdminOp(cause);
+    
+    try {
+      CoreAdminOperation.RENAME_OP.execute(callInfo);
+      fail("Expected core-rename execution to fail with exception.");
+    } catch  (Exception thrown) {
+      assertSolrExceptionWithCodeAndCause(thrown, ErrorCode.SERVER_ERROR.code, cause);
+    }
+  }
+  
+  @Test
+  public void testRenameMissingCoreParamResultsIn400SolrException() {
+    final Map<String, String> params = Maps.newHashMap();
+    params.put("other", "some-core-name");
+    whenCoreAdminOpHasParams(params);
+    
+    try {
+      CoreAdminOperation.RENAME_OP.execute(callInfo);
+      fail("Expected core-rename execution to fail when no 'core' param present");
+    } catch (Exception thrown) {
+      assertSolrExceptionWithCode(thrown, ErrorCode.BAD_REQUEST.code);
+    }
+  }
+  
+  @Test
+  public void testRenameMissingOtherParamResultsIn400SolrException() {
+    final Map<String, String> params = Maps.newHashMap();
+    params.put("core", "some-core-name");
+    whenCoreAdminOpHasParams(params);
+    
+    try {
+      CoreAdminOperation.RENAME_OP.execute(callInfo);
+      fail("Expected core-rename execution to fail when no 'other' param present");
+    } catch (Exception thrown) {
+      assertSolrExceptionWithCode(thrown, ErrorCode.BAD_REQUEST.code);
+    }
+  }
+  
+  @Test
+  public void testMergeUnexpectedFailuresResultIn500SolrException() {
+    final Throwable cause = new NullPointerException();
+    whenUnexpectedErrorOccursDuringCoreAdminOp(cause);
+    
+    try {
+      CoreAdminOperation.MERGEINDEXES_OP.execute(callInfo);
+      fail("Expected core-merge execution to fail with exception.");
+    } catch  (Exception thrown) {
+      assertSolrExceptionWithCodeAndCause(thrown, ErrorCode.SERVER_ERROR.code, cause);
+    }
+  }
+  
+  @Test
+  public void testMergeMissingCoreParamResultsIn400SolrException() {
+    final Map<String, String> params = Maps.newHashMap();
+    params.put("indexDir", "some/index/dir");
+    whenCoreAdminOpHasParams(params);
+    
+    try {
+      CoreAdminOperation.MERGEINDEXES_OP.execute(callInfo);
+      fail("Expected core-merge execution to fail when no 'core' param present");
+    } catch (Exception thrown) {
+      assertSolrExceptionWithCode(thrown, ErrorCode.BAD_REQUEST.code);
+    }
+  }
+  
+  @Test
+  public void testSplitUnexpectedFailuresResultIn500SolrException() {
+    final Throwable cause = new NullPointerException();
+    whenUnexpectedErrorOccursDuringCoreAdminOp(cause);
+    
+    try {
+      CoreAdminOperation.SPLIT_OP.execute(callInfo);
+      fail("Expected split execution to fail with exception.");
+    } catch  (Exception thrown) {
+      assertSolrExceptionWithCodeAndCause(thrown, ErrorCode.SERVER_ERROR.code, cause);
+    }
+  }
+  
+  @Test
+  public void testSplitMissingCoreParamResultsIn400SolrException() {
+    whenCoreAdminOpHasParams(Maps.newHashMap());
+    
+    try {
+      CoreAdminOperation.SPLIT_OP.execute(callInfo);
+      fail("Expected split execution to fail when no 'core' param present");
+    } catch (Exception thrown) {
+      assertSolrExceptionWithCode(thrown, ErrorCode.BAD_REQUEST.code);
+    }
+  }
+  
+  @Test
+  public void testPrepRecoveryUnexpectedFailuresResultIn500SolrException() {
+    final Throwable cause = new NullPointerException();
+    whenUnexpectedErrorOccursDuringCoreAdminOp(cause);
+    
+    try {
+      CoreAdminOperation.PREPRECOVERY_OP.execute(callInfo);
+      fail("Expected preprecovery execution to fail with exception.");
+    } catch  (Exception thrown) {
+      assertSolrExceptionWithCodeAndCause(thrown, ErrorCode.SERVER_ERROR.code, cause);
+    }
+  }
+  
+  @Test
+  public void testRequestRecoveryUnexpectedFailuresResultIn500Exception() {
+    final Throwable cause = new NullPointerException();
+    whenUnexpectedErrorOccursDuringCoreAdminOp(cause);
+    
+    try {
+      CoreAdminOperation.REQUESTRECOVERY_OP.execute(callInfo);
+      fail("Expected request-recovery execution to fail with exception.");
+    } catch  (Exception thrown) {
+      assertSolrExceptionWithCodeAndCause(thrown, ErrorCode.SERVER_ERROR.code, cause);
+    }    
+  }
+  
+  @Test
+  public void testRequestRecoveryMissingCoreParamResultsIn400SolrException() {
+    whenCoreAdminOpHasParams(Maps.newHashMap());
+    
+    try {
+      CoreAdminOperation.REQUESTRECOVERY_OP.execute(callInfo);
+      fail("Expected request-recovery execution to fail when no 'core' param present");
+    } catch (Exception thrown) {
+      thrown.printStackTrace();
+      assertSolrExceptionWithCode(thrown, ErrorCode.BAD_REQUEST.code);
+    }  
+  }
+  
+  @Test
+  public void testRequestSyncUnexpectedFailuresResultIn500Exception() {
+    final Throwable cause = new NullPointerException();
+    whenUnexpectedErrorOccursDuringCoreAdminOp(cause);
+    
+    try {
+      CoreAdminOperation.REQUESTSYNCSHARD_OP.execute(callInfo);
+      fail("Expected request-sync execution to fail with exception.");
+    } catch  (Exception thrown) {
+      assertSolrExceptionWithCodeAndCause(thrown, ErrorCode.SERVER_ERROR.code, cause);
+    }
+  }
+  
+  @Test
+  public void testRequestSyncMissingCoreParamResultsIn400SolrException() {
+    whenCoreAdminOpHasParams(Maps.newHashMap());
+    
+    try {
+      CoreAdminOperation.REQUESTSYNCSHARD_OP.execute(callInfo);
+      fail("Expected request-sync execution to fail when no 'core' param present");
+    } catch (Exception thrown) {
+      assertSolrExceptionWithCode(thrown, ErrorCode.BAD_REQUEST.code);
+    }      
+  }
+  
+  @Test
+  public void testRequestBufferUpdatesUnexpectedFailuresResultIn500Exception() {
+    final Throwable cause = new NullPointerException();
+    whenUnexpectedErrorOccursDuringCoreAdminOp(cause);
+    
+    try {
+      CoreAdminOperation.REQUESTBUFFERUPDATES_OP.execute(callInfo);
+      fail("Expected request-buffer-updates execution to fail with exception.");
+    } catch  (Exception thrown) {
+      assertSolrExceptionWithCodeAndCause(thrown, ErrorCode.SERVER_ERROR.code, cause);
+    }
+  }
+  
+  @Test
+  public void testRequestBufferUpdatesMissingCoreParamResultsIn400SolrException() {
+    whenCoreAdminOpHasParams(Maps.newHashMap());
+
+    try {
+      CoreAdminOperation.REQUESTBUFFERUPDATES_OP.execute(callInfo);
+      fail("Expected request-buffer-updates execution to fail when no 'core' param present");
+    } catch (Exception thrown) {
+      assertSolrExceptionWithCode(thrown, ErrorCode.BAD_REQUEST.code);
+    }
+  }
+  
+  @Test
+  public void testRequestApplyUpdatesUnexpectedFailuresResultIn500Exception() {
+    final Throwable cause = new NullPointerException();
+    whenUnexpectedErrorOccursDuringCoreAdminOp(cause);
+    
+    try {
+      CoreAdminOperation.REQUESTAPPLYUPDATES_OP.execute(callInfo);
+      fail("Expected request-apply-updates execution to fail with exception.");
+    } catch  (Exception thrown) {
+      assertSolrExceptionWithCodeAndCause(thrown, ErrorCode.SERVER_ERROR.code, cause);
+    }    
+  }
+  
+  @Test
+  public void testRequestApplyUpdatesMissingCoreParamResultsIn400SolrException() {
+    whenCoreAdminOpHasParams(Maps.newHashMap());
+
+    try {
+      CoreAdminOperation.REQUESTAPPLYUPDATES_OP.execute(callInfo);
+      fail("Expected request-apply-updates execution to fail when no 'core' param present");
+    } catch (Exception thrown) {
+      assertSolrExceptionWithCode(thrown, ErrorCode.BAD_REQUEST.code);
+    }    
+  }
+
+  @Test
+  public void testOverseerOpUnexpectedFailuresResultIn500Exception() {
+    final Throwable cause = new NullPointerException();
+    whenUnexpectedErrorOccursDuringCoreAdminOp(cause);
+    
+    try {
+      CoreAdminOperation.OVERSEEROP_OP.execute(callInfo);
+      fail("Expected overseerop execution to fail with exception.");
+    } catch  (Exception thrown) {
+      assertSolrExceptionWithCode(thrown, ErrorCode.SERVER_ERROR.code);
+    }
+  }
+  
+  @Test
+  public void testRequestStatusUnexpectedFailuresResultIn500Exception() {
+    final Throwable cause = new NullPointerException();
+    whenUnexpectedErrorOccursDuringCoreAdminOp(cause);
+    
+    try {
+      CoreAdminOperation.REQUESTSTATUS_OP.execute(callInfo);
+      fail("Expected request-status execution to fail with exception.");
+    } catch  (Exception thrown) {
+      assertSolrExceptionWithCodeAndCause(thrown, ErrorCode.SERVER_ERROR.code, cause);
+    }
+  }
+  
+  @Test
+  public void testRequestStatusMissingRequestIdParamResultsIn400SolrException() {
+    whenCoreAdminOpHasParams(Maps.newHashMap());
+
+    try {
+      CoreAdminOperation.REQUESTSTATUS_OP.execute(callInfo);
+      fail("Expected request-status execution to fail when no 'requestid' param present");
+    } catch (Exception thrown) {
+      assertSolrExceptionWithCode(thrown, ErrorCode.BAD_REQUEST.code);
+    }    
+  }
+
+  @Test
+  public void testRejoinLeaderElectionUnexpectedFailuresResultIn500Exception() {
+    final Throwable cause = new NullPointerException();
+    whenUnexpectedErrorOccursDuringCoreAdminOp(cause);
+    
+    try {
+      CoreAdminOperation.REJOINLEADERELECTION_OP.execute(callInfo);
+      fail("Expected rejoin-leader-election execution to fail with exception.");
+    } catch  (Exception thrown) {
+      assertSolrExceptionWithCode(thrown, ErrorCode.SERVER_ERROR.code);
+    }    
+  }
+ 
+
+  @Test
+  public void testInvokeUnexpectedFailuresResultIn500Exception() {
+    final Throwable cause = new NullPointerException();
+    whenUnexpectedErrorOccursDuringCoreAdminOp(cause);
+    
+    try {
+      CoreAdminOperation.INVOKE_OP.execute(callInfo);
+      fail("Expected invoke execution to fail with exception.");
+    } catch  (Exception thrown) {
+      assertSolrExceptionWithCodeAndCause(thrown, ErrorCode.SERVER_ERROR.code, cause);
+    }    
+  }
+  
+  @Test
+  public void testInvokeMissingClassParamResultsIn400SolrException() {
+    whenCoreAdminOpHasParams(Maps.newHashMap());
+
+    try {
+      CoreAdminOperation.INVOKE_OP.execute(callInfo);
+      fail("Expected invoke to fail when no 'class' param present");
+    } catch (Exception thrown) {
+      assertSolrExceptionWithCode(thrown, ErrorCode.BAD_REQUEST.code);
+    }    
+  }
+  
+  @Test
+  public void testBackupUnexpectedFailuresResultIn500Exception() {
+    final Throwable cause = new NullPointerException();
+    whenUnexpectedErrorOccursDuringCoreAdminOp(cause);
+    
+    try {
+      CoreAdminOperation.BACKUPCORE_OP.execute(callInfo);
+      fail("Expected backup-core execution to fail with exception.");
+    } catch  (Exception thrown) {
+      assertSolrExceptionWithCodeAndCause(thrown, ErrorCode.SERVER_ERROR.code, cause);
+    }  
+  }
+  
+  @Test
+  public void testBackupMissingCoreParamResultsIn400SolrException() {
+    final Map<String, String> params = Maps.newHashMap();
+    params.put("name", "any-name-param");
+    whenCoreAdminOpHasParams(params);
+
+    try {
+      CoreAdminOperation.BACKUPCORE_OP.execute(callInfo);
+      fail("Expected backup-core to fail when no 'core' param present");
+    } catch (Exception thrown) {
+      assertSolrExceptionWithCode(thrown, ErrorCode.BAD_REQUEST.code);
+    }   
+  }
+  
+  @Test
+  public void testBackupMissingNameParamResultsIn400SolrException() {
+    final Map<String, String> params = Maps.newHashMap();
+    params.put("core", "any-core-param");
+    whenCoreAdminOpHasParams(params);
+
+    try {
+      CoreAdminOperation.BACKUPCORE_OP.execute(callInfo);
+      fail("Expected backup-core to fail when no 'name' param present");
+    } catch (Exception thrown) {
+      assertSolrExceptionWithCode(thrown, ErrorCode.BAD_REQUEST.code);
+    }   
+  }
+  
+  @Test
+  public void testRestoreUnexpectedFailuresResultIn500Exception() {
+    final Throwable cause = new NullPointerException();
+    whenUnexpectedErrorOccursDuringCoreAdminOp(cause);
+    
+    try {
+      CoreAdminOperation.RESTORECORE_OP.execute(callInfo);
+      fail("Expected restore-core execution to fail with exception.");
+    } catch  (Exception thrown) {
+      assertSolrExceptionWithCodeAndCause(thrown, ErrorCode.SERVER_ERROR.code, cause);
+    }  
+  }
+  
+  @Test
+  public void testRestoreMissingCoreParamResultsIn400SolrException() {
+    final Map<String, String> params = Maps.newHashMap();
+    params.put("name", "any-name-param");
+    whenCoreAdminOpHasParams(params);
+
+    try {
+      CoreAdminOperation.RESTORECORE_OP.execute(callInfo);
+      fail("Expected restore-core to fail when no 'core' param present");
+    } catch (Exception thrown) {
+      assertSolrExceptionWithCode(thrown, ErrorCode.BAD_REQUEST.code);
+    }   
+  }
+  
+  @Test
+  public void testRestoreMissingNameParamResultsIn400SolrException() {
+    final Map<String, String> params = Maps.newHashMap();
+    params.put("core", "any-core-param");
+    whenCoreAdminOpHasParams(params);
+
+    try {
+      CoreAdminOperation.RESTORECORE_OP.execute(callInfo);
+      fail("Expected restore-core to fail when no 'name' param present");
+    } catch (Exception thrown) {
+      assertSolrExceptionWithCode(thrown, ErrorCode.BAD_REQUEST.code);
+    }   
+  }
+  
+  @Test
+  public void testCreateSnapshotUnexpectedFailuresResultIn500Exception() {
+    final Throwable cause = new NullPointerException();
+    whenUnexpectedErrorOccursDuringCoreAdminOp(cause);
+    
+    try {
+      CoreAdminOperation.CREATESNAPSHOT_OP.execute(callInfo);
+      fail("Expected create-snapshot execution to fail with exception.");
+    } catch  (Exception thrown) {
+      assertSolrExceptionWithCodeAndCause(thrown, ErrorCode.SERVER_ERROR.code, cause);
+    }  
+  }
+  
+  @Test
+  public void testCreateSnapshotMissingCoreParamResultsIn400SolrException() {
+    final Map<String, String> params = Maps.newHashMap();
+    params.put("commitName", "anyCommitName");
+    whenCoreAdminOpHasParams(params);
+
+    try {
+      CoreAdminOperation.CREATESNAPSHOT_OP.execute(callInfo);
+      fail("Expected create-snapshot to fail when no 'core' param present");
+    } catch (Exception thrown) {
+      assertSolrExceptionWithCode(thrown, ErrorCode.BAD_REQUEST.code);
+    }   
+  }
+  
+  @Test
+  public void testCreateSnapshotMissingCommitNameParamResultsIn400SolrException() {
+    final Map<String, String> params = Maps.newHashMap();
+    params.put("core", "any-core-param");
+    whenCoreAdminOpHasParams(params);
+
+    try {
+      CoreAdminOperation.CREATESNAPSHOT_OP.execute(callInfo);
+      fail("Expected create-snapshot to fail when no 'commitName' param present");
+    } catch (Exception thrown) {
+      assertSolrExceptionWithCode(thrown, ErrorCode.BAD_REQUEST.code);
+    }   
+  }
+  
+  @Test
+  public void testDeleteSnapshotUnexpectedFailuresResultIn500Exception() {
+    final Throwable cause = new NullPointerException();
+    whenUnexpectedErrorOccursDuringCoreAdminOp(cause);
+    
+    try {
+      CoreAdminOperation.DELETESNAPSHOT_OP.execute(callInfo);
+      fail("Expected delete-snapshot execution to fail with exception.");
+    } catch  (Exception thrown) {
+      assertSolrExceptionWithCodeAndCause(thrown, ErrorCode.SERVER_ERROR.code, cause);
+    }  
+  }
+  
+  @Test
+  public void testDeleteSnapshotMissingCoreParamResultsIn400SolrException() {
+    final Map<String, String> params = Maps.newHashMap();
+    params.put("commitName", "anyCommitName");
+    whenCoreAdminOpHasParams(params);
+
+    try {
+      CoreAdminOperation.DELETESNAPSHOT_OP.execute(callInfo);
+      fail("Expected delete-snapshot to fail when no 'core' param present");
+    } catch (Exception thrown) {
+      assertSolrExceptionWithCode(thrown, ErrorCode.BAD_REQUEST.code);
+    }   
+  }
+  
+  @Test
+  public void testDeleteSnapshotMissingCommitNameParamResultsIn400SolrException() {
+    final Map<String, String> params = Maps.newHashMap();
+    params.put("core", "any-core-param");
+    whenCoreAdminOpHasParams(params);
+
+    try {
+      CoreAdminOperation.DELETESNAPSHOT_OP.execute(callInfo);
+      fail("Expected delete-snapshot to fail when no 'commitName' param present");
+    } catch (Exception thrown) {
+      assertSolrExceptionWithCode(thrown, ErrorCode.BAD_REQUEST.code);
+    }   
+  }
+  
+  @Test
+  public void testListSnapshotUnexpectedFailuresResultIn500Exception() {
+    final Throwable cause = new NullPointerException();
+    whenUnexpectedErrorOccursDuringCoreAdminOp(cause);
+    
+    try {
+      CoreAdminOperation.LISTSNAPSHOTS_OP.execute(callInfo);
+      fail("Expected list-snapshot execution to fail with exception.");
+    } catch  (Exception thrown) {
+      assertSolrExceptionWithCodeAndCause(thrown, ErrorCode.SERVER_ERROR.code, cause);
+    }  
+  }
+  
+  @Test
+  public void testListSnapshotMissingCoreParamResultsIn400SolrException() {
+    whenCoreAdminOpHasParams(Maps.newHashMap());
+
+    try {
+      CoreAdminOperation.LISTSNAPSHOTS_OP.execute(callInfo);
+      fail("Expected list-snapshot to fail when no 'commitName' param present");
+    } catch (Exception thrown) {
+      assertSolrExceptionWithCode(thrown, ErrorCode.BAD_REQUEST.code);
+    }  
+  }
+  
+  private void whenUnexpectedErrorOccursDuringCoreAdminOp(Throwable cause) {
+    when(mockRequest.getParams()).thenThrow(cause);
+  }
+  
+  private void whenCoreAdminOpHasParams(Map<String, String> solrParams) {
+    when(mockRequest.getParams()).thenReturn(new MapSolrParams(solrParams));
+  }
+  
+  private void assertSolrExceptionWithCodeAndCause(Throwable thrownException, int expectedStatus, Throwable expectedCause) {
+    assertEquals(SolrException.class, thrownException.getClass());
+    
+    final SolrException solrException = (SolrException) thrownException;
+    assertEquals(expectedStatus, solrException.code());
+    
+    if (expectedCause != null) assertEquals(expectedCause, solrException.getCause());
+  }
+  
+  private void assertSolrExceptionWithCode(Throwable thrownException, int expectedStatus) {
+    assertSolrExceptionWithCodeAndCause(thrownException, expectedStatus, null);
+  }
+}

solr/core/src/test/org/apache/solr/handler/admin/CoreMergeIndexesAdminHandlerTest.java

@@ -22,6 +22,7 @@ import java.io.IOException;
 import org.apache.lucene.store.Directory;
 import org.apache.lucene.store.LockFactory;
 import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.SolrException;
 import org.apache.solr.common.params.CoreAdminParams;
 import org.apache.solr.core.CoreContainer;
 import org.apache.solr.core.DirectoryFactory;
@@ -48,11 +49,12 @@ public class CoreMergeIndexesAdminHandlerTest extends SolrTestCaseJ4 {
   public TestRule solrTestRules = RuleChain.outerRule(new SystemPropertiesRestoreRule());
 
 
-  private static String FAILING_MSG = "Creating a directory using FailingDirectoryFactoryException always fails";
+  private static String WRAPPED_FAILING_MSG = "Error handling 'mergeindexes' action";
+  private static String FAILING_CAUSE_MSG = "Creating a directory using FailingDirectoryFactoryException always fails";
   public static class FailingDirectoryFactory extends MockFSDirectoryFactory {
     public static class FailingDirectoryFactoryException extends RuntimeException {
       public FailingDirectoryFactoryException() {
-        super(FAILING_MSG);
+        super(FAILING_CAUSE_MSG);
       }
     }
 
@@ -81,7 +83,7 @@ public class CoreMergeIndexesAdminHandlerTest extends SolrTestCaseJ4 {
 
       try {
         dirFactory.fail = true;
-        ignoreException(FAILING_MSG);
+        ignoreException(WRAPPED_FAILING_MSG);
 
         SolrQueryResponse resp = new SolrQueryResponse();
         admin.handleRequestBody
@@ -91,10 +93,11 @@ public class CoreMergeIndexesAdminHandlerTest extends SolrTestCaseJ4 {
                 CoreAdminParams.INDEX_DIR, workDir.getAbsolutePath()),
                 resp);
         fail("exception expected");
-      } catch (FailingDirectoryFactory.FailingDirectoryFactoryException e) {
+      } catch (SolrException e) {
         // expected if error handling properly
+        assertEquals(FailingDirectoryFactory.FailingDirectoryFactoryException.class, e.getCause().getClass());
       } finally {
-        unIgnoreException(FAILING_MSG);
+        unIgnoreException(WRAPPED_FAILING_MSG);
       }
       dirFactory.fail = false;
     }