From f7bcc09dccfd071b7eeb3b4ed7b307868fb44c74 Mon Sep 17 00:00:00 2001
From: Demian Katz <demian.katz@villanova.edu>
Date: Wed, 16 Oct 2019 09:11:54 -0400
Subject: [PATCH] Do not fail over to NoILS for configuration errors. (#1467)
- This potentially masks problems that the administrator should know about immediately.
---
.../VuFind/src/VuFind/Exception/BadConfig.php | 41 +++++++++++++++++++
module/VuFind/src/VuFind/ILS/Connection.php | 37 +++++++++++++----
.../src/VuFind/ILS/Driver/AbstractAPI.php | 11 ++---
3 files changed, 75 insertions(+), 14 deletions(-)
create mode 100644 module/VuFind/src/VuFind/Exception/BadConfig.php
diff --git a/module/VuFind/src/VuFind/Exception/BadConfig.php b/module/VuFind/src/VuFind/Exception/BadConfig.php
new file mode 100644
index 00000000000..06b4020fe17
--- /dev/null
+++ b/module/VuFind/src/VuFind/Exception/BadConfig.php
@@ -0,0 +1,41 @@
+<?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
+{
+}
diff --git a/module/VuFind/src/VuFind/ILS/Connection.php b/module/VuFind/src/VuFind/ILS/Connection.php
index d2568b618db..cc57f909bad 100644
--- a/module/VuFind/src/VuFind/ILS/Connection.php
+++ b/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;
diff --git a/module/VuFind/src/VuFind/ILS/Driver/AbstractAPI.php b/module/VuFind/src/VuFind/ILS/Driver/AbstractAPI.php
index 0c69752e84a..572f9a5d31b 100644
--- a/module/VuFind/src/VuFind/ILS/Driver/AbstractAPI.php
+++ b/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.');
}
}
}
--
GitLab