Try to show original exception info for yaml (and other) errors (#24468)

* show original exception for yaml (and other) errors

In places where we need to catch a yaml error and raise
an AnsibleError, add the orig yaml exc to the AnsibleError
via the orig_exc arg.

When the AnsibleError is displayed it will now include the
AnsibleError (AnsibleParserError for example) and the type
and message from the original yaml exception.

This provides more detail to the error messages related to
yaml errors.

This also improves errors from dataloader (for example,
previously if a wrong password was used for a vault encrypted
yaml file, the error was very vague and suggested yaml errors,
but now the message includes the original exception from vault
indicating the password was incorrect or missing).

Add a text note to playbook helper asserts. For playbook
syntax/layout errors that aren't yaml errors, but errors
indicating invalid data structures for a playbook/task/role/block,
we now include some info about where the assert was and
why it was raised.

In places we raise an AnsibleParserError in an except
clause, pass the original exception to AnsibleParserError via
orig_exc arg.

Make assorted error messages a little more specific (like
the playbook helper load methods)

* Revert "Include the original YAML error in syntax error messages"

This reverts commit 781bb44b02.
This commit is contained in:
Adrian Likins 2017-06-09 13:13:15 -04:00 committed by GitHub
parent ccc7f788c0
commit 4befefd78c
16 changed files with 66 additions and 60 deletions

View file

@ -46,7 +46,7 @@ class AnsibleError(Exception):
which should be returned by the DataLoader() class.
'''
def __init__(self, message="", obj=None, show_content=True, suppress_extended_error=False):
def __init__(self, message="", obj=None, show_content=True, suppress_extended_error=False, orig_exc=None):
# we import this here to prevent an import loop problem,
# since the objects code also imports ansible.errors
from ansible.parsing.yaml.objects import AnsibleBaseYAMLObject
@ -61,6 +61,10 @@ class AnsibleError(Exception):
self.message = '%s' % to_native(message)
else:
self.message = '%s' % to_native(message)
if orig_exc:
self.orig_exc = orig_exc
self.message += '\nexception type: %s' % to_native(type(orig_exc))
self.message += '\nexception: %s' % to_native(orig_exc)
def __str__(self):
return self.message

View file

@ -186,7 +186,7 @@ class DataLoader:
return (data, show_content)
except (IOError, OSError) as e:
raise AnsibleParserError("an error occurred while trying to read the file '%s': %s" % (file_name, str(e)))
raise AnsibleParserError("an error occurred while trying to read the file '%s': %s" % (file_name, str(e)), orig_exc=e)
def _handle_error(self, yaml_exc, file_name, show_content):
'''
@ -202,12 +202,7 @@ class DataLoader:
err_obj = AnsibleBaseYAMLObject()
err_obj.ansible_pos = (file_name, yaml_exc.problem_mark.line + 1, yaml_exc.problem_mark.column + 1)
if hasattr(yaml_exc, 'problem'):
err_str = "%s\nThe YAML error was:\n > %s" % (YAML_SYNTAX_ERROR.strip(), yaml_exc.problem)
else:
err_str = YAML_SYNTAX_ERROR
raise AnsibleParserError(err_str, obj=err_obj, show_content=show_content)
raise AnsibleParserError(YAML_SYNTAX_ERROR, obj=err_obj, show_content=show_content, orig_exc=yaml_exc)
def get_basedir(self):
''' returns the current basedir '''
@ -424,7 +419,7 @@ class DataLoader:
return real_path
except (IOError, OSError) as e:
raise AnsibleParserError("an error occurred while trying to read the file '%s': %s" % (to_native(real_path), to_native(e)))
raise AnsibleParserError("an error occurred while trying to read the file '%s': %s" % (to_native(real_path), to_native(e)), orig_exc=e)
def cleanup_tmp_file(self, file_path):
"""

View file

@ -95,8 +95,9 @@ class ModuleArgsParser:
Args may also be munged for certain shell command parameters.
"""
# FIXME: mutable default arg
def __init__(self, task_ds=dict()):
assert isinstance(task_ds, dict)
assert isinstance(task_ds, dict), "the type of 'task_ds' should be a dict, but is a %s" % type(task_ds)
self._task_ds = task_ds
def _split_module_string(self, module_string):

View file

@ -62,7 +62,7 @@ def parse_kv(args, check_raw=False):
vargs = split_args(args)
except ValueError as ve:
if 'no closing quotation' in str(ve).lower():
raise AnsibleParserError("error parsing argument string, try quoting the entire line.")
raise AnsibleParserError("error parsing argument string, try quoting the entire line.", orig_exc=ve)
else:
raise

View file

@ -99,7 +99,10 @@ class AnsibleConstructor(SafeConstructor):
ciphertext_data = to_bytes(value)
if self._b_vault_password is None:
raise ConstructorError(None, None, "found vault but no vault password provided", node.start_mark)
raise ConstructorError(context=None, context_mark=None,
problem="found !vault but no vault password provided",
problem_mark=node.start_mark,
note=None)
# could pass in a key id here to choose the vault to associate with
vault = self._vaults['default']

View file

@ -70,7 +70,7 @@ class AnsibleSequence(AnsibleBaseYAMLObject, list):
# Unicode like object that is not evaluated (decrypted) until it needs to be
# TODO: is there a reason these objects are subclasses for YAMLObject?
class AnsibleVaultEncryptedUnicode(yaml.YAMLObject, AnsibleUnicode):
class AnsibleVaultEncryptedUnicode(yaml.YAMLObject, AnsibleBaseYAMLObject):
__UNSAFE__ = True
__ENCRYPTED__ = True
yaml_tag = u'!vault'

View file

@ -220,7 +220,7 @@ class Base(with_metaclass(BaseMeta, object)):
def load_data(self, ds, variable_manager=None, loader=None):
''' walk the input datastructure and assign any values '''
assert ds is not None
assert ds is not None, 'ds (%s) should not be None but it is.' % ds
# cache the datastructure internally
setattr(self, '_ds', ds)
@ -440,12 +440,12 @@ class Base(with_metaclass(BaseMeta, object)):
setattr(self, name, value)
except (TypeError, ValueError) as e:
raise AnsibleParserError("the field '%s' has an invalid value (%s), and could not be converted to an %s. "
"The error was: %s" % (name, value, attribute.isa, e), obj=self.get_ds())
raise AnsibleParserError("the field '%s' has an invalid value (%s), and could not be converted to an %s."
"The error was: %s" % (name, value, attribute.isa, e), obj=self.get_ds(), orig_exc=e)
except (AnsibleUndefinedVariable, UndefinedError) as e:
if templar._fail_on_undefined_errors and name != 'name':
raise AnsibleParserError("the field '%s' has an invalid value, which appears to include a variable that is undefined. "
"The error was: %s" % (name, e), obj=self.get_ds())
raise AnsibleParserError("the field '%s' has an invalid value, which appears to include a variable that is undefined."
"The error was: %s" % (name, e), obj=self.get_ds(), orig_exc=e)
self._finalized = True
@ -477,10 +477,11 @@ class Base(with_metaclass(BaseMeta, object)):
return {}
else:
raise ValueError
except ValueError:
raise AnsibleParserError("Vars in a %s must be specified as a dictionary, or a list of dictionaries" % self.__class__.__name__, obj=ds)
except ValueError as e:
raise AnsibleParserError("Vars in a %s must be specified as a dictionary, or a list of dictionaries" % self.__class__.__name__,
obj=ds, orig_exc=e)
except TypeError as e:
raise AnsibleParserError("Invalid variable name in vars specified for %s: %s" % (self.__class__.__name__, e), obj=ds)
raise AnsibleParserError("Invalid variable name in vars specified for %s: %s" % (self.__class__.__name__, e), obj=ds, orig_exc=e)
def _extend_value(self, value, new_value, prepend=False):
'''
@ -554,7 +555,7 @@ class Base(with_metaclass(BaseMeta, object)):
and extended.
'''
assert isinstance(data, dict)
assert isinstance(data, dict), 'data (%s) should be a dict but is a %s' % (data, type(data))
for (name, attribute) in iteritems(self._valid_attrs):
if name in data:

View file

@ -121,8 +121,8 @@ class Block(Base, Become, Conditional, Taggable):
loader=self._loader,
use_handlers=self._use_handlers,
)
except AssertionError:
raise AnsibleParserError("A malformed block was encountered.", obj=self._ds)
except AssertionError as e:
raise AnsibleParserError("A malformed block was encountered while loading a block", obj=self._ds, orig_exc=e)
def _load_rescue(self, attr, ds):
try:
@ -136,8 +136,8 @@ class Block(Base, Become, Conditional, Taggable):
loader=self._loader,
use_handlers=self._use_handlers,
)
except AssertionError:
raise AnsibleParserError("A malformed block was encountered.", obj=self._ds)
except AssertionError as e:
raise AnsibleParserError("A malformed block was encountered while loading rescue.", obj=self._ds, orig_exc=e)
def _load_always(self, attr, ds):
try:
@ -151,8 +151,8 @@ class Block(Base, Become, Conditional, Taggable):
loader=self._loader,
use_handlers=self._use_handlers,
)
except AssertionError:
raise AnsibleParserError("A malformed block was encountered.", obj=self._ds)
except AssertionError as e:
raise AnsibleParserError("A malformed block was encountered while loading always", obj=self._ds, orig_exc=e)
def get_dep_chain(self):
if self._dep_chain is None:

View file

@ -43,7 +43,7 @@ def load_list_of_blocks(ds, play, parent_block=None, role=None, task_include=Non
from ansible.playbook.task_include import TaskInclude
from ansible.playbook.role_include import IncludeRole
assert isinstance(ds, (list, type(None)))
assert isinstance(ds, (list, type(None))), '%s should be a list or None but is %s' % (ds, type(ds))
block_list = []
if ds:
@ -89,11 +89,11 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h
from ansible.playbook.handler_task_include import HandlerTaskInclude
from ansible.template import Templar
assert isinstance(ds, list)
assert isinstance(ds, list), 'The ds (%s) should be a list but was a %s' % (ds, type(ds))
task_list = []
for task_ds in ds:
assert isinstance(task_ds, dict)
assert isinstance(task_ds, dict), 'The ds (%s) should be a dict but was a %s' % (ds, type(ds))
if 'block' in task_ds:
t = Block.load(
@ -190,7 +190,7 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h
if not found:
try:
include_target = templar.template(t.args['_raw_params'])
except AnsibleUndefinedVariable:
except AnsibleUndefinedVariable as e:
raise AnsibleParserError(
"Error when evaluating variable in include name: %s.\n\n"
"When using static includes, ensure that any variables used in their names are defined in vars/vars_files\n"
@ -198,7 +198,7 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h
"sources like group or host vars." % t.args['_raw_params'],
obj=task_ds,
suppress_extended_error=True,
)
orig_exc=e)
if t._role:
include_file = loader.path_dwim_relative(t._role._role_path, subdir, include_target)
else:
@ -341,7 +341,7 @@ def load_list_of_roles(ds, play, current_role_path=None, variable_manager=None,
# we import here to prevent a circular dependency with imports
from ansible.playbook.role.include import RoleInclude
assert isinstance(ds, list)
assert isinstance(ds, list), 'ds (%s) should be a list but was a %s' % (ds, type(ds))
roles = []
for role_def in ds:

View file

@ -121,7 +121,7 @@ class Play(Base, Taggable, Become):
Adjusts play datastructure to cleanup old/legacy items
'''
assert isinstance(ds, dict)
assert isinstance(ds, dict), 'while preprocessing data (%s), ds should be a dict but was a %s' % (ds, type(ds))
# The use of 'user' in the Play datastructure was deprecated to
# line up with the same change for Tasks, due to the fact that
@ -145,8 +145,8 @@ class Play(Base, Taggable, Become):
'''
try:
return load_list_of_blocks(ds=ds, play=self, variable_manager=self._variable_manager, loader=self._loader)
except AssertionError:
raise AnsibleParserError("A malformed block was encountered.", obj=self._ds)
except AssertionError as e:
raise AnsibleParserError("A malformed block was encountered while loading tasks", obj=self._ds, orig_exc=e)
def _load_pre_tasks(self, attr, ds):
'''
@ -155,8 +155,8 @@ class Play(Base, Taggable, Become):
'''
try:
return load_list_of_blocks(ds=ds, play=self, variable_manager=self._variable_manager, loader=self._loader)
except AssertionError:
raise AnsibleParserError("A malformed block was encountered.", obj=self._ds)
except AssertionError as e:
raise AnsibleParserError("A malformed block was encountered while loading pre_tasks", obj=self._ds, orig_exc=e)
def _load_post_tasks(self, attr, ds):
'''
@ -165,8 +165,8 @@ class Play(Base, Taggable, Become):
'''
try:
return load_list_of_blocks(ds=ds, play=self, variable_manager=self._variable_manager, loader=self._loader)
except AssertionError:
raise AnsibleParserError("A malformed block was encountered.", obj=self._ds)
except AssertionError as e:
raise AnsibleParserError("A malformed block was encountered while loading post_tasks", obj=self._ds, orig_exc=e)
def _load_handlers(self, attr, ds):
'''
@ -175,8 +175,8 @@ class Play(Base, Taggable, Become):
'''
try:
return load_list_of_blocks(ds=ds, play=self, use_handlers=True, variable_manager=self._variable_manager, loader=self._loader)
except AssertionError:
raise AnsibleParserError("A malformed block was encountered.", obj=self._ds)
except AssertionError as e:
raise AnsibleParserError("A malformed block was encountered while loading handlers", obj=self._ds, orig_exc=e)
def _load_roles(self, attr, ds):
'''
@ -189,8 +189,8 @@ class Play(Base, Taggable, Become):
try:
role_includes = load_list_of_roles(ds, play=self, variable_manager=self._variable_manager, loader=self._loader)
except AssertionError:
raise AnsibleParserError("A malformed role declaration was encountered.", obj=self._ds)
except AssertionError as e:
raise AnsibleParserError("A malformed role declaration was encountered.", obj=self._ds, orig_exc=e)
roles = []
for ri in role_includes:

View file

@ -105,7 +105,7 @@ class PlaybookInclude(Base, Conditional, Taggable):
up with what we expect the proper attributes to be
'''
assert isinstance(ds, dict)
assert isinstance(ds, dict), 'ds (%s) should be a dict but was a %s' % (ds, type(ds))
# the new, cleaned datastructure, which will have legacy
# items reduced to a standard structure

View file

@ -209,16 +209,18 @@ class Role(Base, Become, Conditional, Taggable):
if task_data:
try:
self._task_blocks = load_list_of_blocks(task_data, play=self._play, role=self, loader=self._loader, variable_manager=self._variable_manager)
except AssertionError:
raise AnsibleParserError("The tasks/main.yml file for role '%s' must contain a list of tasks" % self._role_name, obj=task_data)
except AssertionError as e:
raise AnsibleParserError("The tasks/main.yml file for role '%s' must contain a list of tasks" % self._role_name,
obj=task_data, orig_exc=e)
handler_data = self._load_role_yaml('handlers')
if handler_data:
try:
self._handler_blocks = load_list_of_blocks(handler_data, play=self._play, role=self, use_handlers=True, loader=self._loader,
variable_manager=self._variable_manager)
except AssertionError:
raise AnsibleParserError("The handlers/main.yml file for role '%s' must contain a list of tasks" % self._role_name, obj=handler_data)
except AssertionError as e:
raise AnsibleParserError("The handlers/main.yml file for role '%s' must contain a list of tasks" % self._role_name,
obj=handler_data, orig_exc=e)
# vars and default vars are regular dictionaries
self._role_vars = self._load_role_yaml('vars', main=self._from_files.get('vars'))

View file

@ -80,7 +80,7 @@ class RoleMetadata(Base):
role_def['name'] = def_parsed['name']
roles.append(role_def)
except AnsibleError as exc:
raise AnsibleParserError(str(exc), obj=role_def)
raise AnsibleParserError(str(exc), obj=role_def, orig_exc=exc)
current_role_path = None
if self._owner:
@ -89,8 +89,8 @@ class RoleMetadata(Base):
try:
return load_list_of_roles(roles, play=self._owner._play, current_role_path=current_role_path, variable_manager=self._variable_manager,
loader=self._loader)
except AssertionError:
raise AnsibleParserError("A malformed list of role dependencies was encountered.", obj=self._ds)
except AssertionError as e:
raise AnsibleParserError("A malformed list of role dependencies was encountered.", obj=self._ds, orig_exc=e)
def _load_galaxy_info(self, attr, ds):
'''

View file

@ -161,7 +161,7 @@ class Task(Base, Conditional, Taggable, Become):
keep it short.
'''
assert isinstance(ds, dict)
assert isinstance(ds, dict), 'ds (%s) should be a dict but was a %s' % (ds, type(dict))
# the new, cleaned datastructure, which will have legacy
# items reduced to a standard structure suitable for the
@ -177,7 +177,7 @@ class Task(Base, Conditional, Taggable, Become):
try:
(action, args, delegate_to) = args_parser.parse()
except AnsibleParserError as e:
raise AnsibleParserError(to_native(e), obj=ds)
raise AnsibleParserError(to_native(e), obj=ds, orig_exc=e)
# the command/shell/script modules used to support the `cmd` arg,
# which corresponds to what we now call _raw_params, so move that

View file

@ -385,7 +385,7 @@ class Templar:
are being changed.
'''
assert isinstance(variables, dict)
assert isinstance(variables, dict), "the type of 'variables' should be a dict but was a %s" % (type(variables))
self._available_variables = variables
self._cached_result = {}

View file

@ -142,7 +142,7 @@ class VariableManager:
@extra_vars.setter
def extra_vars(self, value):
''' ensures a clean copy of the extra_vars are used to set the value '''
assert isinstance(value, MutableMapping)
assert isinstance(value, MutableMapping), "the type of 'value' for extra_vars should be a MutableMapping, but is a %s" % type(value)
self._extra_vars = value.copy()
def set_inventory(self, inventory):
@ -156,7 +156,7 @@ class VariableManager:
@options_vars.setter
def options_vars(self, value):
''' ensures a clean copy of the options_vars are used to set the value '''
assert isinstance(value, dict)
assert isinstance(value, dict), "the type of 'value' for options_vars should be a dict, but is a %s" % type(value)
self._options_vars = value.copy()
def _preprocess_vars(self, a):
@ -577,7 +577,7 @@ class VariableManager:
Sets or updates the given facts for a host in the fact cache.
'''
assert isinstance(facts, dict)
assert isinstance(facts, dict), "the type of 'facts' to set for host_facts should be a dict but is a %s" % type(facts)
if host.name not in self._fact_cache:
self._fact_cache[host.name] = facts
@ -592,7 +592,7 @@ class VariableManager:
Sets or updates the given facts for a host in the fact cache.
'''
assert isinstance(facts, dict)
assert isinstance(facts, dict), "the type of 'facts' to set for nonpersistent_facts should be a dict but is a %s" % type(facts)
if host.name not in self._nonpersistent_fact_cache:
self._nonpersistent_fact_cache[host.name] = facts