Skip to content

Remove unneeded catch/throw from 'mkdir' and 'oss' functions#8425

Merged
daxian-dbw merged 1 commit intoPowerShell:masterfrom
daxian-dbw:mkdir
Dec 10, 2018
Merged

Remove unneeded catch/throw from 'mkdir' and 'oss' functions#8425
daxian-dbw merged 1 commit intoPowerShell:masterfrom
daxian-dbw:mkdir

Conversation

@daxian-dbw
Copy link
Member

@daxian-dbw daxian-dbw commented Dec 7, 2018

PR Summary

Remove unneeded catch/throw from 'mkdir' and 'oss' functions.

PR Checklist

@daxian-dbw daxian-dbw self-assigned this Dec 7, 2018
@daxian-dbw daxian-dbw requested a review from BrucePay as a code owner December 7, 2018 23:30
@daxian-dbw daxian-dbw added the CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log label Dec 8, 2018
@daxian-dbw daxian-dbw merged commit 2a12fb5 into PowerShell:master Dec 10, 2018
@daxian-dbw daxian-dbw deleted the mkdir branch December 10, 2018 04:53
@lzybkr
Copy link
Contributor

lzybkr commented Feb 26, 2019

@daxian-dbw - sorry to have just noticed this - the seemingly unnecessary try/catch statements did serve a useful purpose at one point. I recall hating adding them, but IIRC, it improved error reporting in some useful way. Sorry, but I don't remember more than that though.

@iSazonov
Copy link
Collaborator

Does it update ScriptStackTrace in errorrecord?

@daxian-dbw
Copy link
Member Author

@lzybkr Thanks for bringing up that context. Looks like those try/catch were added only for functions that uses SteppablePipeline. Might it be related to errors thrown from within the steppable pipeline? I will play with it a bit to see if I can find out why.

@daxian-dbw
Copy link
Member Author

The "better error reporting" might be related to PowerShell.AddScript(...).Invoke() doesn't throw on terminating error or exception, but instead keep the errors in the error stream.
Here is one example of that:

$ps = [powershell]::create()
$ps.AddScript(@'
 try {
     Get-Process -Invalid 42
 } catch {
     throw
 }
'@)
## try/catch/throw is needed for the exception to be thrown from $ps.invoke()
$ps.Invoke()

But it doesn't seem to be the exact reason for the try/catch/throw in the original mkdir and oss.
The following throws the exception even without the try/catch/throw in mkdir:

$ps = [powershell]::create()
$ps.AddScript("mkdir -Path blahhfe:\jkfjkjakfklajlkf -ea stop") > $null
$ps.Invoke()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments