Avoided silent errors on loading psake and enabled source information in task error messages. #20

Closed
wants to merge 2 commits into
from

2 participants

@nightroman

Avoided silent but still added to $Error redundant errors on loading psake.

Remove-Module "[p]sake" is the same as Remove-Module "psake" because the
pattern "[p]sake" matches just "psake" and nothing else. At the same time
Remove-Module does not emit errors when a pattern is resolved to nothing
(unlike in the case with a literal argument). As a result we eliminate the
redundant error silently added to $Error on every first loading of psake.

Enabled source information in task error messages shown by psake.

Proposed ($_ | Out-String) gets the error message with source information.
In contrast, the original ($_) gets messages without this useful info. Just a
name of a failed task is often not enough in order to locate the problem fast,
for example in a typical case when an error happens in a script called from a
task. Existing verbose error mode is too verbose in many cases when error file
names and line numbers in messages would be enough.

nightroman added some commits Aug 11, 2011
@nightroman nightroman Avoided silent but still added to $Error redundant errors on loading …
…psake.

Remove-Module "[p]sake" is the same as Remove-Module "psake" because the
pattern "[p]sake" matches just "psake" and nothing else. At the same time
Remove-Module does not emit errors when a pattern is resolved to nothing
(unlike in the case with a literal argument). As a result we eliminate the
redundant error silently added to $Error on every first loading of psake.
b19958c
@nightroman nightroman Enabled source information in task error messages shown by psake.
Proposed ($_ | Out-String) gets the error message with source information.
In contrast, the original ($_) gets messages without this useful info. Just a
name of a failed task is often not enough in order to locate the problem fast,
for example in a typical case when an error happens in a script called from a
task. Existing verbose error mode is too verbose in many cases when error file
names and line numbers in messages would be enough.
06adbdd
@nightroman

If this is not needed, let’s close it. I am perfectly fine with that, really. It would be much better than have it dangling.

@nightroman

Perhaps I should provide more details. Here they are for the change 1.

Change 1.

In the PowerShell command prompt invoke

Import-Module psake
$Error

We can see two errors added:

Import-LocalizedData : Cannot find PowerShell data file 'psake.psd1' in directory 'C:\Windows\system32\WindowsPowerShell\v1.0\Modules\psake\en-US\' or any parent c
ulture directories.
At C:\Windows\system32\WindowsPowerShell\v1.0\Modules\psake\psake.psm1:707 char:21
+ import-localizeddata <<<<  -bindingvariable msgs -erroraction silentlycontinue
...

Remove-Module : No modules were removed. Verify that the specification of modules to remove is correct and those modules exist in the runspace.
At C:\Windows\system32\WindowsPowerShell\v1.0\Modules\psake\psake.psm1:24 char:14
+ remove-module <<<<  psake -erroraction silentlycontinue
...

The proposed change 1 (just 2 extra characters) eliminates the first error
(errors are shown above in reversed order). In many cases this is not a big
deal, indeed. But in other cases, on using psake for testing, for example, it
is often useful to analyse the $Error. The less noise errors it contains the
better.

@nightroman

Change 2

Demo default.ps1

task Test {
    .\Test.ps1
}

where Test.ps1 is a troublesome script like this:

# ...
1/$null
# ...

Now in the PS prompt we call the test:

Import-Module psake
Invoke-psake default.ps1 Test

Output is correct but it does not really help much to realize where the error is:

psake version 4.00
Copyright (c) 2010 James Kovacs

Executing Test
2011-10-04 18:15:24: An Error Occurred:
Attempted to divide by zero.

With the proposed change the output would be more informative:

psake version 4.00
Copyright (c) 2010 James Kovacs

Executing Test
2011-10-04 18:20:17: An Error Occurred:
Attempted to divide by zero.
At C:\TEMP\_111004_180705\Test.ps1:3 char:3
+ 1/ <<<< $null
    + CategoryInfo          : NotSpecified: (:) [], RuntimeException
    + FullyQualifiedErrorId : RuntimeException

This shows the script path, the line number and even the culprit line itself.

Another reason to show the errors in this way is standard PS behaviour users
are used to (presumably). The same error we can see in the PS console if we
invoke test.ps1 directly:

.\test.ps1

That is what default PS console shows:

Attempted to divide by zero.
At C:\TEMP\_111004_180705\Test.ps1:3 char:3
+ 1/ <<<< $null
    + CategoryInfo          : NotSpecified: (:) [], ParentContainsErrorRecordException
    + FullyQualifiedErrorId : RuntimeException

P.S. This example is silly, indeed. But I hope it demonstrates why the proposed change might be useful.

@whut
psake member

I'm new psake developer, I hope that extra pair of hands will make this quicker next time.

Thanks for your patch, it's cool that you shared that with psake, merged.

(I rebased your changed)

@whut whut closed this Oct 5, 2011
@nightroman

Great, thanks!

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