dsccommunity / UpdateServicesDsc

This module contains community maintained DSC resources for deployment and configuration of Windows Server Update Services.
MIT License
31 stars 27 forks source link

UpdateServicesServer: Null value passed to New-InvalidResultException #66

Closed Sudman1 closed 2 years ago

Sudman1 commented 3 years ago

Details of the scenario you tried and the problem that is occurring

On line 663 in the Set-TargetResource function, the following call is made:

New-InvalidResultException -Message $errorMessage -ErrorRecord $_

However, unlike previous calls to similar commands (see line 416), this command is not inside a try/catch block. Therefore, the $_ variable is null and the function errors out without detailing the resource failure. Instead, it leads the user on a wild goose chase tracking down $null values. See #58 for an example.

Verbose logs showing the problem

Suggested solution to the issue

Proposals:

  1. Replace $_ with a meaningful value such as "Set-TargetResource has been run. However, the resource is not in the desired state. The following parameters are not set to the desired values: ..."
  2. Remove lines 660-664 altogether, as testing is in its own function

I prefer option 2, as the Set function has already performed its work and the state has already changed. The Unit and Integration tests should be providing feedback as to the effectiveness of the Set function, not the Set function itself. If, on subsequent execution of the configuration block, the test continues to fail and Set-TargetResource continues to be called, then an issue with the Set-TargetResource function should be filed and addressed.

Additionally, Issues #45 and #47 are related to apparent false negatives related to the Set-TargetResource calling Test-TargetResource

The DSC configuration that is used to reproduce the issue (as detailed as possible)

See #58

The operating system the target node is running

See #58

Version and build of PowerShell the target node is running

Version of the DSC module that was used

v1.2.1

Sudman1 commented 3 years ago

@johlju , this seems like an issue that needs your attention 😉

ericscheffler commented 3 years ago

I can confirm that option 2 above seems to run without issues and completes the desired state changes given my configuration in #58; I commented these lines out on a local copy of the resource and ran it without issues.