humhub · luke- · Jan 22, 2024 · Dec 21, 2023 · Dec 21, 2023 · Dec 21, 2023
diff --git a/CHANGELOG-DEV.md b/CHANGELOG-DEV.md
@@ -3,6 +3,7 @@ HumHub Changelog
 
 1.16.0 (Unreleased)
 -------------------
+- Fix #6770: Improve migration error messages
 - Fix #6772: Polymorphic relation lookup (Regression #6587)
 - Enh #6745: Harmonise term `enabled/disabled` vs `active/inactive` for modules
 - Fix #6754: Regression due to return type (#6550)

diff --git a/protected/humhub/commands/MigrateController.php b/protected/humhub/commands/MigrateController.php
@@ -12,7 +12,9 @@
 use humhub\helpers\DatabaseHelper;
 use humhub\services\MigrationService;
 use Yii;
+use yii\console\controllers\BaseMigrateController;
 use yii\console\Exception;
+use yii\db\Migration;
 use yii\db\MigrationInterface;
 use yii\web\Application;
 
@@ -77,6 +79,13 @@ class MigrateController extends \yii\console\controllers\MigrateController
      */
     protected array $migrationPathMap = [];
 
+    /**
+     * Stores the last executed migration.
+     *
+     * @var Migration|null
+     */
+    private ?Migration $lastMigration = null;
+
 
     /**
      * @inerhitdoc
@@ -149,7 +158,13 @@ protected function createMigration($class): MigrationInterface
             $this->migrationPath = $this->getMigrationPath($class);
         }
 
-        return parent::createMigration($class);
+        /**
+         * Storing the last executed migration
+         *
+         * @see BaseMigrateController::migrateUp()
+         * @see BaseMigrateController::migrateDown()
+         * */
+        return $this->lastMigration = parent::createMigration($class);
     }
 
     /**
@@ -180,7 +195,7 @@ public function getMigrationPath(string $migration): string
     protected function getMigrationPaths(): array
     {
         $migrationPaths = ['base' => $this->migrationPath];
-        foreach (Yii::$app->getModules() as $id => $config) {
+        foreach (($this->module ?? Yii::$app)->getModules() as $id => $config) {
             $class = null;
             if (is_array($config) && isset($config['class'])) {
                 $class = $config['class'];
@@ -265,4 +280,12 @@ public function stderr($string)
 
         return parent::stderr($string);
     }
+
+    /**
+     * @return mixed
+     */
+    public function getLastMigration()
+    {
+        return $this->lastMigration;
+    }
 }
diff --git a/protected/humhub/components/Migration.php b/protected/humhub/components/Migration.php
@@ -38,6 +38,11 @@ class Migration extends \yii\db\Migration
      */
     protected string $driverName;
 
+    /**
+     * @var Throwable|null Exception that occurred during migration
+     */
+    protected ?Throwable $lastException = null;
+
     /**
      * Initializes static::$driverName
      *
@@ -726,7 +731,14 @@ protected function logException(Throwable $e, string $method): void
      */
     private function printException(Throwable $t): void
     {
+        $this->lastException = $t;
+
         echo 'Exception: ' . $t->getMessage() . ' (' . $t->getFile() . ':' . $t->getLine() . ")\n";
         echo $t->getTraceAsString() . "\n";
     }
+
+    public function getLastException(): ?Throwable
+    {
+        return $this->lastException;
+    }
 }
diff --git a/protected/humhub/services/MigrationService.php b/protected/humhub/services/MigrationService.php
@@ -11,18 +11,22 @@
 use humhub\commands\MigrateController;
 use humhub\components\Application;
 use humhub\components\Event;
+use humhub\components\Migration;
 use humhub\components\Module;
 use humhub\events\MigrationEvent;
 use humhub\interfaces\ApplicationInterface;
 use humhub\libs\Helpers;
 use Throwable;
+use uninstall;
 use Yii;
 use yii\base\ActionEvent;
 use yii\base\Component;
 use yii\base\Controller;
 use yii\base\InvalidConfigException;
 use yii\base\Module as BaseModule;
 use yii\console\ExitCode;
+use yii\db\Exception;
+use yii\db\MigrationInterface;
 
 /**
  * @since 1.16
@@ -199,7 +203,7 @@ private function migrate(string $action): ?bool
             ]
         );
 
-        return $this->checkMigrationStatus($result);
+        return $this->checkMigrationStatus($result, $controller->getLastMigration());
     }
 
     /**
@@ -243,7 +247,7 @@ private function checkMigrationBefore(string $migrationAction): ?MigrationEvent
      * @throws InvalidConfigException
      * @throws Throwable
      */
-    private function checkMigrationStatus(MigrationEvent $result): bool
+    private function checkMigrationStatus(MigrationEvent $result, ?MigrationInterface $migration): bool
     {
         $this->lastMigrationOutput = $result->output ?: 'Migration output unavailable';
         $this->lastMigrationResult = $result->result;
@@ -260,13 +264,26 @@ private function checkMigrationStatus(MigrationEvent $result): bool
         /** @see \yii\console\controllers\BaseMigrateController::actionUp() */
         if ($result->result > ExitCode::OK) {
             $errorMessage = "Migration failed!";
-
-            if (YII_DEBUG) {
-                throw new InvalidConfigException($errorMessage);
+            $exception = null;
+
+            if ($migration instanceof Migration && $exception = $migration->getLastException()) {
+                $errorMessage .= "\n" . $exception->getMessage() . "\nSee application log for full trace.";
+            } elseif (
+                preg_match(
+                    '@^Exception:\s+(?<message>.*?$)\s+(?<trace>.*?\{main\})$@ms',
+                    $result->output ?? '',
+                    $matches
+                )
+            ) {
+                $errorMessage .= "\n" . $matches['message'] . "\nSee application log for full trace.";
             }
 
             Yii::error($errorMessage, $this->module->id);
 
+            if (YII_DEBUG) {
+                throw new InvalidConfigException($errorMessage, 0, $exception);
+            }
+
             return false;
         }
 
@@ -295,12 +312,12 @@ public function uninstall(): ?bool
         ob_start();
         require_once($uninstallMigration);
 
-        $migration = new \uninstall();
+        $migration = new uninstall();
         $migration->compact = false;
 
         try {
             $result->result = $migration->up() === false ? ExitCode::UNSPECIFIED_ERROR : ExitCode::OK;
-        } catch (\yii\db\Exception $ex) {
+        } catch (Exception $ex) {
             Yii::error($ex, $this->module->id);
             $result->result = ExitCode::UNSPECIFIED_ERROR;
         }
@@ -311,19 +328,19 @@ public function uninstall(): ?bool
          */
         $migrations = opendir($path);
         $params = [];
-        while (false !== ($migration = readdir($migrations))) {
-            if ($migration === '.' || $migration === '..' || $migration === 'uninstall.php') {
+        while (false !== ($filename = readdir($migrations))) {
+            if ($filename === '.' || $filename === '..' || $filename === 'uninstall.php') {
                 continue;
             }
 
             $command ??= Yii::$app->db->createCommand()->delete('migration', 'version = :version', $params);
 
-            $version = str_replace('.php', '', $migration);
+            $version = str_replace('.php', '', $filename);
             $command->bindValue(':version', $version)->execute();
             $result->output .= "    > migration entry $version removed.\n";
         }
 
-        return $this->checkMigrationStatus($result);
+        return $this->checkMigrationStatus($result, $migration);
     }
 
     /**

diff --git a/protected/humhub/tests/codeception/unit/components/ModuleManagerTest.php b/protected/humhub/tests/codeception/unit/components/ModuleManagerTest.php
@@ -20,14 +20,15 @@
 use Some\Name\Space\module1\Module as Module1;
 use Some\Name\Space\module2\Module as Module2;
 use Some\Name\Space\moduleWithMigration\Module as ModuleWithMigration;
+use SplFileInfo;
 use tests\codeception\_support\HumHubDbTestCase;
+use Throwable;
 use Yii;
 use yii\base\ErrorException;
 use yii\base\Event;
 use yii\base\Exception;
 use yii\base\InvalidConfigException;
 use yii\caching\ArrayCache;
-use yii\console\ExitCode;
 use yii\db\StaleObjectException;
 use yii\helpers\FileHelper;
 use yii\log\Logger;
@@ -70,8 +71,8 @@ public static function setUpBeforeClass(): void
             if ($filename !== '.' && $filename !== '..' && is_dir("$appModuleRoot/$filename")) {
                 static::$moduleDirCount++;
 
-                $config_file = new \SplFileInfo("$appModuleRoot/$filename/config.php");
-                $config_file = new \SplFileInfo($config_file->getRealPath());
+                $config_file = new SplFileInfo("$appModuleRoot/$filename/config.php");
+                $config_file = new SplFileInfo($config_file->getRealPath());
 
                 if (!$config_file->isFile() || !$config_file->isReadable()) {
                     continue;
@@ -434,20 +435,25 @@ public function testGetEnabledModules()
         // Workaround for internal SaaS core module
         unset($modules['hostinginfo']);
 
+        $locallyEnabledModules = array_intersect_key(static::$moduleDirList, array_flip(array_column(
+            static::dbSelect('module_enabled', 'module_id'),
+            'module_id'
+        )));
+
         $expected = array_merge(
             [],
             array_flip(ModuleAutoLoaderTest::EXPECTED_CORE_MODULES),
-            static::$moduleDirList
+            $locallyEnabledModules
         );
 
-
         static::assertEquals($expected, $modules);
 
         static::assertIsArray($modules = $moduleManager->getEnabledModules([
             'enabled' => false,
             'returnClass' => true,
         ]));
-        static::assertEquals(static::$moduleDirList, $modules);
+
+        static::assertEquals($locallyEnabledModules, $modules);
     }
 
     /**
@@ -514,7 +520,7 @@ public function testRemoveModule()
      * @noinspection MissedFieldInspection
      */
     /**
-     * @throws \Throwable
+     * @throws Throwable
      * @throws InvalidConfigException
      * @throws StaleObjectException
      * @throws Exception
@@ -619,11 +625,27 @@ public function testEnableModules()
 
         Yii::$app->set('moduleManager', $oldMM);
 
-        static::assertNotLog('Module has not been enabled due to beforeEnable() returning false', Logger::LEVEL_WARNING, [$module->id]);
-        static::assertLog('Module has no migrations directory.', Logger::LEVEL_TRACE, [$module->id]);
+        static::assertNotLog(
+            'Module has not been enabled due to beforeEnable() returning false',
+            Logger::LEVEL_WARNING,
+            [$module->id]
+        );
+        static::assertLog(
+            'Module has no migrations directory.',
+            Logger::LEVEL_TRACE,
+            [$module->id]
+        );
 
-        static::assertNotLog('Module has not been enabled due to beforeEnable() returning false', Logger::LEVEL_WARNING, ['module2']);
-        static::assertLogRegex('@No new migrations found\. Your system is up-to-date\.@', Logger::LEVEL_INFO, ['module2']);
+        static::assertNotLog(
+            'Module has not been enabled due to beforeEnable() returning false',
+            Logger::LEVEL_WARNING,
+            ['module2']
+        );
+        static::assertLogRegex(
+            '@No new migrations found\. Your system is up-to-date\.@',
+            Logger::LEVEL_INFO,
+            ['module2']
+        );
 
         static::logReset();
 
@@ -1101,10 +1123,14 @@ public function reset(): void
             ]
         ]);
 
-        static::$moduleEnabledList ??= array_column(
-            static::dbSelect('module_enabled', 'module_id'),
-            'module_id'
-        );
+        if (Yii::$app->isDatabaseInstalled()) {
+            static::$moduleEnabledList ??= array_column(
+                static::dbSelect('module_enabled', 'module_id'),
+                'module_id'
+            );
+        } else {
+            static::$moduleEnabledList ??= [];
+        }
 
         $this->moduleManager = new ModuleManagerMock();
     }