Log shipping - Let helper failures reach the caller instead of escaping it

private/functions/New-DbaLogShippingPrimaryDatabase.ps1

@@ -368,7 +368,10 @@ function New-DbaLogShippingPrimaryDatabase {
             }
         } catch {
             Write-Message -Message "$($_.Exception.InnerException.InnerException.InnerException.InnerException.Message)" -Level Warning
-            Stop-Function -Message "Error executing the query.`n$($_.Exception.Message)`n$($Query)" -ErrorRecord $_ -Target $SqlInstance -Continue
+            # No -Continue here: no loop encloses this call, so the continue would escape into the
+            # caller and bypass the catch that Invoke-DbaDbLogShipping wraps around this function.
+            Stop-Function -Message "Error executing the query.`n$($_.Exception.Message)`n$($Query)" -ErrorRecord $_ -Target $SqlInstance
+            return
         }
     }
 

private/functions/New-DbaLogShippingPrimarySecondary.ps1

@@ -85,12 +85,16 @@ function New-DbaLogShippingPrimarySecondary {
 
     # Check if the database is present on the source sql server
     if ($serverPrimary.Databases.Name -notcontains $PrimaryDatabase) {
-        Stop-Function -Message "Database $PrimaryDatabase is not available on instance $SqlInstance" -Target $SqlInstance -Continue
+        # No -Continue on any stop in this function: no loop encloses them, so the continue would
+        # escape into the caller and bypass the catch that Invoke-DbaDbLogShipping wraps around it.
+        Stop-Function -Message "Database $PrimaryDatabase is not available on instance $SqlInstance" -Target $SqlInstance
+        return
     }
 
     # Check if the database is present on the destination sql server
     if ($serverSecondary.Databases.Name -notcontains $SecondaryDatabase) {
-        Stop-Function -Message "Database $SecondaryDatabase is not available on instance $SecondaryServer" -Target $SecondaryServer -Continue
+        Stop-Function -Message "Database $SecondaryDatabase is not available on instance $SecondaryServer" -Target $SecondaryServer
+        return
     }
 
     $Query = "SELECT primary_database FROM msdb.dbo.log_shipping_primary_databases WHERE primary_database = '$PrimaryDatabase'"
@@ -99,10 +103,12 @@ function New-DbaLogShippingPrimarySecondary {
         Write-Message -Message "Executing query:`n$Query" -Level Verbose
         $Result = $serverPrimary.Query($Query)
         if ($Result.Count -eq 0 -or $Result[0] -ne $PrimaryDatabase) {
-            Stop-Function -Message "Database $PrimaryDatabase does not exist as log shipping primary.`nPlease run New-DbaLogShippingPrimaryDatabase first."  -ErrorRecord $_ -Target $SqlInstance -Continue
+            Stop-Function -Message "Database $PrimaryDatabase does not exist as log shipping primary.`nPlease run New-DbaLogShippingPrimaryDatabase first."  -ErrorRecord $_ -Target $SqlInstance
+            return
         }
     } catch {
-        Stop-Function -Message "Error executing the query.`n$($_.Exception.Message)`n$Query" -ErrorRecord $_ -Target $SqlInstance -Continue
+        Stop-Function -Message "Error executing the query.`n$($_.Exception.Message)`n$Query" -ErrorRecord $_ -Target $SqlInstance
+        return
     }
 
     # Set the query for the log shipping primary and secondary
@@ -125,7 +131,8 @@ function New-DbaLogShippingPrimarySecondary {
             $serverPrimary.Query($Query)
         } catch {
             Write-Message -Message "$($_.Exception.InnerException.InnerException.InnerException.InnerException.Message)" -Level Warning
-            Stop-Function -Message "Error executing the query.`n$($_.Exception.Message)`n$Query" -ErrorRecord $_ -Target $SqlInstance -Continue
+            Stop-Function -Message "Error executing the query.`n$($_.Exception.Message)`n$Query" -ErrorRecord $_ -Target $SqlInstance
+            return
         }
     }
 

private/functions/New-DbaLogShippingSecondaryDatabase.ps1

@@ -156,12 +156,16 @@ function New-DbaLogShippingSecondaryDatabase {
 
     # Check if the database is present on the primary sql server
     if ($ServerPrimary.Databases.Name -notcontains $PrimaryDatabase) {
-        Stop-Function -Message "Database $PrimaryDatabase is not available on instance $PrimaryServer" -Target $PrimaryServer -Continue
+        # No -Continue on any stop in this function: no loop encloses them, so the continue would
+        # escape into the caller and bypass the catch that Invoke-DbaDbLogShipping wraps around it.
+        Stop-Function -Message "Database $PrimaryDatabase is not available on instance $PrimaryServer" -Target $PrimaryServer
+        return
     }
 
     # Check if the database is present on the primary sql server
     if ($ServerSecondary.Databases.Name -notcontains $SecondaryDatabase) {
-        Stop-Function -Message "Database $SecondaryDatabase is not available on instance $ServerSecondary" -Target $SqlInstance -Continue
+        Stop-Function -Message "Database $SecondaryDatabase is not available on instance $ServerSecondary" -Target $SqlInstance
+        return
     }
 
     # Check the restore mode
@@ -194,7 +198,8 @@ function New-DbaLogShippingSecondaryDatabase {
             [int]$DisconnectUsers = 0
             Write-Message -Message "Illegal combination of database restore mode $RestoreMode and disconnect users $DisconnectUsers. Setting it to $DisconnectUsers." -Level Warning
         } else {
-            Stop-Function -Message "Illegal combination of database restore mode $RestoreMode and disconnect users $DisconnectUsers." -Target $SqlInstance -Continue
+            Stop-Function -Message "Illegal combination of database restore mode $RestoreMode and disconnect users $DisconnectUsers." -Target $SqlInstance
+            return
         }
     }
 
@@ -311,7 +316,8 @@ function New-DbaLogShippingSecondaryDatabase {
             }
         } catch {
             Write-Message -Message "$($_.Exception.InnerException.InnerException.InnerException.InnerException.Message)" -Level Warning
-            Stop-Function -Message "Error executing the query.`n$($_.Exception.Message)`n$Query"  -ErrorRecord $_ -Target $SqlInstance -Continue
+            Stop-Function -Message "Error executing the query.`n$($_.Exception.Message)`n$Query"  -ErrorRecord $_ -Target $SqlInstance
+            return
         }
     }
 

private/functions/New-DbaLogShippingSecondaryPrimary.ps1

@@ -243,7 +243,10 @@ function New-DbaLogShippingSecondaryPrimary {
 
         # Check the MonitorServerSecurityMode if it's SQL Server authentication
         if ($MonitorServerSecurityMode -eq 0 -and -not $MonitorCredential) {
-            Stop-Function -Message "The MonitorServerCredential cannot be empty when using SQL Server authentication." -Target $SqlInstance -Continue
+            # No -Continue on any stop in this function: no loop encloses them, so the continue would
+            # escape into the caller (before the return below ever ran) and bypass the catch that
+            # Invoke-DbaDbLogShipping wraps around it.
+            Stop-Function -Message "The MonitorServerCredential cannot be empty when using SQL Server authentication." -Target $SqlInstance
             return
         } elseif ($MonitorServerSecurityMode -eq 0 -and $MonitorCredential) {
             # Get the username and password from the credential
@@ -252,14 +255,14 @@ function New-DbaLogShippingSecondaryPrimary {
 
             # Check if the user is in the database
             if ($ServerSecondary.Databases['master'].Users.Name -notcontains $MonitorLogin) {
-                Stop-Function -Message "User $MonitorLogin for monitor login must be in the master database." -Target $SqlInstance -Continue
+                Stop-Function -Message "User $MonitorLogin for monitor login must be in the master database." -Target $SqlInstance
                 return
             }
         }
 
         # Check if the database is present on the primary sql server
         if ($ServerPrimary.Databases.Name -notcontains $PrimaryDatabase) {
-            Stop-Function -Message "Database $PrimaryDatabase is not available on instance $PrimaryServer" -Target $PrimaryServer -Continue
+            Stop-Function -Message "Database $PrimaryDatabase is not available on instance $PrimaryServer" -Target $PrimaryServer
             return
         }
     }
@@ -330,7 +333,8 @@ function New-DbaLogShippingSecondaryPrimary {
             $ServerSecondary.Query($Query)
         } catch {
             Write-Message -Message "$($_.Exception.InnerException.InnerException.InnerException.InnerException.Message)" -Level Warning
-            Stop-Function -Message "Error executing the query.`n$($_.Exception.Message)"  -ErrorRecord $_ -Target $SqlInstance -Continue
+            Stop-Function -Message "Error executing the query.`n$($_.Exception.Message)"  -ErrorRecord $_ -Target $SqlInstance
+            return
         }
     }
 

public/Invoke-DbaDbLogShipping.ps1

@@ -710,7 +710,10 @@ function Invoke-DbaDbLogShipping {
                 $DatabaseCollection = $SourceServer.Databases | Where-Object { $_.Name -in $Database }
             }
         } else {
-            Stop-Function -Message "Please supply a database to set up log shipping for" -Target $SourceSqlInstance -Continue
+            # No -Continue here: no loop encloses this call, so the continue would escape the
+            # command and eat an iteration of whatever loop the caller runs in.
+            Stop-Function -Message "Please supply a database to set up log shipping for" -Target $SourceSqlInstance
+            return
         }
 
         $existingPrimaryConfigurations = @{ }
@@ -1867,6 +1870,9 @@ WHERE pd.primary_database = N'$escapedPrimaryDatabase'
                                 MonitorCredential         = $PrimaryMonitorCredential
                                 ThresholdAlertEnabled     = $PrimaryThresholdAlertEnabled
                                 Force                     = $Force
+                                # A failure inside the helper has to throw so that the catch below
+                                # marks the setup as failed and the later phases are skipped.
+                                EnableException           = $true
                             }
 
                             # Add Azure credential if provided (for storage account key authentication)
@@ -1923,12 +1929,16 @@ WHERE pd.primary_database = N'$escapedPrimaryDatabase'
                                 SecondaryDatabase      = $SecondaryDatabase
                                 SecondaryServer        = $destInstance
                                 SecondarySqlCredential = $DestinationSqlCredential
+                                EnableException        = $true
                             }
                             New-DbaLogShippingPrimarySecondary @splatPrimarySecondary
                         } catch {
                             $setupResult = "Failed"
                             $comment = "Something went wrong setting up log shipping for primary instance"
-                            Stop-Function -Message "Something went wrong setting up log shipping for primary instance" -ErrorRecord $_ -Target $SourceSqlInstance -Continue
+                            # No -Continue here: it would advance the database loop past the status
+                            # object at the end of the iteration, so a failure would return nothing.
+                            # $setupResult already suppresses the later phases.
+                            Stop-Function -Message "Something went wrong setting up log shipping for primary instance" -ErrorRecord $_ -Target $SourceSqlInstance
                         }
                     }
                 }
@@ -1957,6 +1967,7 @@ WHERE pd.primary_database = N'$escapedPrimaryDatabase'
                                 PrimaryDatabase            = $($db.Name)
                                 RestoreJob                 = $DatabaseRestoreJob
                                 Force                      = $Force
+                                EnableException            = $true
                             }
 
                             # Add Azure credential if provided (for storage account key authentication)
@@ -2036,6 +2047,7 @@ WHERE pd.primary_database = N'$escapedPrimaryDatabase'
                                 MonitorServer             = $SecondaryMonitorServer
                                 MonitorServerSecurityMode = $SecondaryMonitorServerSecurityMode
                                 MonitorCredential         = $SecondaryMonitorCredential
+                                EnableException           = $true
                             }
                             New-DbaLogShippingSecondaryDatabase @splatSecondaryDatabase
 
@@ -2059,7 +2071,9 @@ WHERE pd.primary_database = N'$escapedPrimaryDatabase'
                         } catch {
                             $setupResult = "Failed"
                             $comment = "Something went wrong setting up log shipping for secondary instance"
-                            Stop-Function -Message "Something went wrong setting up log shipping for secondary instance.`n$($_.Exception.Message)" -ErrorRecord $_ -Target $destInstance -Continue
+                            # No -Continue here: it would advance the database loop past the status
+                            # object at the end of the iteration, so a failure would return nothing.
+                            Stop-Function -Message "Something went wrong setting up log shipping for secondary instance.`n$($_.Exception.Message)" -ErrorRecord $_ -Target $destInstance
                         }
                     }
                 }

tests/Invoke-DbaDbLogShipping.Tests.ps1

@@ -101,3 +101,96 @@ Describe $CommandName -Tag UnitTests {
         }
     }
 }
+
+Describe $CommandName -Tag IntegrationTests {
+    Context "When no database name is supplied" {
+        It "Warns without eating an iteration of the caller's loop" {
+            # The empty-database guard used to run Stop-Function -Continue without an enclosing loop -
+            # the continue escaped the command and consumed an iteration of this very loop, so the
+            # counter fell short. The log shipping helper functions carried the same defect and their
+            # escapes bypassed this command's own catch blocks (#10638).
+            $loopCount = 0
+            foreach ($i in 1..3) {
+                # The share is never touched: it only has to look like a UNC path so that the guards
+                # before the database check pass, and IgnoreFileChecks skips the reachability test.
+                # On CI $TestConfig.Temp is a local path and would trip the UNC form check instead.
+                $splatEmptyDatabase = @{
+                    SourceSqlInstance      = $TestConfig.InstanceHadr
+                    DestinationSqlInstance = $TestConfig.InstanceHadr
+                    Database               = ""
+                    SharedPath             = "\\dbatoolsci\notashare"
+                    IgnoreFileChecks       = $true
+                    WarningAction          = "SilentlyContinue"
+                }
+                $null = Invoke-DbaDbLogShipping @splatEmptyDatabase
+                $loopCount++
+            }
+            $loopCount | Should -Be 3
+            $WarnVar | Should -BeLike "*Please supply a database*"
+        }
+    }
+
+    Context "When a helper fails after the setup phases" {
+        BeforeAll {
+            $PSDefaultParameterValues["*-Dba*:EnableException"] = $true
+
+            $primaryDatabase = "dbatoolsci_lsfail_$(Get-Random)"
+            $secondaryDatabase = "$($primaryDatabase)_ls"
+            $sharedPath = Join-Path -Path $TestConfig.Temp -ChildPath "dbatoolsci_lsfail_$(Get-Random)"
+            $null = New-Item -Path $sharedPath -ItemType Directory
+            # The copy destination has to exist and be passed explicitly: for a missing default
+            # folder the command falls into a raw PromptForChoice, which a non-interactive session
+            # cannot answer.
+            $copyDestinationFolder = Join-Path -Path $sharedPath -ChildPath "copy"
+            $null = New-Item -Path $copyDestinationFolder -ItemType Directory
+
+            $null = New-DbaDatabase -SqlInstance $TestConfig.InstanceHadr -Name $primaryDatabase -RecoveryModel Full
+            # The full backup is taken here and passed via UseExistingFullBackup: letting the
+            # command generate its own backup fails on this lab, because SQL Server cannot verify
+            # the freshly created per-database subfolder below the share.
+            $null = Backup-DbaDatabase -SqlInstance $TestConfig.InstanceHadr -Database $primaryDatabase -Path $sharedPath -Type Full
+
+            # The helper is the first call inside the primary region try block, so failing it
+            # exercises the catch without creating log shipping metadata or agent jobs. Everything
+            # before the helper - backup generation, restore of the secondary - runs for real.
+            Mock -CommandName New-DbaLogShippingPrimaryDatabase -ModuleName dbatools -MockWith {
+                throw "Simulated helper failure"
+            }
+
+            $PSDefaultParameterValues.Remove("*-Dba*:EnableException")
+        }
+
+        AfterAll {
+            $PSDefaultParameterValues["*-Dba*:EnableException"] = $true
+
+            $null = Remove-DbaDatabase -SqlInstance $TestConfig.InstanceHadr -Database $primaryDatabase, $secondaryDatabase
+            Remove-Item -Path $sharedPath -Recurse -Force -ErrorAction SilentlyContinue
+
+            $PSDefaultParameterValues.Remove("*-Dba*:EnableException")
+        }
+
+        It "Emits the failed status object instead of skipping it" {
+            # Before the fix the catch around the primary helpers ran Stop-Function -Continue,
+            # which advanced the per-database loop past the status object at the end of the
+            # iteration: a helper failure returned nothing at all.
+            $splatLogShipping = @{
+                SourceSqlInstance       = $TestConfig.InstanceHadr
+                DestinationSqlInstance  = $TestConfig.InstanceHadr
+                Database                = $primaryDatabase
+                SharedPath              = $sharedPath
+                CopyDestinationFolder   = $copyDestinationFolder
+                UseExistingFullBackup   = $true
+                SecondaryDatabaseSuffix = "_ls"
+                WarningAction           = "SilentlyContinue"
+            }
+            $results = Invoke-DbaDbLogShipping @splatLogShipping
+
+            $results | Should -Not -BeNullOrEmpty
+            $results.Result | Should -Be "Failed"
+            $results.Comment | Should -Be "Something went wrong setting up log shipping for primary instance"
+            $results.PrimaryDatabase | Should -Be $primaryDatabase
+            $WarnVar | Should -BeLike "*primary instance*"
+            Should -Invoke -CommandName New-DbaLogShippingPrimaryDatabase -ModuleName dbatools -Times 1 -Exactly
+        }
+    }
+}