Get-Service Ignore common errors #24245

jborean93 · 2024-09-04T21:23:33Z

PR Summary

Ignore common errors when attempting to retrieve some properties for a service like the Description, UserName, StartupType. These properties are not critical for using a service object or to retrieve other information like the name and whether a service exists or not and by emitting an error record it could fail a script if it did not expect an error.

Instead if the service failed to retrieve the property due to permissions or other environmental problems, it will set the affected properties to $null.

PR Context

The more common cause was doing Get-Service to enumerate all services or to get a particular service where the description points to a dll to extract the MUI entry. If that dll didn't exist or was set to an invalid index it would fail to get the description and then fail with a generic permission error message from PowerShell. Another less common problem was if the caller didn't have the SERVICE_QUERY_CONFIG access to retrieve the extra info requested.

As per the conversation in #10371 (comment) the WG choice was to not emit an error and set the properties to $null.

Fixes: #10371

PR Checklist

jborean93 · 2024-09-04T21:34:05Z

I’m not the biggest fan of throwing away errors as it makes it harder for people to figure out why a value is not set or not what they expect. I understand using an error is not ideal and a warning might be too noisy as well but could we potentially emit the errors as:

a verbose/debug message, or
an information record with a specific tag

The latter option could potentially turn into a common pattern used by other cmdlets for similar scenarios.

iSazonov

Instead of adding verbose/debug messages (which may be in too many numbers - several for each service, i.e. hundreds in total), we could simply add to the documentation that if the property is empty, then the reason is most likely the lack of rights.

jborean93 · 2024-09-05T07:00:47Z

While I do agree that using the error or even warning stream will be too noisy for scenarios like this I do think that errors should not be ignored and there should be a way to view them for troubleshooting purposes. For example without actually stepping through the code it’s not apparent if the properties are empty due to a permission issue, missing MUI/environmental setting, or even incorrect code calls. Being able to give end users a way to trace that info to provide in issue reports would be quite invaluable, at least from a triage perspective.

I see that as the whole point of the debug or verbose streams. Especially since debug is no longer inquire by default I think we should be taking advantage of them to log more info. Even if we wanted to keep that clear for backwards compatibility I think there is some utility on using the information stream and a common tag like PSTRACE to log such events.

I don’t think that needs to be answered here but thought I should bring it up.

daxian-dbw · 2024-09-09T18:44:18Z

src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs

+
+            // As these are optional values, a failure due to permissions or
+            // other problem is ignored and the properties are set to null.
+            bool isDelayedAutoStart = false;


Does it make sense to have the default value for isDelayedAutoStart to be null too? In case we failed to retrieve the actual value for this property, we probably should not use false as the default value.

I can make it nullable, it just means that we would need to decide how GetServiceStartupType treats the value as null. We could either

Treated as false

Treated the whole start type as invalid (or when Automatic)

The former is simpler but it may not be true if the service is truly automatic but delayed and we couldn't get the correct result.

https://github.com/PowerShell/PowerShell/blob/27e8cb3c9212d1d3e9a14198cbe822e7f7aec9dc/src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs#L2915-L2931

Given that startupType depends on the real value of autostartInfo.fDelayedAutostart, if we cannot get the real value of isDelayedAutoStart I think we may have to treat the startupType as invalid.

Has been updated to keep it as InvalidValue (what it was if the config couldn't be retrieved) if we couldn't get the automatic delayed start value.

isDelayedAutoStart is not updated to have a null value when it cannot be retrieved.

src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs

daxian-dbw · 2024-09-09T19:25:55Z

I see that as the whole point of the debug or verbose streams. Especially since debug is no longer inquire by default I think we should be taking advantage of them to log more info. Even if we wanted to keep that clear for backwards compatibility I think there is some utility on using the information stream and a common tag like PSTRACE to log such events.

I agree we would want to capture the error messages in another stream. Would the debug/verbose stream be better as they are displayed with highlighted color? Information records are displayed in plain text color by default I guess. Also, we may not want to use 'PSTRACE' as the tag if going with the information record, because we have Disable/Enable-PSTrace commands from PSDiagnostics module and a user may be confused about what it is for.

Can you please open an issue for this?

jborean93 · 2024-09-10T18:39:01Z

Can you please open an issue for this?

If I get time to do it I will but if I am being honest I mostly avoid opening issues here because they either don't get seen or do months ahead in time.

SteveL-MSFT · 2024-09-18T16:12:44Z

@PowerShell/wg-powershell-cmdlets reviewed this and agree with the new behavior.

Ignore common errors when attempting to retrieve some properties for a service like the Description, UserName, StartupType. These properties are not critical for using a service object or to retrieve other information like the name and whether a service exists or not and by emitting an error record it could fail a script if it did not expect an error. Instead if the service failed to retrieve the property due to permissions or other environmental problems, it will set the affected properties to $null.

iSazonov · 2024-10-04T04:25:10Z

@daxian-dbw Can you take another look?

daxian-dbw

@jborean93 Sorry for my long delay :(. I'm happy to take over this PR if you are no longer interested in making changes.

src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs

daxian-dbw · 2025-04-01T01:29:08Z

src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs

+
+            // As these are optional values, a failure due to permissions or
+            // other problem is ignored and the properties are set to null.
+            bool isDelayedAutoStart = false;


isDelayedAutoStart is not updated to have a null value when it cannot be retrieved.

src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs

daxian-dbw

LGTM!

jborean93 · 2025-04-21T21:36:21Z

Thanks for the review and merge @daxian-dbw

…ical properties for a service (PowerShell#24245)

eabase · 2025-06-30T12:10:11Z

Still an issue!
#20188

jborean93 · 2025-06-30T15:54:03Z

@daxian-dbw can you add this to the 7.4 and 7.5 backport process? I think it’s a bugfix that should meet the backport requirements so people don’t have to wait for 7.6.

eabase · 2025-07-01T10:48:33Z

@jborean93

I'm not a Windows OS coder, so it's not clear to me if this will fix the errors, or just hide them? I.e. will Get-Service truly show all the kernel drivers?

jborean93 · 2025-07-01T18:35:07Z

It'll hide the errors like Windows PowerShell did when retrieving service info for services that cannot do things like resolve the resource names. It does not change any logic when it comes to drivers, they still need to be queried manually.

So after this change

Get-Service | Where-Object {$_.ServiceType -eq "KernelDriver"}

Will no longer show the error records just like it was in Windows PowerShell 5.1 but it will not show the kernel drivers as Get-Service only returns those service types if you explicitly request them by name.

jborean93 · 2025-07-01T18:39:39Z

See this screenshot, top is 7.5.2, middle is this repo built with all the current changes (including this PR fix), and the bottom is 5.1

All 3 do not enumerate KernelDriver service types if you just did Get-Service. But now instead of showing the unrelated error records for unrelated services it doesn't do that anymore bringing the behaviour back to how 5.1 works.

jborean93 requested review from daxian-dbw and adityapatwardhan as code owners September 4, 2024 21:23

jborean93 mentioned this pull request Sep 4, 2024

Some drivers throw an exception when fetched with Get-Service, but still return the object anyway #10371

Closed

iSazonov previously approved these changes Sep 5, 2024

View reviewed changes

iSazonov added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Sep 5, 2024

daxian-dbw reviewed Sep 9, 2024

View reviewed changes

src/Microsoft.PowerShell.Commands.Management/commands/management/Service.cs Show resolved Hide resolved

jborean93 force-pushed the service-error branch from 27e8cb3 to 68133f5 Compare September 10, 2024 18:33

kilasuit mentioned this pull request Sep 10, 2024

Add verbose message to Get-Service when certain properties cannot be returned #24272

Open

microsoft-github-policy-service bot added the Review - Needed The PR is being reviewed label Sep 17, 2024

SteveL-MSFT added WG-Cmdlets general cmdlet issues WG-NeedsReview Needs a review by the labeled Working Group labels Sep 18, 2024

SteveL-MSFT added WG-Reviewed A Working Group has reviewed this and made a recommendation and removed WG-NeedsReview Needs a review by the labeled Working Group labels Sep 18, 2024

microsoft-github-policy-service bot removed the Review - Needed The PR is being reviewed label Sep 18, 2024

microsoft-github-policy-service bot added the Review - Needed The PR is being reviewed label Sep 26, 2024

jborean93 force-pushed the service-error branch from 68133f5 to 3a5b829 Compare October 3, 2024 20:29

jborean93 force-pushed the service-error branch from 3a5b829 to 2687612 Compare October 3, 2024 21:58

microsoft-github-policy-service bot removed the Review - Needed The PR is being reviewed label Oct 4, 2024

microsoft-github-policy-service bot added the Review - Needed The PR is being reviewed label Oct 11, 2024

iSazonov self-requested a review February 20, 2025 04:43

Use PSNoteProperty instead to make the added ETS properties obvious

bbd16f7

daxian-dbw requested changes Apr 1, 2025

View reviewed changes

microsoft-github-policy-service bot added Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept and removed Review - Needed The PR is being reviewed labels Apr 1, 2025

daxian-dbw added this to PowerShell Team Reviews/Investigations Apr 1, 2025

daxian-dbw moved this to PR In Progress in PowerShell Team Reviews/Investigations Apr 1, 2025

daxian-dbw self-assigned this Apr 1, 2025

jborean93 dismissed iSazonov’s stale review via bbd16f7 April 1, 2025 03:46

microsoft-github-policy-service bot removed the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Apr 1, 2025

Make the delayed auto start nullable in case we can't retrieve it

9764c58

microsoft-github-policy-service bot added the Review - Needed The PR is being reviewed label Apr 9, 2025

daxian-dbw approved these changes Apr 21, 2025

View reviewed changes

daxian-dbw merged commit 352fd38 into PowerShell:master Apr 21, 2025
35 checks passed

github-project-automation bot moved this from PR In Progress to Done in PowerShell Team Reviews/Investigations Apr 21, 2025

daxian-dbw removed the Review - Needed The PR is being reviewed label Apr 21, 2025

jborean93 deleted the service-error branch April 21, 2025 21:20

Sysoiev-Yurii pushed a commit to Sysoiev-Yurii/PowerShell that referenced this pull request May 12, 2025

Update Get-Service to ignore common errors when retrieving non-crit…

edf74f2

…ical properties for a service (PowerShell#24245)

Get-Service Ignore common errors #24245

Get-Service Ignore common errors #24245

Uh oh!

Conversation

jborean93 commented Sep 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

PR Context

PR Checklist

Uh oh!

jborean93 commented Sep 4, 2024

Uh oh!

iSazonov left a comment

Choose a reason for hiding this comment

Uh oh!

jborean93 commented Sep 5, 2024

Uh oh!

daxian-dbw Sep 9, 2024

Choose a reason for hiding this comment

Uh oh!

jborean93 Sep 9, 2024

Choose a reason for hiding this comment

Uh oh!

daxian-dbw Sep 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jborean93 Sep 10, 2024

Choose a reason for hiding this comment

Uh oh!

daxian-dbw Apr 1, 2025

Choose a reason for hiding this comment

Uh oh!

jborean93 Apr 2, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

daxian-dbw commented Sep 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jborean93 commented Sep 10, 2024

Uh oh!

SteveL-MSFT commented Sep 18, 2024

Uh oh!

iSazonov commented Oct 4, 2024

Uh oh!

daxian-dbw left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

daxian-dbw Apr 1, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

daxian-dbw left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jborean93 commented Apr 21, 2025

Uh oh!

eabase commented Jun 30, 2025

Uh oh!

jborean93 commented Jun 30, 2025

Uh oh!

eabase commented Jul 1, 2025

Uh oh!

jborean93 commented Jul 1, 2025

Uh oh!

jborean93 commented Jul 1, 2025

Uh oh!

Uh oh!

jborean93 commented Sep 4, 2024 •

edited

Loading

daxian-dbw Sep 9, 2024 •

edited

Loading

daxian-dbw commented Sep 9, 2024 •

edited

Loading