Skip to content

Fix command runtime so StopUpstreamCommandsException doesn't get populated in -ErrorVariable#10840

Merged
anmenaga merged 2 commits intoPowerShell:masterfrom
SteveL-MSFT:StopUpstreamCommandsException
Oct 22, 2019
Merged

Fix command runtime so StopUpstreamCommandsException doesn't get populated in -ErrorVariable#10840
anmenaga merged 2 commits intoPowerShell:masterfrom
SteveL-MSFT:StopUpstreamCommandsException

Conversation

@SteveL-MSFT
Copy link
Copy Markdown
Member

@SteveL-MSFT SteveL-MSFT commented Oct 18, 2019

PR Summary

StopUpstreamCommandsException is a special exception used by cmdlets to tell PowerShell to stop upstream commands as it doesn't need any additional input. For example: "1,2,3 | select-object -first 1" should stop the first part of the pipeline after select-object received 1 object as the request has been fulfilled. However, if you use 1,2,3 | select-object -first 1 -errorvariable err, the internal exception (which is handled) is put into the $err variable (although $error it not populated). This means that scripts may treat this as an error or special case this System error and throwing it away.

The fix is in the code where the engine populates -ErrorVariable to discard StopUpstreamCommandsException.

I don't believe this is a breaking change as anyone working around this by throwing away that specific error (because select-object actually succeeded) won't be impacted by this change.

PR Context

Fix #9185

PR Checklist

@iSazonov
Copy link
Copy Markdown
Collaborator

@SteveL-MSFT Why do you start to add prefixes to PR titles? This is contrary to our conventions.

@SteveL-MSFT
Copy link
Copy Markdown
Member Author

@iSazonov trying something based on an article I read about good commit messages. I suppose the prefix can be just in the commit and not in the PR title.

@SteveL-MSFT SteveL-MSFT changed the title BUG: Fix command runtime so StopUpstreamCommandsException doesn't get populated in -ErrorVariable Fix command runtime so StopUpstreamCommandsException doesn't get populated in -ErrorVariable Oct 20, 2019
@iSazonov
Copy link
Copy Markdown
Collaborator

@SteveL-MSFT All that we can not formalize is a human factor. I mean, something that can make things clearer can also be misleading. In this case, the simpler the better. So better not to have prefixes at all.
The most useful thing that we now have is the PR number in the commit title. This allows us to quickly find PR on GitHub and a history of the change. GitLens in VS Code greatly support this.

@iSazonov iSazonov added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Oct 20, 2019
@iSazonov iSazonov added this to the 7.0.0-preview.5 milestone Oct 20, 2019
@vexx32
Copy link
Copy Markdown
Collaborator

vexx32 commented Oct 20, 2019

@SteveL-MSFT :bug: (🐛) -gt BUG: 😉

https://gitmoji.carloscuesta.me/

But regardless, I agree that for commit messages having an easily-identifiable prefix can be a nice-to-have. Whether that's emoji or text is, I suppose, up to you guys. 😄

An emoji guide for your commit messages.

@anmenaga anmenaga merged commit 26c380d into PowerShell:master Oct 22, 2019
@SteveL-MSFT SteveL-MSFT deleted the StopUpstreamCommandsException branch October 23, 2019 17:54
@ghost
Copy link
Copy Markdown

ghost commented Nov 21, 2019

🎉v7.0.0-preview.6 has been released which incorporates this pull request.:tada:

Handy links:

awsr added a commit to awsr/PS-StreamXRef that referenced this pull request Jun 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Select-Object -First populates -ErrorVariable

4 participants