Skip to content

Commit d2b9cce

Browse files
SeeminglyScienceSeeminglyScience
authored
Better error handling in untitled workspaces and internal functions (#22)
* Fix unhelpful errors in untitled workspaces This change fixes an issue where any command that attempted to infer the workspace manifest would fail with vague errors when in a untitled workspace. This is also the beginning of a refactor of error handling in internal functions. Currently when a internal function throws an error it uses PSCmdlet.ThrowTerminatingError through the internal function ThrowError. This makes it difficult for ThrowError to reliably determine which context to throw from, and makes it impossible to catch. Internal functions will now use the throw statement with an error record so the caller can rethrow or suppress. * Switch GetInferredMember function Also remove the checking for arguments in the AST and the logic that used it in Type.FindMembers. * Switch GetInferredType and GetType * Switch SetEditorLocation * Fix error preference gathering in ThrowError Move the preference check to after context is retrieved so we can check the preference of the caller. * More fixes for untitled workspaces - Fix additional parameter binding issues in GetSettings - Remove CmdletBinding from GetancestorOrThrow. This internal function will still call ThrowError and CmdletBinding will cause ThrowError to throw in the wrong context * Fix workspace path resolution and resources Some of the changes broke resolving paths relative to the workspace path, this change fixes that. Also add some additional handling to ConvertTo-LocalizationString that accounts for the file not existing yet.
1 parent 06337f4 commit d2b9cce

16 files changed

+320
-149
lines changed

‎module/Private/GetAncestorOrThrow.ps1

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ using namespace System.Management.Automation.Language
22

33
function GetAncestorOrThrow {
44
[OutputType([System.Management.Automation.Language.Ast])]
5-
[CmdletBinding()]
65
param(
76
[System.Management.Automation.Language.Ast]
87
$Ast,

‎module/Private/GetInferredManifest.ps1

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,30 @@
11
using namespace Microsoft.PowerShell.EditorServices.Extensions
2+
using namespace System.Management.Automation
23

34
function GetInferredManifest {
45
[CmdletBinding()]
56
param()
67
end {
78
$manifestPath = ResolveRelativePath (GetSettings).SourceManifestPath
8-
if (-not $manifestPath -or -not (Test-Path $manifestPath)) {
9-
ThrowError -Exception ([IO.InvalidDataException]::new($Strings.InvalidManifestSetting)) `
10-
-Id InvalidManifestSetting `
11-
-Category InvalidDataException `
12-
-Target $manifestPath
9+
10+
if ([string]::IsNullOrWhiteSpace($manifestPath)) {
11+
throw [ErrorRecord]::new(
12+
<# exception: #> [PSInvalidOperationException]::new($Strings.MissingWorkspaceManifest),
13+
<# errorId: #> 'MissingWorkspaceManifest',
14+
<# errorCategory: #> 'InvalidOperation',
15+
<# targetObject: #> $manifestPath)
16+
}
17+
18+
if (-not (Test-Path $manifestPath)) {
19+
$exception = [IO.InvalidDataException]::new(
20+
$Strings.InvalidSettingValue -f 'SourceManifestPath')
21+
throw [ErrorRecord]::new(
22+
<# exception: #> $exception,
23+
<# errorId: #> 'InvalidSettingValue',
24+
<# errorCategory: #> 'InvalidData',
25+
<# targetObject: #> $manifestPath)
1326
}
27+
1428
$data = Import-LocalizedData -BaseDirectory (Split-Path $manifestPath) `
1529
-FileName (Split-Path -Leaf $manifestPath)
1630
$null = $data.Add('Name', ((Split-Path $manifestPath -Leaf) -replace '.psd1$'))

‎module/Private/GetInferredMember.ps1

Lines changed: 19 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -36,43 +36,31 @@ function GetInferredMember {
3636
$Ast
3737
)
3838
process {
39-
$type = GetInferredType -Ast $Ast.Expression
40-
41-
# Predicate to use with FindMembers.
42-
$predicate = {
43-
param($Member, $Criteria)
44-
45-
$nameFilter, $argCountFilter = $Criteria
46-
47-
$Member.Name -eq $nameFilter -and
48-
(-not $argCountFilter -or $Member.GetParameters().Count -eq $argCountFilter)
49-
}
50-
# Check if it looks like the argument count was specified explicitly.
51-
$argumentCount = $Ast.Arguments.Count
52-
if ($Ast.Arguments.Count -eq 1 -and $Ast.Arguments.StaticType -eq ([int])) {
53-
$argumentCount = $Ast.Arguments.Value
39+
try {
40+
$type = GetInferredType -Ast $Ast.Expression
41+
} catch {
42+
throw [System.Management.Automation.ErrorRecord]::new(
43+
<# exception: #> $exception,
44+
<# errorId: #> 'MissingMember',
45+
<# errorCategory: #> 'InvalidResult',
46+
<# targetObject: #> $Ast)
5447
}
48+
5549
$member = $type.FindMembers(
5650
<# memberType: #> 'All',
5751
<# bindingAttr: #> [BindingFlags]'NonPublic, Public, Instance, Static, IgnoreCase',
58-
<# filter: #> $predicate,
59-
<# filterCriteria: #> @(($Ast.Member.Value -replace '^new$', '.ctor'), $argumentCount)
60-
61-
# Prioritize properties over fields and methods with smaller parameter counts.
62-
) | Sort-Object -Property `
63-
@{Expression = { $PSItem.MemberType }; Ascending = $false },
64-
@{Expression = {
65-
if ($PSItem -is [MethodBase]) { $PSItem.GetParameters().Count }
66-
else { 0 }
67-
}}
52+
<# filter: #> { param($member, $criteria) $member.Name -eq $criteria },
53+
<# filterCriteria: #> ($Ast.Member.Value -replace '^new$', '.ctor'))
6854

69-
if (-not $member) {
70-
ThrowError -Exception ([MissingMemberException]::new($Ast.Expression, $Ast.Member.Value)) `
71-
-Id MissingMember `
72-
-Category InvalidResult `
73-
-Target $Ast
55+
if ($member) {
56+
return $member
7457
}
7558

76-
$member
59+
$exception = [System.MissingMemberException]::new($Ast.Expression, $Ast.Member.Value)
60+
throw [System.Management.Automation.ErrorRecord]::new(
61+
<# exception: #> $exception,
62+
<# errorId: #> 'MissingMember',
63+
<# errorCategory: #> 'InvalidResult',
64+
<# targetObject: #> $Ast)
7765
}
7866
}

‎module/Private/GetInferredType.ps1

Lines changed: 44 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,13 @@ function GetInferredType {
101101

102102
if ($Ast -is [System.Management.Automation.Language.MemberExpressionAst]) {
103103
$PSCmdlet.WriteDebug('TYPEINF: Starting member inference')
104-
if ($member = GetInferredMember -Ast $Ast) {
104+
try {
105+
$member = GetInferredMember -Ast $Ast
106+
} catch [MissingMemberException] {
107+
$PSCmdlet.WriteDebug("Couldn't find member for AST $Ast")
108+
}
109+
110+
if ($member) {
105111
return (
106112
$member.ReturnType,
107113
$member.PropertyType,
@@ -112,23 +118,37 @@ function GetInferredType {
112118

113119
if ($Ast -is [System.Management.Automation.Language.VariableExpressionAst]) {
114120
$PSCmdlet.WriteDebug('TYPEINF: Starting module state inference')
115-
$inferredManifest = GetInferredManifest -ErrorAction Ignore
116-
$moduleVariable = Get-Module |
117-
Where-Object Guid -eq $inferredManifest.GUID |
118-
ForEach-Object { $PSItem.SessionState.PSVariable.GetValue($Ast.VariablePath.UserPath) } |
119-
Where-Object { $null -ne $PSItem }
120-
121-
if ($moduleVariable) {
122-
return $moduleVariable.Where({ $null -ne $PSItem }, 'First')[0].GetType()
121+
try {
122+
$inferredManifest = GetInferredManifest
123+
} catch {
124+
$PSCmdlet.WriteVerbose($Strings.VerboseInvalidManifest)
125+
}
126+
127+
if ($inferredManifest) {
128+
$moduleVariable = Get-Module |
129+
Where-Object Guid -eq $inferredManifest.GUID |
130+
ForEach-Object { $PSItem.SessionState.PSVariable.GetValue($Ast.VariablePath.UserPath) } |
131+
Where-Object { $null -ne $PSItem }
132+
133+
if ($moduleVariable) {
134+
return $moduleVariable.Where({ $null -ne $PSItem }, 'First')[0].GetType()
135+
}
123136
}
124137

138+
125139
$PSCmdlet.WriteDebug('TYPEINF: Starting global state inference')
126140

127-
$foundInGlobal = $ExecutionContext.
128-
SessionState.
129-
Module.
130-
GetVariableFromCallersModule(
131-
$Ast.VariablePath.UserPath)
141+
# I'd rather this use Module.GetVariableFromCallersModule but it appears to throw
142+
# when a frame in the call stack doesn't have a session state, like the scriptblock
143+
# of a editor command in some cases.
144+
$getVariableSplat = @{
145+
Scope = 'Global'
146+
Name = $Ast.VariablePath.UserPath
147+
ErrorAction = 'Ignore'
148+
}
149+
150+
$foundInGlobal = Get-Variable @getVariableSplat
151+
132152
if ($foundInGlobal -and $null -ne $foundInGlobal.Value) {
133153
return $foundInGlobal.Value.GetType()
134154
}
@@ -137,14 +157,17 @@ function GetInferredType {
137157
}
138158
end {
139159
$type = GetInferredTypeImpl
140-
if (-not $type) {
141-
ThrowError -Exception ([InvalidOperationException]::new($Strings.CannotInferType -f $Ast)) `
142-
-Id CannotInferType `
143-
-Category InvalidOperation `
144-
-Target $Ast
145-
return
160+
if ($type) {
161+
return $type
146162
}
147163

148-
$type
164+
$exception = [System.Management.Automation.PSInvalidOperationException]::new(
165+
$Strings.CannotInferType -f $Ast)
166+
167+
throw [System.Management.Automation.ErrorRecord]::new(
168+
<# exception: #> $exception,
169+
<# errorId: #> 'CannotInferType',
170+
<# errorCategory: #> 'InvalidOperation',
171+
<# targetObject: #> $Ast)
149172
}
150173
}

‎module/Private/GetSettings.ps1

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,18 @@ function GetSettings {
55
function GetHashtable {
66
if ($script:CSSettings) { return $script:CSSettings }
77

8-
$targetPath = Join-Path $psEditor.Workspace.Path -ChildPath 'ESCSSettings.psd1'
9-
10-
if (Test-Path $targetPath) {
11-
$script:CSSettings = Import-LocalizedData -BaseDirectory $psEditor.Workspace.Path `
12-
-FileName 'ESCSSettings.psd1'
13-
14-
return $script:CSSettings
8+
if (-not [string]::IsNullOrWhiteSpace($psEditor.Workspace.Path)) {
9+
$targetPath = Join-Path $psEditor.Workspace.Path -ChildPath 'ESCSSettings.psd1'
10+
11+
if (Test-Path $targetPath) {
12+
$importLocalizedDataSplat = @{
13+
BaseDirectory = $psEditor.Workspace.Path
14+
FileName = 'ESCSSettings.psd1'
15+
}
16+
17+
$script:CSSettings = Import-LocalizedData @importLocalizedDataSplat
18+
return $script:CSSettings
19+
}
1520
}
1621

1722
$script:CSSettings = $script:DEFAULT_SETTINGS

‎module/Private/GetType.ps1

Lines changed: 33 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
using namespace System.Management.Automation
2-
31
function GetType {
42
<#
53
.SYNOPSIS
@@ -26,30 +24,42 @@ function GetType {
2624
[string]
2725
$TypeName
2826
)
29-
process {
30-
$type = $TypeName -as [type]
27+
begin {
28+
function GetTypeImpl {
29+
param()
30+
end {
31+
if ($type = $TypeName -as [type]) {
32+
return $type
33+
}
3134

32-
if (-not $type) {
33-
$type = [AppDomain]::CurrentDomain.
34-
GetAssemblies().
35-
ForEach{ $PSItem.GetType($TypeName, $false, $true) }.
36-
Where({ $PSItem }, 'First')[0]
37-
}
35+
$type = [AppDomain]::CurrentDomain.
36+
GetAssemblies().
37+
ForEach{ $PSItem.GetType($TypeName, $false, $true) }.
38+
Where({ $PSItem }, 'First')[0]
39+
40+
if ($type) {
41+
return $type
42+
}
3843

39-
if (-not $type) {
40-
$type = [AppDomain]::CurrentDomain.
41-
GetAssemblies().
42-
GetTypes().
43-
Where({ $PSItem.ToString() -match "$TypeName$" }, 'First')[0]
44+
$type = [AppDomain]::CurrentDomain.
45+
GetAssemblies().
46+
GetTypes().
47+
Where({ $PSItem.ToString() -match "$TypeName$" }, 'First')[0]
48+
49+
return $type
50+
}
4451
}
45-
# TODO: Pull using statements from the ast to catch some edge cases.
46-
if (-not $type) {
47-
ThrowError -Exception ([RuntimeException]::new($Strings.TypeNotFound -f $TypeName)) `
48-
-Id TypeNotFound `
49-
-Category InvalidOperation `
50-
-Target $TypeName
51-
return
52+
}
53+
process {
54+
if ($type = GetTypeImpl) {
55+
return $type
5256
}
53-
$type
57+
58+
$exception = [PSArgumentException]::new($Strings.TypeNotFound -f $TypeName)
59+
throw [System.Management.Automation.ErrorRecord]::new(
60+
<# exception: #> $exception,
61+
<# errorId: #> 'TypeNotFound',
62+
<# errorCategory: #> 'InvalidArgument',
63+
<# targetObject: #> $TypeName)
5464
}
5565
}

‎module/Private/ResolveRelativePath.ps1

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,37 @@ function ResolveRelativePath {
22
[OutputType([System.Management.Automation.PathInfo])]
33
[CmdletBinding()]
44
param([string]$Path)
5+
begin {
6+
function GetTargetPath {
7+
param()
8+
end {
9+
if ([System.IO.Path]::IsPathRooted($Path)) {
10+
return $Path
11+
}
12+
13+
$basePath = $psEditor.Workspace.Path
14+
if ([string]::IsNullOrWhiteSpace($basePath)) {
15+
$basePath = $PWD.Path
16+
}
17+
18+
if ([string]::IsNullOrWhiteSpace($Path)) {
19+
return $basePath
20+
}
21+
22+
return Join-Path $basePath -ChildPath $Path
23+
}
24+
}
25+
}
526
end {
6-
if ($resolved = (Resolve-Path (Join-Path $psEditor.Workspace.Path $Path) -ErrorAction Ignore)) {
7-
return $resolved
27+
$targetPath = GetTargetPath
28+
if (-not $PSCmdlet.SessionState.Path.IsProviderQualified($targetPath)) {
29+
$targetPath = 'Microsoft.PowerShell.Core\FileSystem::' + $targetPath
30+
}
31+
32+
try {
33+
return $PSCmdlet.SessionState.Path.GetResolvedProviderPathFromPSPath($targetPath, [ref]$null)
34+
} catch {
35+
return $PSCmdlet.SessionState.Path.GetUnresolvedProviderPathFromPSPath($targetPath)
836
}
9-
return Resolve-Path $Path
1037
}
1138
}

‎module/Private/SetEditorLocation.ps1

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,17 @@ function SetEditorLocation {
33
param([string]$Path)
44
end {
55
$resolved = ResolveRelativePath $Path
6+
7+
if (-not (Test-Path $resolved)) {
8+
$exception = [System.Management.Automation.ItemNotFoundException]::new(
9+
[Microsoft.PowerShell.Commands.UtilityResources]::PathDoesNotExist -f $resolved)
10+
throw [System.Management.Automation.ErrorRecord]::new(
11+
<# exception: #> $exception,
12+
<# errorId: #> 'PathNotFound',
13+
<# errorCategory: #> 'ObjectNotFound',
14+
<# targetObject: #> $resolved)
15+
}
16+
617
$psEditor.Workspace.OpenFile($resolved)
718

819
WaitUntil { $psEditor.GetEditorContext().CurrentFile.Path -eq $resolved.Path }

‎module/Private/ThrowError.ps1

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,19 +36,19 @@ function ThrowError {
3636
$Show
3737
)
3838
end {
39-
# Need to manually check error action because of calling the error methods from a different
40-
# cmdlet context. Also reading/setting the error preference variable when the value is "Ignore"
41-
# throws, so we get it through variable intrinsics.
42-
$errorPreference = $ExecutionContext.SessionState.PSVariable.GetValue('ErrorActionPreference')
43-
if ($errorPreference -eq 'Ignore') { return }
44-
4539
if (-not $ErrorContext) {
4640
foreach ($frame in (Get-PSCallStack)) {
4741
if ($frame.Command -eq $MyInvocation.MyCommand.Name) { continue }
4842
if ($ErrorContext = $frame.GetFrameVariables().PSCmdlet.Value) { break }
4943
}
5044
if (-not $ErrorContext) { $ErrorContext = $PSCmdlet }
5145
}
46+
47+
$errorPreference = $ErrorContext.SessionState.PSVariable.GetValue('ErrorActionPreference')
48+
if ($errorPreference -eq 'Ignore') {
49+
return
50+
}
51+
5252
if ($PSCmdlet.ParameterSetName -eq 'New') {
5353
$ErrorRecord = [ErrorRecord]::new($Exception, $Id, $Category, $TargetObject)
5454
}

0 commit comments

Comments
 (0)