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

[andreasjordan]

Summary

Seventh fix from the #10638 inventory, batched as one PR because fifteen sites share a single mechanism: the four New-DbaLogShipping* helper functions carried fourteen Stop-Function -Continue calls without any enclosing loop, plus one in Invoke-DbaDbLogShipping itself.

The compound failure mode: when a helper failed without EnableException, the continue unwound out of the helper, bypassed the try/catch that Invoke-DbaDbLogShipping wraps around every helper call (flow control is not an exception), and consumed an iteration of its per-database loop. The database vanished from the run - $setupResult never became Failed, the secondary phases were skipped silently, and no status object was emitted.

Fix

Two changes, one mechanism:

• The helpers stop and return at all fourteen sites - they warn, or throw under EnableException.
• Invoke-DbaDbLogShipping passes EnableException = $true into all four helper splats, so a helper failure throws into the catch that was always meant to receive it: $setupResult = "Failed", later phases skipped as designed, proper status emitted. Its own empty-database guard also stops and returns now.

Tests

The test file gains its first integration test - and with it its first instance reference, moving it into the HADR lane; until now it ran in no scenario-scoped lane at all (see #10629). The loop-counter pattern over an empty -Database: red on the unfixed code ("Expected 3, but got 0"), green on the fix, verified via the lab harness against SQL04\SQL2025.

References #10638

created by Claude and reviewed by Andreas Jordan

🤖 Generated with Claude Code

[potatoqualitee]

The new EnableException = $true propagation correctly routes helper failures into the caller's catches, but those catches still call Stop-Function ... -Continue (public/Invoke-DbaDbLogShipping.ps1, primary catch around line 1938 and secondary catch around line 2071). Both are inside the per-database foreach, so -Continue advances that loop before the status object around line 2079 is emitted. A helper failure therefore still produces no Result = "Failed" object, contrary to this PR's central behavior claim. Please remove the catch-level -Continue so $setupResult can suppress later phases and reach the result object, and add a regression test that drives a helper failure through the catch and asserts the failed status object is returned.

[andreasjordan]

Fixed in 76948da - the review is right, and the catch-level -Continue defeated exactly the mechanism this PR claimed to restore.

The change: both catch sites (primary region and secondary region) drop -Continue; a comment at each states why it is forbidden there. $setupResult already suppresses the later phases, so control now falls through to the status object at the end of the database iteration: without -EnableException a helper failure warns and returns Result = "Failed" with the region's comment, with it the throw is unchanged.

The regression test runs the real path on the HADR instance - create the database, take the full backup, let the command restore the secondary - and mocks only New-DbaLogShippingPrimaryDatabase (the first call inside the primary try, so nothing is half-created on the instance) to throw. It asserts the Failed status object, its comment, and the warning. Red on the unfixed code with exactly the predicted symptom ("Expected a value, but got $null"), green on the fix: 3/3 via the lab harness, no warnings, lab left clean.

Two adjacent defects documented while working this, deliberately not changed here:

• The same emission-skip exists at every validation site of the loop: 23 sites (lines 1232, 1280, 1286, 1295, 1349, 1363, 1394, 1471, 1493, 1509, 1521, 1541, 1547, 1632, 1710, 1728, 1734, 1750, 1756, 1839 and the connection catch at 1058) set $setupResult = "Failed" and then -Continue past the status object. Fixing those changes the command's output contract for every validation failure - callers that today get nothing would start receiving Failed objects - so it needs a maintainer decision rather than riding along in this review response.
• When the default copy destination folder is missing and -Force is not set, lines 1109 and 1413 run a raw $host.ui.PromptForChoice($title, ...) where $title is never assigned - in a non-interactive session this null-refs instead of prompting. The new test passes -CopyDestinationFolder explicitly to stay off that path; the proper fix (ShouldContinue instead of the raw prompt) changes interactive behavior and deserves its own change.

created by Claude and reviewed by Andreas Jordan

[potatoqualitee]

The updated head resolves the prior blocking feedback; no material issues remain.