Skip to content
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

PSEval can be called as $PSEval, @PSEval or %PSEval, but null/empty returns only make sense for $PSEval #25

Closed
jimbobmcgee opened this issue Dec 19, 2024 · 1 comment

Comments

@jimbobmcgee
Copy link

Consider the arbitrary OtterScript...

set @fileLines = @PSEval(Get-Content -LiteralPath 'X:\PathTo\File')
if $ListCount(@fileLines) != 0 {
   Log-Debug "OK"
}

If the Get-Content PowerShell cmdlet returns no lines, then the current approach is to attempt to set the return value of @PSEval to be an empty string, rather than an empty array. This in turn causes Otter to throw a rather nasty exception:

Unhandled exception: System.ArgumentException: Cannot assign a Scalar value to a Vector variable.
   at Inedo.ExecutionEngine.Executer.ExecuterThread.InitializeVariable(RuntimeVariableName name, RuntimeValue value, VariableAssignmentMode mode)
   at Inedo.ExecutionEngine.Executer.ExecuterThread.ExecuteAsync(AssignVariableStatement assignVariableStatement)
   at Inedo.ExecutionEngine.Executer.ExecuterThread.ExecuteNextAsync()

From an end-user perspective, trying to resolve whether a PSEval will return scalar, vector or hash is therefore fraught -- the best I can think of is try { set global @r = @PSEval(...) } catch { set global @r = @() }, which technically proceeds, but the exception is still thrown and the overall job still results in an error state.

Ideally, PSEval should behave differently, depending on whether it was called as $PSEval, @PSEval or %PSEval -- in scalar context, it is fine to return the empty string; in vector context, it would be better if it returned an empty vector; in hash context, an empty hash.

I appreciate that this might not be resolvable straight away, as you probably need assistance from whatever parses and invokes the script line upstream (maybe ExecuterThread?) to pass the desired RuntimeValueType to you. I expect it either needs to be suppled by extending IVariableFunctionContext (which may have side-effects on implementors of that interface); or—more likely—be a protected internal settable property of VariableFunction itself.

Alternatively, if RuntimeValue can be able to be altered to have dedicated constructor/static sentinel value for empty values, ExecuterThread.InitializeVariable might be able to better-placed to handle the scalar/vector/hash decision, and you can just return the sentinel from EvaluateAsync.

@gdivis
Copy link
Contributor

gdivis commented Dec 19, 2024

Hi @jimbobmcgee,

Great feedback! I can appreciate how this could be a problem. You're right that PSEval should take into consideration how it's invoked. Going off my memory of how this works at the ExecuterThread level, the information about how it was called is available so it ought to be possible to pass that to the VariableFunction as you suggest.

I'll take a look at that and other options such as the RuntimeValue change you suggested and let you know. In any case, it seems like something we should be able to resolve pretty easily.

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

No branches or pull requests

3 participants