From fad0dbb6efcd772670a4cfb27bac57473deec37b Mon Sep 17 00:00:00 2001
From: Dag <me@dvikan.no>
Date: Wed, 22 Jun 2022 18:30:06 +0200
Subject: [PATCH] refactor: fix exception handling (#2835)

* refactor: fix exception handling

The removed catch is never uses in php versions above 7.
The need for multiple catch statements like this is to support both php 5 and 7.

* remove traces of old exception handling

* add typehints

* dont treat exception code 0 specially
---
 actions/DisplayAction.php | 62 +++++----------------------------------
 lib/Exceptions.php        | 32 +++-----------------
 2 files changed, 12 insertions(+), 82 deletions(-)

diff --git a/actions/DisplayAction.php b/actions/DisplayAction.php
index 44516b85..df850b9d 100644
--- a/actions/DisplayAction.php
+++ b/actions/DisplayAction.php
@@ -153,7 +153,7 @@ class DisplayAction extends ActionAbstract {
 					'donationUri'  => $bridge->getDonationURI(),
 					'icon' => $bridge->getIcon()
 				);
-			} catch(Error $e) {
+			} catch(\Throwable $e) {
 				error_log($e);
 
 				if(logBridgeError($bridge::NAME, $e->getCode()) >= Configuration::getConfig('error', 'report_limit')) {
@@ -163,22 +163,12 @@ class DisplayAction extends ActionAbstract {
 						// Create "new" error message every 24 hours
 						$this->userData['_error_time'] = urlencode((int)(time() / 86400));
 
-						// Error 0 is a special case (i.e. "trying to get property of non-object")
-						if($e->getCode() === 0) {
-							$item->setTitle(
-								'Bridge encountered an unexpected situation! ('
-								. $this->userData['_error_time']
-								. ')'
-							);
-						} else {
-							$item->setTitle(
-								'Bridge returned error '
-								. $e->getCode()
-								. '! ('
-								. $this->userData['_error_time']
-								. ')'
-							);
-						}
+						$message = sprintf(
+							'Bridge returned error %s! (%s)',
+							$e->getCode(),
+							$this->userData['_error_time']
+						);
+						$item->setTitle($message);
 
 						$item->setURI(
 							(isset($_SERVER['REQUEST_URI']) ? parse_url($_SERVER['REQUEST_URI'], PHP_URL_PATH) : '')
@@ -189,38 +179,6 @@ class DisplayAction extends ActionAbstract {
 						$item->setTimestamp(time());
 						$item->setContent(buildBridgeException($e, $bridge));
 
-						$items[] = $item;
-					} elseif(Configuration::getConfig('error', 'output') === 'http') {
-						header('Content-Type: text/html', true, $this->get_return_code($e));
-						die(buildTransformException($e, $bridge));
-					}
-				}
-			} catch(Exception $e) {
-				error_log($e);
-
-				if(logBridgeError($bridge::NAME, $e->getCode()) >= Configuration::getConfig('error', 'report_limit')) {
-					if(Configuration::getConfig('error', 'output') === 'feed') {
-						$item = new \FeedItem();
-
-						// Create "new" error message every 24 hours
-						$this->userData['_error_time'] = urlencode((int)(time() / 86400));
-
-						$item->setURI(
-							(isset($_SERVER['REQUEST_URI']) ? parse_url($_SERVER['REQUEST_URI'], PHP_URL_PATH) : '')
-							. '?'
-							. http_build_query($this->userData)
-						);
-
-						$item->setTitle(
-							'Bridge returned error '
-							. $e->getCode()
-							. '! ('
-							. $this->userData['_error_time']
-							. ')'
-						);
-						$item->setTimestamp(time());
-						$item->setContent(buildBridgeException($e, $bridge));
-
 						$items[] = $item;
 					} elseif(Configuration::getConfig('error', 'output') === 'http') {
 						header('Content-Type: text/html', true, $this->get_return_code($e));
@@ -251,11 +209,7 @@ class DisplayAction extends ActionAbstract {
 			header('Content-Type: ' . $format->getMimeType() . '; charset=' . $format->getCharset());
 
 			echo $format->stringify();
-		} catch(Error $e) {
-			error_log($e);
-			header('Content-Type: text/html', true, $e->getCode());
-			die(buildTransformException($e, $bridge));
-		} catch(Exception $e) {
+		} catch(\Throwable $e) {
 			error_log($e);
 			header('Content-Type: text/html', true, $e->getCode());
 			die(buildTransformException($e, $bridge));
diff --git a/lib/Exceptions.php b/lib/Exceptions.php
index c749780c..a9d2365b 100644
--- a/lib/Exceptions.php
+++ b/lib/Exceptions.php
@@ -66,20 +66,8 @@ function buildGitHubIssueQuery($title, $body, $labels = null, $maintainer = null
 	return $uri;
 }
 
-/**
- * Returns the exception message as HTML string
- *
- * @param object $e Exception The exception to show
- * @param object $bridge object The bridge object
- * @return string|null Returns the exception as HTML string or null.
- *
- * @todo This function belongs inside a class
- */
-function buildBridgeException($e, $bridge){
-	if(( !($e instanceof \Exception) && !($e instanceof \Error)) || !($bridge instanceof \BridgeInterface)) {
-		return null;
-	}
-
+function buildBridgeException(\Throwable $e, BridgeInterface $bridge): string
+{
 	$title = $bridge->getName() . ' failed with error ' . $e->getCode();
 
 	// Build a GitHub compatible message
@@ -106,20 +94,8 @@ EOD;
 	return $section;
 }
 
-/**
- * Returns the exception message as HTML string
- *
- * @param object $e Exception The exception to show
- * @param object $bridge object The bridge object
- * @return string|null Returns the exception as HTML string or null.
- *
- * @todo This function belongs inside a class
- */
-function buildTransformException($e, $bridge){
-	if(( !($e instanceof \Exception) && !($e instanceof \Error)) || !($bridge instanceof \BridgeInterface)) {
-		return null;
-	}
-
+function buildTransformException(\Throwable $e, BridgeInterface $bridge): string
+{
 	$title = $bridge->getName() . ' failed with error ' . $e->getCode();
 
 	// Build a GitHub compatible message