Skip to content

Internal error when expanding auto variable holding Linq.GroupBy result. #4174

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

Closed
6 tasks done
kborowinski opened this issue Sep 15, 2022 · 6 comments · Fixed by PowerShell/PowerShellEditorServices#1975
Closed
6 tasks done
Assignees
Labels

Comments

@kborowinski
Copy link

Prerequisites

  • I have written a descriptive issue title.
  • I have searched all open and closed issues to ensure it has not already been reported.
  • I have read the troubleshooting guide.
  • I am sure this issue is with the extension itself and does not reproduce in a standalone PowerShell instance.
  • I have verified that I am using the latest version of Visual Studio Code and the PowerShell extension.
  • If this is a security issue, I have read the security issue reporting guidance.

Summary

Auto variable of type GroupedEnumerable<int, bool> fails on expansion in Variables panel with internal error:
image

PowerShell Version

Name                           Value
----                           -----
PSVersion                      7.2.6
PSEdition                      Core
GitCommitId                    7.2.6
OS                             Microsoft Windows 10.0.19044
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0, 5.0, 5.1.10032.0, 6.0.0, 6.1… 
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

Visual Studio Code Version

1.72.0-insider
cfc01197555cb14a0d2d3463b331ce41de5733be
x64

Extension Version

Steps to Reproduce

  1. Start debugging following script:
[int[]]$a = 1..100
$b = [Linq.Enumerable]::GroupBy($a, [Func[int, bool]]{if ($args[0] % 3) {$false} else {$true}})
$b.Count
  1. Try to expand $b variable in Variables panel view

Visuals

Animation

Logs

1663251725-e5f15643-1440-4168-a26a-5c23a0f319ce1663251721624.zip

@kborowinski kborowinski added the Issue-Bug A bug to squash. label Sep 15, 2022
@ghost ghost added the Needs: Triage Maintainer attention needed! label Sep 15, 2022
@SeeminglyScience
Copy link
Collaborator

We talked about this a little bit on discord:

seeminglyscience — Today at 10:55 AM
honestly in that case, the error might be preferable over actually invoking the code
but not necessarily his change, either way fixed by marshaling variable expansion back to the pipeline thread
though I'm tempted to say we shouldn't expand IEnumerable<> that isn't IList<> or something similar. For longer queries that'd likely lock up the process

bukem — Today at 11:02 AM
I think so too. Should we have some info message instead of exception though?

seeminglyscience — Today at 11:03 AM
I'd probably check what C# does. Though their eval is quite a bit more restrictive than ours so it might just say it can't eval native code or some other unhelpful message

@andyleejordan
Copy link
Member

andyleejordan commented Sep 15, 2022

@SeeminglyScience would we want to move this code into ExecuteDelegateAsync with a fresh nested runspace? I did something like that to replace how we get ExecutionContext instead of using Get-Variable and it worked (change is stashed somewhere).

@andyleejordan andyleejordan added Area-Debugging and removed Needs: Triage Maintainer attention needed! labels Oct 4, 2022
@andyleejordan
Copy link
Member

Thought from Patrick to improve the stability here: if a variable implements IEnumerable but not IList or IDictionary or any other concrete type, we need to skip its expansion.

@andyleejordan andyleejordan self-assigned this Dec 7, 2022
@andyleejordan
Copy link
Member

I changed it so the expansion of child variables takes place on the pipeline thread and now we can expand $b into its two values (arrays of numbers computed from the lamba), works splendidly. We might find that this causes problems, but since it works I want to try it in preview. We pass along a cancellation token, and though it's probably possible to lock it up...that would be based on the user's input (the code that's being debugged) and probably not a big deal.

@andyleejordan
Copy link
Member

@kborowinski the fix for this is available in the preview now if you'd like to try it out!

@kborowinski
Copy link
Author

@kborowinski the fix for this is available in the preview now if you'd like to try it out!

@andschwa Works like a charm! Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants