(PUP-4086) Patch ParseErrors with location if missing
Before this commit, ParseErrors that occured in a call from the
evaluator was passed on verbatim to the higher powers. The intent is
that ParseError is used when the file and line in a .pp file is known.
The use of raised ParseErrors without such information does however
permeate throughput the puppet code base and in modules.
This was bad because many errors would then not be associated with
a location in a manifest.
Further complications were that a) some errors were already preformatted
with location information being baked into the message text when
reaching the evaluator (these should pass verbatim), and b) the way
safeevaluate of AST objects work meant that all errors would end up with
a wrapped 'original' exception which leads to duplicated output of the
message and the text 'Wrapped Exception'.
This commit fixes these problems by:
- Introducing a specialized PreformattedError which is a specialized
variant of ParseError (that enables the evaluator to no yet again add
location information). A choice was made to derive it from ParseError to
ensure that all that rescues ParseError would still recognize it.
- Making the evaluator do more extensive introspection and rewrite of
the different kinds of errors that occur while evaluating one
instruction.
- Making the Errors.adderrorcontext method strip away the original error
since it has already effectively made it superflous by poking its
backtrace into its parent error.
Summary - solution means:
- no superflous Wrapped Exception output
- location information for ParseErrors "in the wild" that do not provide
this.
A possible downside is the dropped original error may cause loss of
information in corner cases (non covered by spec tests at least) if
someone explicitly depends on the original error being kept after a
safeevaluate, or a call path that passes through the evaluator. (No such
cases where found in the puppet code base though).