From 91cc0e627aca1372a3ca1493cbe97a9ec4a489b2 Mon Sep 17 00:00:00 2001
From: Bradley Schofield <ionicisere@gmail.com>
Date: Wed, 13 Nov 2024 10:29:43 +0900
Subject: [PATCH] Address Comments

---
 app/worker.php                               | 44 +++++++++++++++
 src/Appwrite/Platform/Workers/Migrations.php | 58 ++++++--------------
 2 files changed, 61 insertions(+), 41 deletions(-)

diff --git a/app/worker.php b/app/worker.php
index 4741afe7ea..6d55bdb2d3 100644
--- a/app/worker.php
+++ b/app/worker.php
@@ -277,6 +277,50 @@ Server::setResource(
     fn () => fn (Document $project, string $resourceType, ?string $resourceId) => false
 );
 
+Server::setResource('logError', function (Registry $register, Document $project) {
+    return function (Throwable $error, string $namespace, string $action, ?array $extras) use ($register, $project) {
+        $logger = $register->get('logger');
+
+        if ($logger) {
+            $version = System::getEnv('_APP_VERSION', 'UNKNOWN');
+
+            $log = new Log();
+            $log->setNamespace($namespace);
+            $log->setServer(\gethostname());
+            $log->setVersion($version);
+            $log->setType(Log::TYPE_ERROR);
+            $log->setMessage($error->getMessage());
+
+            $log->addTag('code', $error->getCode());
+            $log->addTag('verboseType', get_class($error));
+            $log->addTag('projectId', $project->getId() ?? '');
+
+            $log->addExtra('file', $error->getFile());
+            $log->addExtra('line', $error->getLine());
+            $log->addExtra('trace', $error->getTraceAsString());
+            
+
+            foreach ($extras as $key => $value) {
+                $log->addExtra($key, $value);
+            }
+
+            $log->setAction($action);
+
+            $isProduction = System::getEnv('_APP_ENV', 'development') === 'production';
+            $log->setEnvironment($isProduction ? Log::ENVIRONMENT_PRODUCTION : Log::ENVIRONMENT_STAGING);
+
+            try {
+                $responseCode = $logger->addLog($log);
+                Console::info('Error log pushed with status code: ' . $responseCode);
+            } catch (Throwable $th) {
+                Console::error('Error pushing log: ' . $th->getMessage());
+            }
+        }
+
+        Console::warning("Failed: {$error->getMessage()}");
+        Console::warning($error->getTraceAsString());
+    };
+}, ['register', 'project']);
 
 $pools = $register->get('pools');
 $platform = new Appwrite();
diff --git a/src/Appwrite/Platform/Workers/Migrations.php b/src/Appwrite/Platform/Workers/Migrations.php
index 2514c73d6a..6aa3b08211 100644
--- a/src/Appwrite/Platform/Workers/Migrations.php
+++ b/src/Appwrite/Platform/Workers/Migrations.php
@@ -39,7 +39,7 @@ class Migrations extends Action
 
     protected Document $project;
 
-    protected ?Logger $logger;
+    protected $logError;
 
     public static function getName(): string
     {
@@ -56,14 +56,14 @@ class Migrations extends Action
             ->inject('message')
             ->inject('dbForProject')
             ->inject('dbForConsole')
-            ->inject('logger')
-            ->callback(fn (Message $message, Database $dbForProject, Database $dbForConsole, ?Logger $logger) => $this->action($message, $dbForProject, $dbForConsole, $logger));
+            ->inject('logError')
+            ->callback(fn (Message $message, Database $dbForProject, Database $dbForConsole, callable $logError) => $this->action($message, $dbForProject, $dbForConsole, $logError));
     }
 
     /**
      * @throws Exception
      */
-    public function action(Message $message, Database $dbForProject, Database $dbForConsole, ?Logger $logger): void
+    public function action(Message $message, Database $dbForProject, Database $dbForConsole, callable $logError): void
     {
         $payload = $message->getPayload() ?? [];
 
@@ -82,7 +82,7 @@ class Migrations extends Action
         $this->dbForProject = $dbForProject;
         $this->dbForConsole = $dbForConsole;
         $this->project = $project;
-        $this->logger = $logger;
+        $this->logError = $logError;
 
         /**
          * Handle Event execution.
@@ -391,11 +391,21 @@ class Migrations extends Action
                 $source->error();
 
                 foreach ($source->getErrors() as $error) {
-                    $this->triggerExceptionLog($migration, $error);
+                    call_user_func($this->logError, $error, 'appwrite-worker', 'appwrite-queue-' . self::getName(), [
+                        'migrationId' => $migration->getId() ?? '',
+                        'source' => $migration->getAttribute('source') ?? '',
+                        'resourceName' => $error->getResourceName(),
+                        'resourceGroup' => $error->getResourceGroup()
+                    ]);
                 }
 
                 foreach ($destination->getErrors() as $error) {
-                    $this->triggerExceptionLog($migration, $error);
+                    call_user_func($this->logError, $error, 'appwrite-worker', 'appwrite-queue-' . self::getName(), [
+                        'migrationId' => $migration->getId() ?? '',
+                        'source' => $migration->getAttribute('source') ?? '',
+                        'resourceName' => $error->getResourceName(),
+                        'resourceGroup' => $error->getResourceGroup()
+                    ]);
                 }
             }
 
@@ -405,38 +415,4 @@ class Migrations extends Action
             }
         }
     }
-
-    public function triggerExceptionLog(Document $migration, MigrationException $error)
-    {
-        if (empty($this->logger)) {
-            return;
-        }
-
-        $log = new Log();
-
-        $log->setNamespace("appwrite-worker");
-        $log->setServer(\gethostname());
-        $log->setVersion(System::getEnv('_APP_VERSION', 'UNKNOWN'));
-        $log->setType(Log::TYPE_ERROR);
-        $log->setMessage($error->getMessage());
-        $log->setAction('appwrite-queue-' . self::getName());
-        $log->addTag('verboseType', get_class($error));
-        $log->addTag('projectId', $this->project->getId() ?? '');
-        $log->addExtra('file', $error->getFile());
-        $log->addExtra('line', $error->getLine());
-        $log->addExtra('trace', $error->getTraceAsString());
-        $log->addExtra('migrationId', $migration->getId() ?? '');
-        $log->addExtra('source', $migration->getAttribute('source') ?? '');
-        $log->addExtra('resourceName', $error->getResourceName());
-        $log->addExtra('resourceGroup', $error->getResourceGroup());
-        $isProduction = System::getEnv('_APP_ENV', 'development') === 'production';
-        $log->setEnvironment($isProduction ? Log::ENVIRONMENT_PRODUCTION : Log::ENVIRONMENT_STAGING);
-
-        try {
-            $responseCode = $this->logger->addLog($log);
-            Console::info('Error log pushed with status code: ' . $responseCode);
-        } catch (\Throwable $th) {
-            Console::error('Error pushing log: ' . $th->getMessage());
-        }
-    }
 }