Do not fail over to NoILS for configuration errors. (#1467)

- This potentially masks problems that the administrator should know about immediately.

module/VuFind/src/VuFind/Exception/BadConfig.php

+<?php
+/**
+ * Bad Configuration Exception
+ *
+ * PHP version 7
+ *
+ * Copyright (C) Villanova University 2019.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ * @category VuFind
+ * @package  Exceptions
+ * @author   Demian Katz <demian.katz@villanova.edu>
+ * @license  http://opensource.org/licenses/gpl-2.0.php GNU General Public License
+ * @link     https://vufind.org/wiki/development Wiki
+ */
+namespace VuFind\Exception;
+
+/**
+ * Bad Configuration Exception
+ *
+ * @category VuFind
+ * @package  Exceptions
+ * @author   Demian Katz <demian.katz@villanova.edu>
+ * @license  http://opensource.org/licenses/gpl-2.0.php GNU General Public License
+ * @link     https://vufind.org/wiki/development Wiki
+ */
+class BadConfig extends \Exception
+{
+}

module/VuFind/src/VuFind/ILS/Connection.php

@@ -31,6 +31,7 @@
  */
 namespace VuFind\ILS;
 
+use VuFind\Exception\BadConfig;
 use VuFind\Exception\ILS as ILSException;
 use VuFind\I18n\Translator\TranslatorAwareInterface;
 use VuFind\ILS\Driver\DriverInterface;
@@ -174,7 +175,15 @@ class Connection implements TranslatorAwareInterface, LoggerAwareInterface
      */
     protected function initializeDriver()
     {
-        $this->driver->setConfig($this->getDriverConfig());
+        try {
+            $this->driver->setConfig($this->getDriverConfig());
+        } catch (\Exception $e) {
+            // Any errors thrown during configuration should be cast to BadConfig
+            // so we can handle them differently from other runtime problems.
+            throw $e instanceof BadConfig
+                ? $e
+                : new BadConfig('Failure during configuration.', 0, $e);
+        }
         $this->driver->init();
         $this->driverInitialized = true;
     }
@@ -195,10 +204,20 @@ class Connection implements TranslatorAwareInterface, LoggerAwareInterface
      * If configured, fail over to the NoILS driver and return true; otherwise,
      * return false.
      *
+     * @param \Exception $e The exception that triggered the failover.
+     *
      * @return bool
      */
-    protected function failOverToNoILS()
+    protected function failOverToNoILS(\Exception $e = null)
     {
+        // If the exception is caused by a configuration error, the administrator
+        // needs to fix it, but failing over to NoILS will mask the error and cause
+        // confusion. We shouldn't do that!
+        if ($e instanceof BadConfig) {
+            return false;
+        }
+
+        // If we got this far, we want to proceed with failover...
         $this->failing = true;
 
         // Only fail over if we're configured to allow it and we haven't already
@@ -232,7 +251,7 @@ class Connection implements TranslatorAwareInterface, LoggerAwareInterface
             try {
                 $this->initializeDriver();
             } catch (\Exception $e) {
-                if (!$this->failOverToNoILS()) {
+                if (!$this->failOverToNoILS($e)) {
                     throw $e;
                 }
             }
@@ -722,7 +741,7 @@ class Connection implements TranslatorAwareInterface, LoggerAwareInterface
                 return $this->getDriver()->checkRequestIsValid($id, $data, $patron);
             }
         } catch (\Exception $e) {
-            if ($this->failOverToNoILS()) {
+            if ($this->failOverToNoILS($e)) {
                 return call_user_func_array([$this, __METHOD__], func_get_args());
             }
             throw $e;
@@ -757,7 +776,7 @@ class Connection implements TranslatorAwareInterface, LoggerAwareInterface
                 );
             }
         } catch (\Exception $e) {
-            if ($this->failOverToNoILS()) {
+            if ($this->failOverToNoILS($e)) {
                 return call_user_func_array([$this, __METHOD__], func_get_args());
             }
             throw $e;
@@ -789,7 +808,7 @@ class Connection implements TranslatorAwareInterface, LoggerAwareInterface
                 );
             }
         } catch (\Exception $e) {
-            if ($this->failOverToNoILS()) {
+            if ($this->failOverToNoILS($e)) {
                 return call_user_func_array([$this, __METHOD__], func_get_args());
             }
             throw $e;
@@ -873,7 +892,7 @@ class Connection implements TranslatorAwareInterface, LoggerAwareInterface
             return $this->checkCapability('hasHoldings', [$id])
                 ? $this->getDriver()->hasHoldings($id) : true;
         } catch (\Exception $e) {
-            if ($this->failOverToNoILS()) {
+            if ($this->failOverToNoILS($e)) {
                 return call_user_func_array([$this, __METHOD__], func_get_args());
             }
             throw $e;
@@ -894,7 +913,7 @@ class Connection implements TranslatorAwareInterface, LoggerAwareInterface
             return $this->checkCapability('loginIsHidden')
                 ? $this->getDriver()->loginIsHidden() : false;
         } catch (\Exception $e) {
-            if ($this->failOverToNoILS()) {
+            if ($this->failOverToNoILS($e)) {
                 return call_user_func_array([$this, __METHOD__], func_get_args());
             }
             throw $e;
@@ -1066,7 +1085,7 @@ class Connection implements TranslatorAwareInterface, LoggerAwareInterface
                 );
             }
         } catch (\Exception $e) {
-            if ($this->failOverToNoILS()) {
+            if ($this->failOverToNoILS($e)) {
                 return call_user_func_array([$this, __METHOD__], func_get_args());
             }
             throw $e;

module/VuFind/src/VuFind/ILS/Driver/AbstractAPI.php

@@ -27,10 +27,11 @@
  */
 namespace VuFind\ILS\Driver;
 
-use VuFind\Exception\BadRequest as BadRequest;
-use VuFind\Exception\Forbidden as Forbidden;
+use VuFind\Exception\BadConfig;
+use VuFind\Exception\BadRequest;
+use VuFind\Exception\Forbidden;
 use VuFind\Exception\ILS as ILSException;
-use VuFind\Exception\RecordMissing as RecordMissing;
+use VuFind\Exception\RecordMissing;
 use VuFindHttp\HttpServiceAwareInterface;
 use Zend\Log\LoggerAwareInterface;
 
@@ -149,7 +150,7 @@ abstract class AbstractAPI extends AbstractBase implements HttpServiceAwareInter
      * @param array $config Configuration array (usually loaded from a VuFind .ini
      * file whose name corresponds with the driver class name).
      *
-     * @throws ILSException if base url excluded
+     * @throws BadConfig if base url excluded
      * @return void
      */
     public function setConfig($config)
@@ -157,7 +158,7 @@ abstract class AbstractAPI extends AbstractBase implements HttpServiceAwareInter
         parent::setConfig($config);
         // Base URL required for API drivers
         if (!isset($config['API']['base_url'])) {
-            throw new ILSException('API Driver configured without base url.');
+            throw new BadConfig('API Driver configured without base url.');
         }
     }
 }