Skip to content

Fix PSES crash when you format an empty PS doc #685

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 13, 2018

Conversation

rkeithhill
Copy link
Contributor

Invoke-Formatter -ScriptDefinition '' - throws a terminating error:
ParameterBinderValidationException.

I suppose we could have added another catch statement to the InvokePowerShell method
in the AnalysisService but in general, we probably want to know about such bugs.

We could also update the extension to not send a format request for an empty doc
but I'm not sure it's worth the additional code change given this corner case scenario.
But I could be convinced otherwise.

This is a partial fix to vscode issue PowerShell/vscode-powershell#1346 (comment)

Invoke-Formatter -ScriptDefinition '' - throws a terminating error:
ParameterBinderValidationException.

I suppose we could have added another catch statement to the InvokePowerShell method
in the AnalysisService but in general, we probably want to know about such bugs.

Also, we could also update the extension to not send a format request for an empty doc
but I'm not sure it's worth the additional code change given corner case scenario.
But I could be convinced otherwise.
@rkeithhill rkeithhill changed the title Fix PSES crash that happens if you format an empty PS doc Fix PSES crash when format an empty PS doc Jun 11, 2018
@rkeithhill rkeithhill changed the title Fix PSES crash when format an empty PS doc Fix PSES crash when you format an empty PS doc Jun 11, 2018
Copy link
Contributor

@rjmholt rjmholt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Agreed on not catching the exception -- it's much more performant this way, and doing it the other way would probably just pollute the logs.

Also, I agree with handling it on the server side -- if the behaviour of Invoke-Formatter were to change, it would make more sense to me to make the change here.

@rkeithhill rkeithhill merged commit f035aed into master Jun 13, 2018
@rkeithhill rkeithhill deleted the rkeithhill/fix-formatter-crash-empty-string branch June 13, 2018 02:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants