Merge pull request #1547 from cayb0rg/issue/error-handling

API Error Handling Overhaul
ucfopen · May 14, 2024 · 1d34051 · 1d34051
2 parents 7441f64 + f566726
commit 1d34051
Show file tree

Hide file tree

Showing 79 changed files with 2,034 additions and 1,356 deletions.
diff --git a/fuel/app/classes/controller/admin.php b/fuel/app/classes/controller/admin.php
@@ -20,7 +20,7 @@ public function before()
 	public function get_widget()
 	{
 		if ( ! \Materia\Perm_Manager::is_super_user() ) throw new \HttpNotFoundException;
-	
+
 		Js::push_inline('var UPLOAD_ENABLED ="'.Config::get('materia.enable_admin_uploader').'";');
 		Js::push_inline('var HEROKU_WARNING ="'.Config::get('materia.heroku_admin_warning').'";');
 		Js::push_inline('var ACTION_LINK ="/admin/upload";');
@@ -78,8 +78,8 @@ public function post_upload()
 				}
 			}
 		}
-		
-		if ($failed) 
+
+		if ($failed)
 		{
 			throw new HttpServerErrorException;
 		}

diff --git a/fuel/app/classes/controller/api.php b/fuel/app/classes/controller/api.php
@@ -43,23 +43,64 @@ public function before()
 		parent::before();
 	}
 
+	/**
+	 * Recursively search for the status code in execution result
+	 * @param Array
+	 * @return Integer
+	 */
+	public function get_status($data)
+	{
+		if (is_array($data) || is_object($data))
+		{
+			foreach ($data as $key => $value)
+			{
+				if ($key === 'status')
+				{
+					return $value;
+				}
+				elseif (is_array($key) || is_object($key))
+				{
+					$result = $this->get_status($key);
+					if ($result !== null)
+					{
+						return $result;
+					}
+				}
+			}
+		}
+	}
+
 	public function post_call($version, $format, $method)
 	{
 		$input = json_decode(Input::post('data', []));
 
 		$result = $this->execute($version, $method, $input);
 
+		$status = $this->get_status($result);
+
+		if ( ! $status)
+		{
+			$status = 200;
+		}
+
 		$this->no_cache();
-		$this->response($result, 200);
+		$this->response($result, $status);
 	}
 
 	public function get_call($version, $format, $method)
 	{
 		$data   = array_slice($this->request->route->method_params, 3);
 		$result = $this->execute($version, $method, $data);
 
+		$status = $this->get_status($result);
+
+		if ( ! $status)
+		{
+			$status = 200;
+		}
+
 		$this->no_cache();
-		$this->response($result, 200);
+		$this->response($result, $status);
 	}
 
 	protected function execute($version, $method, $args)
@@ -77,6 +118,14 @@ protected function execute($version, $method, $args)
 		{
 			Materia\Log::profile([get_class($e), get_class($api), $method, json_encode($args)], 'exception');
 			trace($e);
+			if ($e instanceof \HttpNotFoundException)
+			{
+				return Materia\Msg::not_found();
+			}
+			else
+			{
+				throw new HttpServerErrorException;
+			}
 		}
 	}
 }
diff --git a/fuel/app/classes/controller/api/admin.php b/fuel/app/classes/controller/api/admin.php
@@ -125,7 +125,7 @@ public function post_widget_instance_undelete(string $inst_id)
 	{
 		if ( ! \Materia\Util_Validator::is_valid_hash($inst_id)) return Msg::invalid_input($inst_id);
 		if (\Service_User::verify_session() !== true) return Msg::no_login();
-		if ( ! ($inst = \Materia\Widget_Instance_Manager::get($inst_id, false, false, true))) return new Msg(Msg::ERROR, 'Widget instance does not exist.');
+		if ( ! ($inst = \Materia\Widget_Instance_Manager::get($inst_id, false, false, true))) return Msg::failure('Widget instance does not exist.');
 		return $inst->db_undelete();
 	}
 }
diff --git a/fuel/app/classes/controller/api/asset.php b/fuel/app/classes/controller/api/asset.php
@@ -34,7 +34,7 @@ public function post_delete($asset_id)
 			\Log::error('Error: In the deletion process');
 			\Log::error($th);
 
-			return new Msg(Msg::ERROR, 'Asset could not be deleted.');
+			return Msg::failure('Asset could not be deleted.');
 		}
 	}
 
@@ -61,7 +61,7 @@ public function post_restore($asset_id)
 			\Log::error('Error: In the deletion process');
 			\Log::error($th);
 
-			return new Msg(Msg::ERROR, 'Asset could not be restored.');
+			return Msg::failure('Asset could not be restored.');
 		}
 	}
 }
diff --git a/fuel/app/classes/controller/api/instance.php b/fuel/app/classes/controller/api/instance.php
@@ -19,10 +19,10 @@ class Controller_Api_Instance extends Controller_Rest
 	 */
 	public function get_history()
 	{
-		if ( ! $inst_id = Input::get('inst_id')) return $this->response('Requires an inst_id parameter!', 401);
+		if ( ! $inst_id = Input::get('inst_id')) return $this->response(\Materia\Msg::invalid_input('Requires an inst_id parameter!'), 401);
 		if ( ! \Materia\Util_Validator::is_valid_hash($inst_id) ) return $this->response(\Materia\Msg::invalid_input($inst_id), 401);
-		if ( ! ($inst = \Materia\Widget_Instance_Manager::get($inst_id))) return $this->response('Instance not found', 404);
-		if ( ! \Materia\Perm_Manager::user_has_any_perm_to(\Model_User::find_current_id(), $inst_id, \Materia\Perm::INSTANCE, [\Materia\Perm::FULL])) return $this->response(\Materia\Msg::no_login(), 401);
+		if ( ! ($inst = \Materia\Widget_Instance_Manager::get($inst_id))) return $this->response(new \Materia\Msg('Instance not found', \Materia\Msg::ERROR), 404);
+		if ( ! \Materia\Perm_Manager::user_has_any_perm_to(\Model_User::find_current_id(), $inst_id, \Materia\Perm::INSTANCE, [\Materia\Perm::FULL])) return $this->response(\Materia\Msg::no_perm(), 401);
 
 		$history = $inst->get_qset_history($inst_id);
 
@@ -36,13 +36,15 @@ public function post_request_access()
 		$inst_id = Input::json('inst_id', null);
 		$owner_id = Input::json('owner_id', null);
 
-		if ( ! $inst_id) return $this->response('Requires an inst_id parameter', 401);
-		if ( ! $owner_id) return $this->response('Requires an owner_id parameter', 401);
+		if ( ! $inst_id) return $this->response(new \Materia\Msg('Requires an inst_id parameter', \Materia\Msg::ERROR), 401);
 
-		if ( ! \Model_User::find_by_id($owner_id)) return $this->response('Owner not found', 404);
-		if ( ! ($inst = \Materia\Widget_Instance_Manager::get($inst_id))) return $this->response('Instance not found', 404);
+		if ( ! $owner_id) return $this->response(new \Materia\Msg('Requires an owner_id parameter', \Materia\Msg::ERROR), 401);
 
-		if ( ! Materia\Perm_Manager::user_has_any_perm_to($owner_id, $inst_id, Materia\Perm::INSTANCE, [Materia\Perm::FULL, Materia\Perm::VISIBLE])) return $this->response('Owner does not own instance', 404);
+		if ( ! \Model_User::find_by_id($owner_id)) return $this->response(new \Materia\Msg('Owner not found', \Materia\Msg::ERROR), 404);
+
+		if ( ! ($inst = \Materia\Widget_Instance_Manager::get($inst_id))) return $this->response(new \Materia\Msg('Instance not found', \Materia\Msg::ERROR), 404);
+
+		if ( ! Materia\Perm_Manager::user_has_any_perm_to($owner_id, $inst_id, Materia\Perm::INSTANCE, [Materia\Perm::FULL, Materia\Perm::VISIBLE])) return $this->response(new \Materia\Msg('Owner does not own instance', \Materia\Msg::ERROR), 404);
 
 		if ( ! \Materia\Util_Validator::is_valid_hash($inst_id) ) return $this->response(\Materia\Msg::invalid_input($inst_id), 401);
 

diff --git a/fuel/app/classes/controller/api/user.php b/fuel/app/classes/controller/api/user.php
@@ -20,7 +20,7 @@ public function get_activity()
 
 	public function post_settings()
 	{
-		if (\Service_User::verify_session() !== true) return $this->response('Not logged in', 401);
+		if (\Service_User::verify_session() !== true) return $this->response(\Materia\Msg::no_login(), 401);
 
 		$success   = false;
 		$set_meta  = [
@@ -42,9 +42,9 @@ public function post_settings()
 
 	public function post_roles()
 	{
-		if (\Service_User::verify_session() !== true) return $this->response('Not logged in', 401);
+		if (\Service_User::verify_session() !== true) return $this->response(\Materia\Msg::no_login(), 401);
 		// this endpoint is only available to superusers!
-		if ( ! \Materia\Perm_Manager::is_super_user()) return $this->response('Not authorized', 403);
+		if ( ! \Materia\Perm_Manager::is_super_user()) return $this->response(\Materia\Msg::no_perm(), 403);
 
 		$success = false;
 		$user_id = Input::json('id', null);
@@ -53,7 +53,7 @@ public function post_roles()
 			'support_user' => Input::json('support_user', false)
 		];
 
-		if ( ! $user_id) return $this->response('User ID not provided', 401);
+		if ( ! $user_id) return $this->response(\Materia\Msg::invalid_input('User ID not provided'), 401);
 
 		$current_roles = \Materia\Perm_Manager::get_user_roles($user_id);
 		$current_roles_condensed = array_map( fn($r) => $r->name, $current_roles);

diff --git a/fuel/app/classes/controller/media.php b/fuel/app/classes/controller/media.php
@@ -106,7 +106,7 @@ public function action_upload()
 		$name = Input::post('name', 'New Asset');
 		$asset = Widget_Asset_Manager::new_asset_from_file($name, $file_info);
 
-		if ( ! isset($asset->id))
+		if ( ! $asset || ! isset($asset->id))
 		{
 			// error
 			trace('Unable to create asset');