RFC Trap handle non terminating errors

[TylerLeonhardt]

Adding ErrorType property to ErrorRecord and making traps handle non-terminating errors as well.

trap {
    if($_.ErrorType -eq "Terminating") {
        "oh no!"
    } else {
        "this is ok"
        continue
    }
}

The purpose would be to offer a single solution to handle both terminating and non-terminating issues

[vexx32]

I like the idea of these constructs being able to handle both terminating and non-terminating errors.

However, why would we create a disparity here? There are very good reasons trap is not used; namely, it separates the error handling an arbitrary amount from the code being called, with no logical way to see what code will be called to handle the error without looking at the code itself from top to bottom with a fine-tooth comb.

Use of trap should not be encouraged over try/catch in my opinion. Once a trap statement has been declared for a scope, you cannot simply "remove" the trapping functionality from that scope.

We should aim to apply this to try/catch as well. I don't see any advantage to trap that doesn't come with significant downsides to whoever has the unfortunate job of needing to debug it later. I also don't see any need to encourage the use of difficult to maintain code patterns via adding a feature to trap that is unavailable on try/catch.

I do however very much like the idea of adding such a property to ErrorRecord; it's long been missing, and it's a very welcome addition.

It's not clear to me if changing the errors caught would be inclusive or exclusive. Does this aim to replace or add on to the existing functionality? I.e., if I use this feature, will I ONLY be catching non-terminating errors now, or will I catch both?

Should we allow users to select one, the other, and both as a third separate option?

[SteveL-MSFT]

@TylerLeonhardt I think there needs to be a Alternate proposal and considerations section explaining why we decided not to go with a try/catch approach. Also add an example where trap is used with try/catch as the two are not mutually exclusive.

[TylerLeonhardt]

Sounds good. If I have some downtime on my travels I'll add something.

[iSazonov]

/cc @mklement0 for information.

[KirkMunro]

Without a broader vision or story describing the plans to fix error handling/exception handling in PowerShell, this RFC seems premature. At face value this feels like it just continues a game of cat and mouse: first we get trap, then try/catch, now trap wants to do more than try/catch can, and so on. Plus, ugh, you want the PowerShell community to go from using try/catch back to trap now, really?

A few other random thoughts on this:

1. Users can already distinguish between terminating and non-terminating errors, and handle them appropriately, except that they need to wrap everything in try/catch in order for terminating errors to properly terminate because of the decision long ago that terminating errors should not terminate without that wrapper. IMHO this issue is far more important to address so that the anti-pattern where scripters wrap everything in try/catch{throw} can go away.
2. The way this is proposed right now would be a breaking change. If you must have this functionality (I'm definitely not convinced it's necessary though), better to have a parameter on trap that can be used to catch specific types of errors (i.e. maybe something like: trap -terminating or trap -nonterminating). But again, I'd really like to be convinced.
3. If you want to keep the RFC on the table, can you please put in a lot more detail? Show a detailed example of the problem you want to solve, show how it can be solved today, and explain why the new use of trap definition is much better?

FWIW, the only use I have found for the trap statement in recent years is in psm1 files so that you can make terminating errors terminate without actually having to wrap the entire psm1 file in try/catch{throw}. But again, the solution to that problem is not a better trap statement -- it's to fix exception handling so that terminating errors actually terminate. There's even a way this can be done without impacting existing scripts and without impacting ad hoc use -- I just need the time to type up the RFC for it.

[KirkMunro]

More food for thought for this RFC and Issue #6010:

1. We have -ErrorAction Break now that breaks into the debugger for terminating or non-terminating errors (or we will, once PR 8205 is merged).
2. We could (and really should) update ActionPreference.Inquire so that it gives users an option to Debug instead of Suspend. The debugger, which can now be entered on demand in the appropriate line (again, once that PR is manually tested/merged), is of much more value than a nested prompt ever was. This gives users an automatic question, which was one of the proposals in an example for Issue #6010. Again, this is dependent on getting PR 8205 merged first (I mentioned I have a few things waiting in the wings behind that PR in the PR discussion).
3. Instead of -OnError or this newfangled trap statement, what about allowing any -*Action common parameter to accept either an ActionPreference value or a ScriptBlock, where the output of the ScriptBlock is the ActionPreference, with the default ActionPreference respected if the ScriptBlock does not return any output? It wouldn't be a breaking change. There would be no issue with redefining break or continue statements, because they wouldn't be used to control what is done. Instead, scripters would still use an ActionPreference value, which is easier with my PR merged because it adds ActionPreference as a type accelerator as well. You could also support script blocks for *Preference variables that hold ActionPreference values and get broader support for logging/error handling without updating trap and without adding -OnError.
4. Part of this RFC talks about adding an ErrorType property to ErrorRecord that can be used to identify if an error is terminating or not. How about [bool] Terminating with a value of $true or $false instead? If ever I'm checking if an error was terminating or not, I'd rather not have to do string comparisons or use enumerations. Further, that change doesn't need to be in this RFC at all and could just be added in a PR, couldn't it? That would add value regardless of the direction of this RFC, and should really just be put in as soon as someone can do it. But please use a boolean rather than an enumeration. Or if you think the enumeration is really necessary, then this RFC needs to identify why that's needed too (i.e. what are the current/future enumerated values you forsee being necessary, and why would users care about anything beyond terminating vs non-terminating?).

[KirkMunro]

To sum up what I suggested above (plus adding some related info):

1. Close this RFC if I'm correct that it's not the right solution to the problems you are trying to solve.
2. Assign resources to manually test PowerShell PR #8205 and work with me to get it merged. Turns out I figured out how to automate debugger commands in Pester tests, and submitted that as a separate PR, so I don't need manual testing done -- I just need to add automated tests once PowerShell PR #9825 is merged.
3. Once the PR is merged, I'll submit another to replace the Suspend option in ActionPreference.Inquire with Debug so that users can enter the debugger on demand.
4. Add [bool] Terminating to ErrorRecord (or an enumeration if you must).
5. As a separate RFC (if an RFC is required for this -- it wouldn't be breaking, but the discussion may be useful), add support for using scriptblock instances with *Preference variables that traditionally would only accept an ActionPreference value, and support scriptblock values in all common -*Action parameters, where the value of $_ in the scriptblock refers to the appropriate record (ErrorRecord, WarningRecord, etc.), and if the output of the scriptblock is an ActionPreference, that value is used for the action to take; otherwise, the default ActionPreference value is used (which may be another script block depending on how the corresponding *Preference variable is defined in the caller's scope).
6. With respect to carrying these behaviors between modules or from modules to scripts, etc., see this comment that talks about how we could resolve that problem once and for all (it could probably be part of the same RFC).
7. Fix terminating error handling where it is needed so that the try/catch{throw} anti-pattern goes away (maybe also part of the same RFC).

If you'd like me to take a kick at such an RFC (or RFCs if you want multiple, with them referencing one another since they are related), let me know.

[rjmholt]

Close this RFC if I'm correct that it's not the right solution to the problems you are trying to solve.

I don't see there being a hard wrong or right here. I think some context might be helpful.

Changing the behaviour of trap (or augmenting it) doesn't amount to telling PowerShell users to prefer trap to try/catch.

The original problem was specified in PowerShell/PowerShell#6010, where @jpsnover suggested a parameter to provide a scriptblock as a continuation to invoke when errors or any kind are encountered.

He particularly had in mind long-running distributed services where interaction is not possible; there are scenarios where logging is the only real solution and nothing handled the case of needing to log and then keep going (i.e. not stop the pipeline) when a non-terminating error is encountered.

(Well almost nothing -- another possibility would be something like command 2>&1 | Log-Errors | Invoke-DownstreamCommand, where I think an alternative non-breaking proposal would be to have a parameter to turn terminating errors into a non-terminating one that stops the pipeline so that downstream commands get one last object to execute)

Looking at this we realised that wrapping every single statement in try/catch is incredibly tedious, but more importantly, the PowerShell compiler already does it for us.

It's well known that PowerShell's error behaviour is tortuous, but fixing it would probably break a very large number of scripts which I think it's in PowerShell's interest to maintain compatibility with. (I know the RFC proposes breaking trap, but personally I'm hoping for trapall or something instead). So instead, augmenting trap is a low-risk way of solving the original problem.

[rjmholt]

My personal preference is trapall, since:

• It doesn't break the existing behaviour of trap
• It isn't a syntactic break; trapall will still parse and execute, but will just throw when the statement invoking trapall is hit in older PowerShell verisons. Compare this to trap -NonTerminating which will not parse in older PowerShell versions. This means you can wrap the former in a version condition and it will still work cross-version.

We might even able to carefully define trapall so that ICustomAstVisitor isn't broken, by reusing the TrapStatementAst type with a new parameter in the constructor detailing whether it's a trap or a trapall statement.

[rjmholt]

However, why would we create a disparity here? There are very good reasons trap is not used; namely, it separates the error handling an arbitrary amount from the code being called, with no logical way to see what code will be called to handle the error without looking at the code itself from top to bottom with a fine-tooth comb.

Use of trap should not be encouraged over try/catch in my opinion. Once a trap statement has been declared for a scope, you cannot simply "remove" the trapping functionality from that scope.

I think these are very valid concerns, but one thing I think should be stated is that there are two opposing design desires here:

• Being able to handle errors consistently without having to write try/catch (or some other thing) on every single line
• Being able to handle an error in a specific statement, distinct from others

If I were designing things from scratch, I would imagine an error handling specification should work like CSS where the most specific definition wins, and when that's ambiguous, the last defined.

But I think we've got too much backward compatibility to bear to rearchitect that.

[vexx32]

Eh, I don't think enhancing only trap is the solution. I mean, try/catch already allows the use of the continue keyword which though not used as often as it could be, allows users to continue execution from the point of the error.

If we enhance trap in the suggested ways, it doesn't make sense not to also enhance try/catch in the same way, in my opinion. All you create is confusion at the outset because these two constructs have up to this point been identical in their capabilities. Once that hurdle is overcome, then we also are introducing huge maintainability problems to come by encouraging the use of trap. Trap unconditionally diverts the flow of execution across the entire torrent scope when an error occurs. A single trap statement inserted at any point in the code can drastically change how execution in handled.

In any sufficiently large script, this becomes a maintainability problem, because there is no clear indicator of how an error at any given point will be handled. Try/catch is generally favoured because it is crystal clear what portions of code are covered by it, there's no guesswork. Creating a scenario where trap is more capable than its counterpart is a very bad idea in my opinion as it encourages very difficult to maintain coding practices that simply do not scale well at all.

[KirkMunro]

@rjmholt Thanks for the details. I had most of that background already. I don't believe that changing trap is the right solution. Nor do I believe that changing try/catch is the right solution either.

It's well known that PowerShell's error behaviour is tortuous, but fixing it would probably break a very large number of scripts which I think it's in PowerShell's interest to maintain compatibility with.

That's a very misleading statement. I believe there are significant changes that can be made to PowerShell's current error behavior (which I would hardly describe as tortuous...onerous, maybe) to fix a lot of the difficulties without introducing breaking changes whatsoever. By stating that it just can't be done sounds like you guys are just washing your hands of making that effort to get it fixed. What's tortuous is that it's taking this long to get those important issues resolved.

[TylerLeonhardt]

I'm still on vacation (which is why I can't add more details to the RFC just yet) but I wanted to enforce the reasoning behind this RFC as stated in this RFC:

The purpose would be to offer a single solution to handle both terminating and non-terminating issues

@KirkMunro I don't see your suggestion to the *Action variables as solving that goal. Don't get me wrong, I actually really like that idea on its own and think an RFC for that would be awesome, but I don't think that solves the problem discussed in this RFC.

@vexx32

try/catch already allows the use of the continue keyword which though not used as often as it could be, allows users to continue execution from the point of the error.

I tried this and this doesn't seem to be true. Spoke to Joel about it and he thinks he was assuming that was the behavior because trap has this behavior today.

If we go down the trapall route, I want to point out that I'm not against a potential catchall as well that does add the behavior of continue and break within that trap has today. Possibly a different RFC, possibly not.

[KirkMunro]

@TylerLeonhardt I read the purpose statement, but I still disagree with this RFC.

The timing of this RFC is wrong (there are other very closely related problems that should be solved before this problem is even considered). There is no overall vision or story that clearly articulates the specific problems that make error handling difficult to work with in PowerShell, so that each of those problems can be considered when making any investment in how PowerShell deals with errors, which creates a significant risk that this RFC, if implemented, will just be adding to the pile and not getting to the crux of the issues with error handling. There are other error handling issues/discussions that should happen first.

Another area of significant risk/concern for me with this RFC is that it blurs the line between terminating and non-terminating errors even more (the other source of blur is how PowerShell treats terminating errors as non-terminating unless wrapped in try/catch), and I don't think that line should be blurred at all. Executing beyond a terminating error should require intentional code, not accidental, and I'm afraid if this RFC is implemented we'll see even more mishandling of terminating errors than we already do today.

With those concerns I have, my reason to push back on this RFC is that it takes scarce attention and resources away from the other issues/discussions in this area that really need to happen first, while risking just throwing more wood onto the fire at the same time. I don't have confidence that you guys are really looking at a broader scope for how errors are handled in PowerShell. I really wish I did, but with this RFC coming out, I'm afraid I just don't.

[KirkMunro]

@TylerLeonhardt PR #187 is my attempt at what should be done to fix much of the error handling in PowerShell, in ways that I talked about above. Note that there are four RFCs in that PR, all related to one another in some way. I think these are tackling the more important problems, while still working on the original issues that started you looking at in this RFC in the first place.

[jtmoree-github-com]

Seems to me that we have not set expectations very well in regards to error handling. This is creating--and getting in the way of solving--many of the problems. For a moment I'm going to ignore all backward compatibility and do some brainstorming. How is this situation handled in other domains? Don't all of them do this the same way as each other and differently than powershell?

In other languages Terminating Errors and Non Terminating errors are called different things. One of them is so important that the current process should stop running and the other is no less important but the caller only needs to be notified of a problem without stopping. These are two distinct workflows.

python, bash, perl, you name it. Errors NEVER terminate. Terminations ONLY come from explicit commands such as throw, raise, and exit. The interpreter blowing up is a different issue but yes that stops the program from running too.

Yeah, I think errors should never terminate in powershell. That model is working pretty well for everyone else. $ErrorActionPreference would not be needed. The only way to eliminate the guesswork is to have the user explicitly use a command that terminates or not. Use write-error to record an error but don't stop. Throw an exception to stop immediately and catch where appropriate. If we eliminate terminating errors we don't have to 'handle' them any different than 'errors' which is what the RFC is calling for.

What about calling non powershell code? All of the commands and programs we call using powershell can't throw. I call some old DOS utility to format the disk and it can only spit out text and return an integer. Handling errors from non powershell commands is one of the things I spend the most time on but that seems like a different problem than error handling within powershell itself. If I am calling an external utility I usually wrap it in try/catch and throw when I get bad results.

Could this be implemented with backward compatibility? seems unlikely given the mess that is powershell error handling and the expectation that ErrorActionPreference = 'stop' will actually work sometimes. I suppose backward compatibility might be possible with $errorActionPreference = 'never'. The problem is exacerbated by having to use try/catch around any piece of code that should be allowed to throw.

Anyway, I'm done rambling.