From b62640489dde923fbe2cecf92a20bd59a062c44a Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Tue, 12 Nov 2024 11:29:19 +0900 Subject: [PATCH 1/6] Fire logs for each individual migration error --- src/Appwrite/Platform/Workers/Migrations.php | 54 +++++++++++++++++--- 1 file changed, 48 insertions(+), 6 deletions(-) diff --git a/src/Appwrite/Platform/Workers/Migrations.php b/src/Appwrite/Platform/Workers/Migrations.php index 4b1926ed26..91056d5a49 100644 --- a/src/Appwrite/Platform/Workers/Migrations.php +++ b/src/Appwrite/Platform/Workers/Migrations.php @@ -17,6 +17,7 @@ use Utopia\Database\Exception\Restricted; use Utopia\Database\Exception\Structure; use Utopia\Database\Helpers\ID; use Utopia\Logger\Log; +use Utopia\Logger\Logger; use Utopia\Migration\Destination; use Utopia\Migration\Destinations\Appwrite as DestinationAppwrite; use Utopia\Migration\Exception as MigrationException; @@ -28,6 +29,7 @@ use Utopia\Migration\Sources\Supabase; use Utopia\Migration\Transfer; use Utopia\Platform\Action; use Utopia\Queue\Message; +use Utopia\System\System; class Migrations extends Action { @@ -37,6 +39,8 @@ class Migrations extends Action protected Document $project; + protected Logger $logger; + public static function getName(): string { return 'migrations'; @@ -53,13 +57,14 @@ class Migrations extends Action ->inject('dbForProject') ->inject('dbForConsole') ->inject('log') - ->callback(fn (Message $message, Database $dbForProject, Database $dbForConsole, Log $log) => $this->action($message, $dbForProject, $dbForConsole, $log)); + ->inject('logger') + ->callback(fn (Message $message, Database $dbForProject, Database $dbForConsole, Log $log, Logger $logger) => $this->action($message, $dbForProject, $dbForConsole, $log, $logger)); } /** * @throws Exception */ - public function action(Message $message, Database $dbForProject, Database $dbForConsole, Log $log): void + public function action(Message $message, Database $dbForProject, Database $dbForConsole, Log $log, Logger $logger): void { $payload = $message->getPayload() ?? []; @@ -78,6 +83,7 @@ class Migrations extends Action $this->dbForProject = $dbForProject; $this->dbForConsole = $dbForConsole; $this->project = $project; + $this->logger = $logger; /** * Handle Event execution. @@ -324,7 +330,6 @@ class Migrations extends Action $errorMessages = []; foreach ($sourceErrors as $error) { - /** @var $sourceErrors $error */ $message = "Error occurred while fetching '{$error->getResourceName()}:{$error->getResourceId()}' from source with message: '{$error->getMessage()}'"; if ($error->getPrevious()) { $message .= " Message: ".$error->getPrevious()->getMessage() . " File: ".$error->getPrevious()->getFile() . " Line: ".$error->getPrevious()->getLine(); @@ -359,7 +364,6 @@ class Migrations extends Action if (! $migration->isEmpty()) { $migration->setAttribute('status', 'failed'); $migration->setAttribute('stage', 'finished'); - $migration->setAttribute('errors', [$th->getMessage()]); return; } @@ -379,7 +383,6 @@ class Migrations extends Action } $migration->setAttribute('errors', $errorMessages); - $log->addTag('migrationErrors', json_encode($errorMessages)); } } finally { if (! $tempAPIKey->isEmpty()) { @@ -394,7 +397,13 @@ class Migrations extends Action $destination->error(); $source->error(); - throw new Exception('Migration failed'); + foreach ($source->getErrors() as $error) { + $this->triggerExceptionLog($migration, $error); + } + + foreach ($destination->getErrors() as $error) { + $this->triggerExceptionLog($migration, $error); + } } if ($migration->getAttribute('status', '') === 'completed') { @@ -403,4 +412,37 @@ 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() ?? 'n/a'); + $log->addExtra('file', $error->getFile()); + $log->addExtra('line', $error->getLine()); + $log->addExtra('trace', $error->getTraceAsString()); + $log->addExtra('migrationId', $migration->getId() ?? 'n/a'); + $log->addExtra('source', $migration->getAttribute('source') ?? 'n/a'); + $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()); + } + } } From a460a41fe56ca2a5ed893bfae62e9580ae10d264 Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Tue, 12 Nov 2024 11:33:13 +0900 Subject: [PATCH 2/6] Run Linter --- src/Appwrite/Platform/Workers/Migrations.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Appwrite/Platform/Workers/Migrations.php b/src/Appwrite/Platform/Workers/Migrations.php index 91056d5a49..770caa2e69 100644 --- a/src/Appwrite/Platform/Workers/Migrations.php +++ b/src/Appwrite/Platform/Workers/Migrations.php @@ -413,7 +413,8 @@ class Migrations extends Action } } - public function triggerExceptionLog(Document $migration, MigrationException $error) { + public function triggerExceptionLog(Document $migration, MigrationException $error) + { if (empty($this->logger)) { return; } @@ -425,7 +426,7 @@ class Migrations extends Action $log->setVersion(System::getEnv('_APP_VERSION', 'UNKNOWN')); $log->setType(Log::TYPE_ERROR); $log->setMessage($error->getMessage()); - $log->setAction('appwrite-queue-' . Self::getName()); + $log->setAction('appwrite-queue-' . self::getName()); $log->addTag('verboseType', get_class($error)); $log->addTag('projectId', $this->project->getId() ?? 'n/a'); $log->addExtra('file', $error->getFile()); @@ -444,5 +445,5 @@ class Migrations extends Action } catch (\Throwable $th) { Console::error('Error pushing log: ' . $th->getMessage()); } - } + } } From 1337ea77b1cb764e8e4cc81a45b16129d306e910 Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Tue, 12 Nov 2024 16:18:09 +0900 Subject: [PATCH 3/6] Fix Optionals --- src/Appwrite/Platform/Workers/Migrations.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Appwrite/Platform/Workers/Migrations.php b/src/Appwrite/Platform/Workers/Migrations.php index 770caa2e69..921d6dc69a 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 ?Logger $logger; public static function getName(): string { @@ -58,13 +58,13 @@ class Migrations extends Action ->inject('dbForConsole') ->inject('log') ->inject('logger') - ->callback(fn (Message $message, Database $dbForProject, Database $dbForConsole, Log $log, Logger $logger) => $this->action($message, $dbForProject, $dbForConsole, $log, $logger)); + ->callback(fn (Message $message, Database $dbForProject, Database $dbForConsole, Log $log, ?Logger $logger) => $this->action($message, $dbForProject, $dbForConsole, $log, $logger)); } /** * @throws Exception */ - public function action(Message $message, Database $dbForProject, Database $dbForConsole, Log $log, Logger $logger): void + public function action(Message $message, Database $dbForProject, Database $dbForConsole, Log $log, ?Logger $logger): void { $payload = $message->getPayload() ?? []; From 29e7d1f7d07719c580be7a7c2d71912e492b61fe Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Wed, 13 Nov 2024 09:13:00 +0900 Subject: [PATCH 4/6] Address Comments --- src/Appwrite/Platform/Workers/Migrations.php | 21 +++++++------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/src/Appwrite/Platform/Workers/Migrations.php b/src/Appwrite/Platform/Workers/Migrations.php index 921d6dc69a..2514c73d6a 100644 --- a/src/Appwrite/Platform/Workers/Migrations.php +++ b/src/Appwrite/Platform/Workers/Migrations.php @@ -56,15 +56,14 @@ class Migrations extends Action ->inject('message') ->inject('dbForProject') ->inject('dbForConsole') - ->inject('log') ->inject('logger') - ->callback(fn (Message $message, Database $dbForProject, Database $dbForConsole, Log $log, ?Logger $logger) => $this->action($message, $dbForProject, $dbForConsole, $log, $logger)); + ->callback(fn (Message $message, Database $dbForProject, Database $dbForConsole, ?Logger $logger) => $this->action($message, $dbForProject, $dbForConsole, $logger)); } /** * @throws Exception */ - public function action(Message $message, Database $dbForProject, Database $dbForConsole, Log $log, ?Logger $logger): void + public function action(Message $message, Database $dbForProject, Database $dbForConsole, ?Logger $logger): void { $payload = $message->getPayload() ?? []; @@ -92,10 +91,7 @@ class Migrations extends Action return; } - $log->addTag('migrationId', $migration->getId()); - $log->addTag('projectId', $project->getId()); - - $this->processMigration($migration, $log); + $this->processMigration($migration); } /** @@ -265,7 +261,7 @@ class Migrations extends Action * @throws \Utopia\Database\Exception * @throws Exception */ - protected function processMigration(Document $migration, Log $log): void + protected function processMigration(Document $migration): void { $project = $this->project; $projectDocument = $this->dbForConsole->getDocument('projects', $project->getId()); @@ -291,8 +287,6 @@ class Migrations extends Action $migration->setAttribute('status', 'processing'); $this->updateMigrationDocument($migration, $projectDocument); - $log->addTag('type', $migration->getAttribute('source')); - $source = $this->processSource($migration); $destination = $this->processDestination($migration, $tempAPIKey->getAttribute('secret')); @@ -349,7 +343,6 @@ class Migrations extends Action } $migration->setAttribute('errors', $errorMessages); - $log->addExtra('migrationErrors', json_encode($errorMessages)); $this->updateMigrationDocument($migration, $projectDocument); return; @@ -428,12 +421,12 @@ class Migrations extends Action $log->setMessage($error->getMessage()); $log->setAction('appwrite-queue-' . self::getName()); $log->addTag('verboseType', get_class($error)); - $log->addTag('projectId', $this->project->getId() ?? 'n/a'); + $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() ?? 'n/a'); - $log->addExtra('source', $migration->getAttribute('source') ?? 'n/a'); + $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'; From 91cc0e627aca1372a3ca1493cbe97a9ec4a489b2 Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Wed, 13 Nov 2024 10:29:43 +0900 Subject: [PATCH 5/6] 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()); - } - } } From 54db5655a15b362a51266cc6f595dcad930d6986 Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Wed, 13 Nov 2024 13:42:49 +0900 Subject: [PATCH 6/6] Run Linter --- app/worker.php | 2 +- src/Appwrite/Platform/Workers/Migrations.php | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/app/worker.php b/app/worker.php index 6d55bdb2d3..6b7aba5c1b 100644 --- a/app/worker.php +++ b/app/worker.php @@ -298,7 +298,7 @@ Server::setResource('logError', function (Registry $register, Document $project) $log->addExtra('file', $error->getFile()); $log->addExtra('line', $error->getLine()); $log->addExtra('trace', $error->getTraceAsString()); - + foreach ($extras as $key => $value) { $log->addExtra($key, $value); diff --git a/src/Appwrite/Platform/Workers/Migrations.php b/src/Appwrite/Platform/Workers/Migrations.php index 6aa3b08211..fdd885effa 100644 --- a/src/Appwrite/Platform/Workers/Migrations.php +++ b/src/Appwrite/Platform/Workers/Migrations.php @@ -16,8 +16,6 @@ use Utopia\Database\Exception\Conflict; use Utopia\Database\Exception\Restricted; use Utopia\Database\Exception\Structure; use Utopia\Database\Helpers\ID; -use Utopia\Logger\Log; -use Utopia\Logger\Logger; use Utopia\Migration\Destination; use Utopia\Migration\Destinations\Appwrite as DestinationAppwrite; use Utopia\Migration\Exception as MigrationException; @@ -29,7 +27,6 @@ use Utopia\Migration\Sources\Supabase; use Utopia\Migration\Transfer; use Utopia\Platform\Action; use Utopia\Queue\Message; -use Utopia\System\System; class Migrations extends Action {