OnPull bug fixes

CHANGELOG.md

@@ -15,6 +15,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Web UI workspace view labels changes as Merge Conflict if there are unmerged changes (#890)
 - Web UI workspace view displays diff correctly for files with merge conflicts (#898)
 - Storage definition changes in persistent classes are now correctly exported to the Git repository (#906)
+- Catch and log exceptions thrown by pull handlers (#909)
+- Fix Incremental Load to first remove production items before removing classes (#907)
 
 ## [2.14.0] - 2025-11-07
 

cls/SourceControl/Git/PullEventHandler.cls

@@ -24,7 +24,9 @@ Method OnPull() As %Status [ Abstract ]
 /// <var>pullEventClass</var>: if defined, override the configured pull event class
 ClassMethod ForModifications(ByRef files, pullEventClass As %String) As %Status
 {
-    set st = $$$OK
+    #Dim err As %Exception.AbstractException
+    #Dim pullError As %Exception.AbstractException
+    #Dim st As %Status = $$$OK
     try {
         set log = ##class(SourceControl.Git.DeploymentLog).%New()
         set log.HeadRevision = ##class(SourceControl.Git.Utils).GetCurrentRevision()
@@ -33,15 +35,20 @@ ClassMethod ForModifications(ByRef files, pullEventClass As %String) As %Status
         quit:$$$ISERR(st)
         set event = $classmethod(
             $select(
-                $data(pullEventClass)#2: pullEventClass, 
+                $data(pullEventClass)#2: pullEventClass,
                 1: ##class(SourceControl.Git.Utils).PullEventClass())
             ,"%New")
         set event.LocalRoot = ##class(SourceControl.Git.Utils).TempFolder()
         merge event.ModifiedFiles = files
-        set st = event.OnPull()
+        try {
+            set st = event.OnPull()
+        }
+        catch pullError {
+            set st = pullError.AsStatus()
+        }
         set log.EndTimestamp = $zdatetime($ztimestamp,3)
         set log.Status = st
-        set st = log.%Save()
+        set st = log.%Save()  // this hides any errors returned or thrown by OnPull()
         quit:$$$ISERR(st)
     } catch err {
         set st = err.AsStatus()

cls/SourceControl/Git/PullEventHandler/IncrementalLoad.cls

@@ -9,7 +9,7 @@ Parameter DESCRIPTION = "Performs an incremental load and compile of all changes
 
 Method OnPull() As %Status
 {
-    set sc = $$$OK
+    #Dim sc As %Status = $$$OK
 
     // certain items must be imported before everything else.
     for i=1:1:$get(..ModifiedFiles) {
@@ -21,22 +21,20 @@ Method OnPull() As %Status
     }
 
     set nFiles = 0
-
     for i=1:1:$get(..ModifiedFiles){
         set internalName = ..ModifiedFiles(i).internalName
-        
+
         // Don't import the config file a second time
         continue:internalName=##class(SourceControl.Git.Settings.Document).#INTERNALNAME
-
+        set tExternalName = ..ModifiedFiles(i).externalName
+
         if ((internalName = "") && (..ModifiedFiles(i).changeType '= "D")) {
-            write !, ..ModifiedFiles(i).externalName, " was not imported into the database and will not be compiled. "
+            write !, tExternalName, " was not imported into the database and will not be compiled. "
         } elseif (..ModifiedFiles(i).changeType = "D") {
-            set delSC = ..DeleteFile(internalName, ..ModifiedFiles(i).externalName)
-            if delSC {
-                write !, ..ModifiedFiles(i).externalName, " was deleted."
-            } else {
-                write !, "WARNING: Deletion of ", ..ModifiedFiles(i).externalName, " failed."
-            }
+            #; Deleted production items will not have an internal name
+            set delIndex = $select(##class(SourceControl.Git.Util.Production).ItemIsPTD(tExternalName): 1, 1: 2)
+            set:(delIndex=1) internalName = "-"_i   ;; This is a total hack because deleted production items will not have an internal name
+            set delList(delIndex, internalName) = tExternalName
         } else {
             set nFiles = nFiles + 1
             if (##class(SourceControl.Git.Utils).Type(internalName) = "ptd") {
@@ -48,29 +46,67 @@ Method OnPull() As %Status
         }
     }
 
+    #; First remove production items, then everything else
+    write:$data(delList) !!,"Removing items..."
+    for orderIndex = 1,2 {
+        set item = $order(delList(orderIndex, ""), 1, tExternalName)
+        while (item '= "") {
+            set delSC = ..RemoveItem(item, tExternalName)
+            set item = $order(delList(orderIndex, item), 1, tExternalName)
+        }
+    }
     if (nFiles = 0) {
-        write !, "Nothing to compile."
+        write !, "Nothing to Load."
         quit $$$OK
     }
-    set sc = $$$ADDSC(sc,$system.OBJ.CompileList(.compilelist, "ckbryu"))
-    // after compilation, deploy any PTD items
-    set key = $order(ptdList(""))
-    while (key '= "") {
-        set sc = $$$ADDSC(sc, ##class(SourceControl.Git.Utils).ImportItem(key,1))
-        set key = $order(ptdList(key))
+
+    if $data(compilelist) {
+        write !
+        set sc = $$$ADDSC(sc,$system.OBJ.CompileList(.compilelist, "ckbryu"))
+    }
+    else {
+        write !,"Nothing to Compile."
     }
+
+    #; Deploy any PTD items
+    if $data(ptdList) {
+        write !,"Loading production items..."
+        set start = $zhorolog
+        set key = $order(ptdList(""))
+        while (key '= "") {
+            set sc = $$$ADDSC(sc, ##class(SourceControl.Git.Utils).ImportItem(key,1))
+            set key = $order(ptdList(key))
+        }
+        write !,"Production items imported in ",($zhorolog-start),"s"
+    }
+
     if $$$comClassDefined("Ens.Director") && ##class(Ens.Director).IsProductionRunning() {
+        set start = $zhorolog
         write !,"Updating production... "
         set sc = $$$ADDSC(sc,##class(Ens.Director).UpdateProduction())
-        write "done."
+        write "done in ",($zhorolog-start),"s."
     }
     quit sc
 }
 
+Method RemoveItem(pInternalName, pExternalName) As %Status
+{
+    #Dim delSC As %Status
+    set delSC = ..DeleteFile(pInternalName, pExternalName)
+    if delSC {
+        write !, pExternalName, " was deleted."
+    } else {
+        write !, "WARNING: Deletion of ", pExternalName, " failed."
+        write !,?3,$system.Status.DisplayError(delSC)
+    }
+    quit delSC
+}
+
 Method DeleteFile(item As %String = "", externalName As %String = "") As %Status
 {
+    #Dim sc As %Status = $$$OK
+    #Dim err As %Exception.AbstractException
     try {
-        set sc = $$$OK
         set type = $select(
             ##class(SourceControl.Git.Util.Production).ItemIsPTD(externalName): "ptd",
             1: ##class(SourceControl.Git.Utils).Type(item)
@@ -80,28 +116,28 @@ Method DeleteFile(item As %String = "", externalName As %String = "") As %Status
         set deleted = 1
         if type = "prj" {
             set sc = $system.OBJ.DeleteProject(name)
-        }elseif type = "cls" {
+        } elseif type = "cls" {
             if ##class(SourceControl.Git.Utils).ItemIsProductionToDecompose(name) {
                 write !, "Production decomposition enabled, skipping delete of production class"
             } else {
                 set sc = $system.OBJ.Delete(item)
             }
-        }elseif $listfind($listbuild("mac","int","inc","bas","mvb","mvi"), type) > 0 {
+        } elseif $listfind($listbuild("mac","int","inc","bas","mvb","mvi"), type) > 0 {
             set sc = ##class(%Routine).Delete(item)
-        }elseif type = "csp" {
+        } elseif type = "csp" {
             set sc = $System.CSP.DeletePage(item)
         } elseif settings.decomposeProductions && (type = "ptd") {
             set normalizedFilePath = ##class(%File).NormalizeFilename(##class(SourceControl.Git.Utils).TempFolder()_externalName)
             set sc = ##class(%SYSTEM.Status).AppendStatus(
                 ##class(SourceControl.Git.Production).RemoveItemByExternalName(normalizedFilePath,"FullExternalName"),
                 ##class(%Library.RoutineMgr).Delete(item)
             )
-        }elseif ##class(SourceControl.Git.Utils).UserTypeCached(item) {
+        } elseif ##class(SourceControl.Git.Utils).UserTypeCached(item) {
             set sc = ##class(%Library.RoutineMgr).Delete(item)
         } else {
             set deleted = 0
         }
-        
+
         if deleted && $$$ISOK(sc) {
             if (item '= "") {
                 do ##class(SourceControl.Git.Utils).RemoveRoutineTSH(item)
@@ -115,16 +151,16 @@ Method DeleteFile(item As %String = "", externalName As %String = "") As %Status
         }
         // Force the catch if failing
         $$$ThrowOnError(sc)
-    } catch e {
+    } catch err {
         set filename = ##class(SourceControl.Git.Utils).FullExternalName(item)
         if (filename = "") || '##class(%File).Exists(filename) {
             do ##class(SourceControl.Git.Utils).RemoveRoutineTSH(item)
             // file doesn't exist anymore despite error -- should be ok
             set sc = $$$OK
         } else {
             // Item still exists and was not deleted -- bad
-            set sc = e.AsStatus()
-            do e.Log() 
+            set sc = err.AsStatus()
+            do err.Log()
         }
     }
     return sc

cls/SourceControl/Git/Utils.cls

@@ -1469,7 +1469,7 @@ ClassMethod ImportItem(InternalName As %String, force As %Boolean = 0, verbose A
                 write !, InternalName," has been imported from ", filename
             }
         } else {
-            write !, "ERROR importing" ,InternalName, !
+            write !, "ERROR importing " ,InternalName, !
             do $system.Status.DisplayError(sc)
         }
     } elseif verbose {