Extension Vault Feature Request: Expose PSCmdlet so that errors can be written at the user stacktrace level

[JustinGrote]

Currently, if a secretvault emits a nonterminating error, it is emitted with the stacktrace of the inner module extension, which is very confusing to the user in order to determine where their error is especially in conciseview.

Requested Solution

#93 (comment)

Then extension module author can opt to emit errors at where the secretmanagement command was called in their script, rather than the low-level extension vault location.

[PaulHigin]

This is by design. Test-SecretVault writes non-terminating errors for any problems the vault has. A terminating error means that Test-SecretVault itself cannot run successfully. PowerShell has a history of wrapping the terminating exception in an outer exception that let's the user know the cmdlet failed to run. So this is a common pattern.

[JustinGrote]

@PaulHigin so is the recommendation to emit non-terminating errors? I'm a bit concerned about the "wall of text" especially because even with ConciseView, Write-Error includes the path to the source part of the extension that would be intimidating to users. I've been using Write-Warning for this reason but obviously that's not ideal.

Is there a way in a script to write a nonterminating error without including the scriptblock so it has a "clean" conciseview?

Basically:

vs

[PaulHigin]

Yes, the pattern is to emit non-terminating errors that describe why the vault won't currently work. Terminating errors should only be thrown if the 'Test-SecretVault' failed to run and cannot provide diagnostic information, and should be rare (internal error). Hopefully the error message will be descriptive enough ('cannot connect', 'access denied for user x', etc.), but as always with PowerShell, a user can dig deeper into the error message/exception.

[JustinGrote]

@PaulHigin That is reasonable though I still worry about "wall of text" for script errors as shown above (I did throw but same happens with Write-Error). binary modules won't do this because there's no scriptstacktrace attached to the exception.

@SteveL-MSFT any input on how to create a nonterminating powershell error but not have the scriptstacktrace? (Or at least modify it so that the stacktrace shows the "external" Get-Secret call rather than the internal vault extension line where it was emitted?)

[JustinGrote]

@PaulHigin after a discussion with @SeeminglyScience, he suggested you could expose the outer PSCmdlet of the caller scope to the extension module, maybe as a variable or as a parameter e.g. $OuterPSCmdlet, so that the extension module could call the .WriteError() method on that and have the error emitted at the caller scope level, so the stacktrace above would show where the user originally called Get-Secret (or whatever) and not the inner extension details.

Is this a reasonable thing to expose? It would still flow with the plan and I can still throw a follow-on terminating error that can be swallowed by your command, but I can also emit an error that is more user friendly and points them to their problem so they can troubleshoot much easier. Examples being a keepass database missing the keyfile, etc.

[JustinGrote]

Also as an example, see this issue where the user had no idea what the problem was, but if he would have looked at $error[0].exception.innerexception.message he would have found the more detailed error I submitted. I think it's unreasonable to expect the average user to have this level of knowledge.
JustinGrote/SecretManagement.KeePass#17 (comment)

[PaulHigin]

Sorry, but I am not understanding your scenario. Why do you need to throw an exception during tests instead of writing cmdlet errors?

[JustinGrote]

@PaulHigin I re-read your specifications in the README and changed everything to non-terminating. This works OK however because of the way you call the extension, there is no $PSCmdlet to call in order to show the user where in their script the error was surfaced, it always shows my internal code.

I'll change the subject of the issue to more accurately reflect the ask in
#93 (comment)

[PaulHigin]

The point of Test-SecretVault is to validate an extension vault functionality and provide users with actionable information. It is not intended to provide diagnostic information and stack traces of user script. If everything is fine then the boolean true is returned. Otherwise false is returned and one or more error messages describing the problem(s) that the user can correct. My thinking was the error messages returned from Test-SecretVault would be something like:

Invalid credentials
Cannot connect to xxx service
Cannot read store file ...
Missing configuration data ...
etc.

[SeeminglyScience]

@JustinGrote is talking about the metadata attached to ErrorRecord's. Basically when full error view is on it'll show the error line as the small anonymous script y'all create to call the extension function, rather than the caller's line where Test-SecretVault is called.

Example:

$oldErrorView = $global:ErrorView
$global:ErrorView = [System.Management.Automation.ErrorView]::NormalView
try {
    function Test-SecretVault {
        [CmdletBinding()]
        param([switch] $DontPass)
        end {
            if ($DontPass) {
                Test-ExtensionVault # less helpful for the user
                return
            }

            Test-ExtensionVault $PSCmdlet
        }
    }

    function Test-ExtensionVault {
        [CmdletBinding()]
        param(
            [System.Management.Automation.PSCmdlet] $Cmdlet
        )
        end {
            $Cmdlet ??= $PSCmdlet
            $Cmdlet.WriteError(
                [System.Management.Automation.ErrorRecord]::new(
                    [Exception]::new('testing'),
                    'SomeId',
                    'NotSpecified',
                    $null))
        }
    }

    Test-SecretVault
    Test-SecretVault -DontPass

} finally {
    $global:ErrorView = $oldErrorView
}

Displays:

Test-SecretVault : testing
At line:33 char:5
+     Test-SecretVault
+     ~~~~~~~~~~~~~~~~
+ CategoryInfo          : NotSpecified: (:) [Test-SecretVault], Exception
+ FullyQualifiedErrorId : SomeId,Test-SecretVault
Test-ExtensionVault : testing
At line:9 char:17
+                 Test-ExtensionVault # less helpful for the user
+                 ~~~~~~~~~~~~~~~~~~~
+ CategoryInfo          : NotSpecified: (:) [Test-ExtensionVault], Exception
+ FullyQualifiedErrorId : SomeId,Test-ExtensionVault

[JustinGrote]

@SeeminglyScience is correct, and this happens even in conciseview, not just full view

[SeeminglyScience]

@SeeminglyScience is correct, and this happens even in conciseview, not just full view

Oh right, I forgot about the command name prefix until looking at the output and then didn't think to change it 😁

[PaulHigin]

Oh I see now. Thanks for the repro. I'll take a look.

[PaulHigin]

The last change to the SecretManagement design was to allow extension vault cmdlets to write directly to error/data streams, and not try to force writing to some common SecretManagement APIs. This gives vault commands much more freedom but it also means they have to put up with PowerShell data stream weirdness. If you don't like how error output is formatted then you can try using other data streams, like warning or verbose (Write-Verbose -Verbose 'Error message').
The important thing is to provide users with clear error message and instructions.

PS E:\> test-secretvault testlocalscript
Test-SecretVault:
Line |
  18 |              & $module "$ImplementingModuleName\$Command" @Params
     |              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | TestLocalScript vault Test Error 1
Test-SecretVault:
Line |
  18 |              & $module "$ImplementingModuleName\$Command" @Params
     |              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | System.InvalidOperationException: TestLocalScript vault Test Error 2
Test-SecretVault:
Line |
  18 |              & $module "$ImplementingModuleName\$Command" @Params
     |              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | TestLocalScript vault Test Error 3
False

PS E:\> test-secretvault testlocalscript
WARNING: TestLocalScript vault Test Error 1
WARNING: TestLocalScript vault Test Error 2
WARNING: TestLocalScript vault Test Error 3
False

PS E:\> Test-SecretVault testlocalscript
VERBOSE: TestLocalScript vault Test Error 1
VERBOSE: TestLocalScript vault Test Error 2
VERBOSE: TestLocalScript vault Test Error 3
False

[JustinGrote]

@PaulHigin I considered warning but the problem is that a user can't interrogate a warning with a stacktrace of where the issue occured in their code, further they can't try/catch a warning without some convoluted logic, hence why I have stuck with nonterminating errors.

I suppose I could trap my own errors and try to interrogate the stacktrace and report in the warning where their code failed, but now we're just basically going completely against standard powershell convention.

All we need is for the $PSCmdlet object for the "outer" command to be exposed to the extension as a variable, then we can use something like $SMCmdlet.WriteError to emit the error at user scope.

[SeeminglyScience]

@PaulHigin Enabling proper error handling only requires a few small changes:

SecretManagement/src/code/Utils.cs

Lines 255 to 271 in 30991fd

	param (
	[string] $ModulePath,
	[string] $ImplementingModuleName,
	[string] $Command,
	[hashtable] $Params
	)
	$verboseEnabled = $Params.AdditionalParameters.ContainsKey('Verbose') -and ($Params.AdditionalParameters['Verbose'] -eq $true)
	$module = Get-Module -Name ([System.IO.Path]::GetFileNameWithoutExtension($ImplementingModuleName)) -ErrorAction Ignore
	if ($null -eq $module) {
	$module = Import-Module -Name $ModulePath -PassThru
	}
	if ($null -eq $module) {
	return
	}
	Write-Verbose ""Invoking command $Command on module $ImplementingModuleName"" -Verbose:$verboseEnabled
	& $module ""$ImplementingModuleName\$Command"" @Params

             param (
                 [string] $ModulePath,
                 [string] $ImplementingModuleName,
                 [string] $Command,
-                [hashtable] $Params
+                [hashtable] $Params,
+                [System.Management.Automation.PSCmdlet] $Cmdlet
             )
             $verboseEnabled = $Params.AdditionalParameters.ContainsKey('Verbose') -and ($Params.AdditionalParameters['Verbose'] -eq $true)
             $module = Get-Module -Name ([System.IO.Path]::GetFileNameWithoutExtension($ImplementingModuleName)) -ErrorAction Ignore
             if ($null -eq $module) {
                 $module = Import-Module -Name $ModulePath -PassThru
             }
             if ($null -eq $module) {
                 return
             }
             Write-Verbose ""Invoking command $Command on module $ImplementingModuleName"" -Verbose:$verboseEnabled
-            & $module ""$ImplementingModuleName\$Command"" @Params
+            & $module {
+                # This variable will be accessible only to the command due to scoping.
+                $TopLevelSecretCommand = $args[0]
+                & ""$($args[1])\$($args[2])"" @args[3]
+            } $Cmdlet $ImplementingModuleName $Command $Params

---

And here:

SecretManagement/src/code/Utils.cs

Line 479 in 30991fd

args: new object[] { ModulePath, ModuleExtensionName, GetSecretCmd, parameters },

-                args: new object[] { ModulePath, ModuleExtensionName, GetSecretCmd, parameters },
+                args: new object[] { ModulePath, ModuleExtensionName, GetSecretCmd, parameters, cmdlet },

---

Probably a few other places and maybe a different variable name but that should give the idea.

[PaulHigin]

I don't think this is an issue for Test-SecretVault, since reported errors are not in the user script but about how the extension vault is mis-configured. A clear error/warning message should help the user understand how to mitigate.

But, I can see this as a general problem for extension vault command error reporting. It seems that the main problem is the error line being reported and is something @SteveL-MSFT is looking to fix.

[JustinGrote]

Realistically people are going to use Test-SecretVault as part of automatic provisioning scripts since the decision was made that Register-SecretVault won't validate by default, so knowing what line it occurs in their script without having to dive extensively into the stacktrace is helpful.

But agreed, the scope matters for all *-SecretVault commands, Test-SecretVault is just what we are using as an example.