fix(Core/Updater): fail the dry run when a SQL update fails to apply (#26795)

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

.github/workflows/dashboard-ci.yml

@@ -247,18 +247,36 @@ jobs:
         run: |
           ./acore.sh test core
 
+      # This job runs zero-conf, so the core falls back to console-only logging and
+      # writes no Errors.log, which is what the build workflows scrape. The grep is a
+      # second net in case a later change stops the updater from failing the run on a
+      # bad import.
       - name: Test authserver dry-run
         run: |
           source ./acore.sh config load
           cd env/dist/bin
-          timeout 5m ./authserver -dry-run
+          timeout 5m ./authserver -dry-run 2>&1 | tee "$RUNNER_TEMP/authserver-dry-run.log"
+          status=${PIPESTATUS[0]}
+          [ "$status" -eq 0 ] || exit "$status"
+
+          if grep -qE "Applying of file .+ failed!" "$RUNNER_TEMP/authserver-dry-run.log"; then
+            echo "::error::A SQL update failed to apply during the authserver dry run"
+            exit 1
+          fi
         continue-on-error: false
 
       - name: Test worldserver dry-run
         run: |
           source ./acore.sh config load
           cd env/dist/bin
-          timeout 5m ./worldserver -dry-run
+          timeout 5m ./worldserver -dry-run 2>&1 | tee "$RUNNER_TEMP/worldserver-dry-run.log"
+          status=${PIPESTATUS[0]}
+          [ "$status" -eq 0 ] || exit "$status"
+
+          if grep -qE "Applying of file .+ failed!" "$RUNNER_TEMP/worldserver-dry-run.log"; then
+            echo "::error::A SQL update failed to apply during the worldserver dry run"
+            exit 1
+          fi
         continue-on-error: false
 
 
@@ -288,6 +306,46 @@ jobs:
         timeout-minutes: 30
         continue-on-error: false
 
+      # Proves the dry-run steps above actually fail on a bad import.
+      - name: Verify a broken SQL update fails the dry run
+        run: |
+          # Absolute: the trap fires after the cd below.
+          probe="$PWD/data/sql/updates/pending_db_world/rev_9999999999999999999.sql"
+          trap 'rm -f "$probe"' EXIT
+
+          cat > "$probe" <<'EOF'
+          DELETE FROM `ci_gate_probe_table_that_does_not_exist` WHERE `id` = 1;
+          EOF
+
+          source ./acore.sh config load
+          cd env/dist/bin
+
+          # Without this the step fails on a healthy tree as soon as it gains a
+          # `shell: bash`: that turns pipefail on, so errexit aborts on the exit 1 the
+          # assertions below are here to check for.
+          set +o pipefail
+          timeout 5m ./worldserver -dry-run 2>&1 | tee "$RUNNER_TEMP/broken-update-dry-run.log"
+          status=${PIPESTATUS[0]}
+
+          # Exactly 1, so a hang (timeout returns 124) does not pass as a detection.
+          if [ "$status" -ne 1 ]; then
+            echo "::error::expected worldserver -dry-run to exit 1 on a broken SQL update, got $status"
+            exit 1
+          fi
+
+          # 1 is also the generic startup failure code, so require the updater to be the one
+          # that failed the run, on the probe file specifically.
+          if ! grep -qE "Applying of file .*rev_9999999999999999999\.sql.* failed!" "$RUNNER_TEMP/broken-update-dry-run.log"; then
+            echo "::error::worldserver -dry-run exited 1 without reporting the broken probe update"
+            exit 1
+          fi
+
+          if ! grep -q "Failed Database Update" "$RUNNER_TEMP/broken-update-dry-run.log"; then
+            echo "::error::the dry run did not terminate on the failed update count"
+            exit 1
+          fi
+        timeout-minutes: 10
+
       # The old "only save on a cache miss" guard is dropped: the per-run key
       # only ever collides on a re-run, since github.run_id is stable across
       # attempts, and there the save is a no-op anyway.

src/server/apps/authserver/Main.cpp

@@ -27,6 +27,7 @@
 #include "AuthSocketMgr.h"
 #include "Banner.h"
 #include "Config.h"
+#include "DBUpdater.h"
 #include "DatabaseEnv.h"
 #include "DatabaseLoader.h"
 #include "GitRevision.h"
@@ -149,6 +150,12 @@ int main(int argc, char** argv)
     // Stop auth server if dry run
     if (sConfigMgr->isDryRun())
     {
+        if (uint32 failed = DBUpdaterUtil::GetFailedUpdateCount())
+        {
+            LOG_FATAL("server.authserver", "Dry run completed with {} failed database update(s), terminating.", failed);
+            return 1;
+        }
+
         LOG_INFO("server.authserver", "Dry run completed, terminating.");
         return 0;
     }

src/server/database/Updater/DBUpdater.cpp

@@ -64,6 +64,22 @@ std::string& DBUpdaterUtil::corrected_path()
     return path;
 }
 
+uint32& DBUpdaterUtil::failed_updates()
+{
+    static uint32 count = 0;
+    return count;
+}
+
+void DBUpdaterUtil::MarkUpdateFailed()
+{
+    ++failed_updates();
+}
+
+uint32 DBUpdaterUtil::GetFailedUpdateCount()
+{
+    return failed_updates();
+}
+
 // Auth Database
 template<>
 std::string DBUpdater<LoginDatabaseConnection>::GetConfigEntry()
@@ -524,6 +540,10 @@ void DBUpdater<T>::ApplyFile(DatabaseWorkerPool<T>& pool, std::string const& hos
             "If you are a developer, please fix your sql query.",
             path.generic_string(), pool.GetConnectionInfo()->database);
 
+        // Recorded in both modes. A dry run does not throw below, so it keeps attempting the
+        // remaining files and this count is the only thing left to fail the run on.
+        DBUpdaterUtil::MarkUpdateFailed();
+
         if (!sConfigMgr->isDryRun())
         {
             if (uint32 delay = sConfigMgr->GetOption<uint32>("Updates.ExceptionShutdownDelay", 10000))

src/server/database/Updater/DBUpdater.h

@@ -60,8 +60,15 @@ public:
 
     static bool CheckExecutable();
 
+    // Counts every update file that failed to apply, in any mode. A dry run does not throw
+    // on a bad file, so it keeps going and a single run reports all of them; whoever ends
+    // the run must check this and exit non-zero, otherwise CI goes green on a failed import.
+    static void MarkUpdateFailed();
+    static uint32 GetFailedUpdateCount();
+
 private:
     static std::string& corrected_path();
+    static uint32& failed_updates();
 };
 
 template <class T>

src/server/game/World/World.cpp

@@ -42,6 +42,7 @@
 #include "CreatureGroups.h"
 #include "CreatureTextMgr.h"
 #include "DBCStores.h"
+#include "DBUpdater.h"
 #include "DatabaseEnv.h"
 #include "DisableMgr.h"
 #include "DynamicVisibility.h"
@@ -1058,6 +1059,13 @@ void World::SetInitialWorldSettings()
     if (sConfigMgr->isDryRun())
     {
         sMapMgr->UnloadAll();
+
+        if (uint32 failed = DBUpdaterUtil::GetFailedUpdateCount())
+        {
+            LOG_FATAL("server.loading", "AzerothCore Dry Run Completed With {} Failed Database Update(s), Terminating.", failed);
+            exit(1);
+        }
+
         LOG_INFO("server.loading", "AzerothCore Dry Run Completed, Terminating.");
         exit(0);
     }

src/tools/dbimport/Main.cpp

@@ -17,6 +17,7 @@
 
 #include "Banner.h"
 #include "Config.h"
+#include "DBUpdater.h"
 #include "DatabaseEnv.h"
 #include "DatabaseLoader.h"
 #include "IoContext.h"
@@ -95,6 +96,12 @@ int main(int argc, char** argv)
 
     LOG_INFO("dbimport", "Halting process...");
 
+    if (uint32 failed = DBUpdaterUtil::GetFailedUpdateCount())
+    {
+        LOG_FATAL("dbimport", "{} update file(s) failed to apply!", failed);
+        return 1;
+    }
+
     return 0;
 }