microsoft
All Resources: Handling error in Get-TargetResource
Description of the issue
Since we removed identity as a key, it is now possible to create a resource without the key id. When we retrieve a resource using the displayname path, the engine could retrieve multiple policies with the same name. We thought raising an error using throw when this case occur would resolve the issue however it doesn't: the current implementation will trigger the catch statement which returns $nullResult with effect to create a new policy from Set-TargetResource every time the dsc engine runs.
I tried to exit from the catch however it process continue to Test-TargetResource and throw an error because it doesn't return true of false
I thought of using a $errorResult but this would potentially imply to modify all resources....
What would be the best way to handle this case and ensure the dsc engine stop processing this resource when it contains a fatal error? @NikCharlebois @ykuijs @andikrueger
Microsoft 365 DSC Version
1.24.131.2
Which workloads are affected
other
The DSC configuration
No response
Verbose logs showing the problem
No response
Environment Information + PowerShell Version
No response
Quick search and found this https://learn.microsoft.com/en-us/powershell/scripting/learn/deep-dives/everything-about-exceptions?view=powershell-7.4#re-throwing-an-exception
instead of returning $nullResult in the catch we should re-throw the exception this way
catch
{
if ($_.Exception -like '*401*' -or $_.ErrorDetails.Message -like "*`"ErrorCode`":`"Forbidden`"*" -or `
$_.Exception -like "*Unable to perform redirect as Location Header is not set in response*")
{
if (Assert-M365DSCIsNonInteractiveShell)
{
Write-Host "`r`n $($Global:M365DSCEmojiYellowCircle) The current tenant is not registered for Intune."
}
}
else
{
New-M365DSCLogEntry -Message 'Error retrieving data:' `
-Exception $_ `
-Source $($MyInvocation.MyCommand.Source) `
-TenantId $TenantId `
-Credential $Credential
}
throw
}
but this will need to apply to all resources
I have looked into this issue. It indeed doesn't work as it is supposed to work. Just like we check for null, we should also check for more than one result. The issue with the catch block is an interesting one. The code now simply continues, fails and then hits the catch block.
One of the issues we ran into in the past is that exceptions were thrown in the Get method, which resulted in exports failing because of those exceptions. That is why we now have these nullResults build in.
Would it be an idea to:
- Update the Get method to return the nullResult when more than one policy is detected.
- Update the Set method to throw an exception when two policies are detected
- Maybe we can change the DisplayName property into something like "Multiple" to make sure the Set method can detect that there are multiple policies found.
What do you think of this idea?
I have implemented a different solution using $nullResult but clearing all authentication parameter ie assigning $null to each of them
catch
{
New-M365DSCLogEntry -Message 'Error retrieving data:' `
-Exception $_ `
-Source $($MyInvocation.MyCommand.Source) `
-TenantId $TenantId `
-Credential $Credential
$nullResult = Clear-M365DSCAuthenticationParameter -BoundParameters $nullResult
return $nullResult
}
During Test-TargetResource if an error, such as duplicated policy, is found the resource will be skipped raising an error
$CurrentValues = Get-TargetResource @PSBoundParameters
if (-not (Test-M365DSCAuthenticationParameter -BoundParameters $CurrentValues))
{
Write-Verbose "An error occured in Get-TargetResource, the policy {$displayName} will not be processed"
throw "An error occured in Get-TargetResource, the policy {$displayName} will not be processed. Refer to the event viewer logs for more information."
}
This method does not stop the dsc engine to process the next resource from the mof file
Are you ok with skipping faulty resource from a mof file or should we raise a fatal error and stop the processing because it contains an error?
for your last point @ykuijs I'm not really fan of modifying displayname to an array as it would modify the parameter definition. I think using $nullResult will have a lesser impact
Implemented the solution in IntuneappConfigurationPolicy in #4323 Let me know if that works for you will update the rest of this PR resources if you validate that change
Will have a look at the mentioned PR.
About the modification of the displayname: I didn't mean returning an array with all of the names, but the actual string "Multiple" (or something similar, to prevent issues if someone named their policy Multiple 😉).
Implemented the solution in IntuneappConfigurationPolicy in #4323 Let me know if that works for you will update the rest of this PR resources if you validate that change
This solution will work, but we also need to update the Export method to make sure the export will also work correctly.
Updated the export with raising an exception when Get-TargetResource returns a policy with no authentication parameter indicating an error