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

Double extended error message in AnsibleParserError (triple in 2.4) #36848

Closed
diafour opened this issue Feb 28, 2018 · 6 comments · Fixed by #36923
Closed

Double extended error message in AnsibleParserError (triple in 2.4) #36848

diafour opened this issue Feb 28, 2018 · 6 comments · Fixed by #36923
Assignees
Labels
affects_2.4 This issue/PR affects Ansible v2.4 bug This issue/PR relates to a bug. support:core This issue/PR relates to code supported by the Ansible Engineering Team.

Comments

@diafour
Copy link
Contributor

diafour commented Feb 28, 2018

ISSUE TYPE
  • Bug Report
COMPONENT NAME

AnsibleError, AnsibleParserError, Task#preprocess_data, ModuleArgsParser#parse

ANSIBLE VERSION
2.4, 2.5, devel
CONFIGURATION

No special configuration, local connection.

ansible-playbook -c local -i localhost, playbook.yml

playbook.yml:

---
- hosts: all
  tasks:
  - apk: msgfd
OS / ENVIRONMENT

Ubuntu 16.04, Python 3.5.2 (default, Nov 23 2017, 16:37:01) in virtualenv

SUMMARY

ModuleArgsParser#parse raises AnsibleParserError if something is wrong with tasks or their arguments.
AnsibleParserError#__init__ append extended error to the message.
Then Task#preprocess_data wraps AnsibleParserError with new instance of AnsibleParserError and thus extended error is doubled in message.

Extended error is printed 3 times in version 2.4. devel branch has merged PR #24468 and extended error is printed 2 times.

May be something wrong with this wrapping?

args_parser = ModuleArgsParser(task_ds=ds)
try:
(action, args, delegate_to) = args_parser.parse()
except AnsibleParserError as e:
raise AnsibleParserError(to_native(e), obj=ds, orig_exc=e)

STEPS TO REPRODUCE

Simple test case that raises error about extra params from

raise AnsibleParserError("this task '%s' has extra params, which is only allowed in the following modules: %s" % (action,

---
- hosts: all
  tasks:
  - apk: msgfd

Create file and run it localy for verification:

$ ansible-playbook -c local -i localhost, playbook.yml
EXPECTED RESULTS

Clean error message with offending lines:

ERROR! this task 'apk' has extra params, which is only allowed in the following modules: command, win_command, shell, win_shell, script, include, include_vars, include_tasks, include_role, import_tasks, import_role, add_host, group_by, set_fact, raw, meta

The error appears to have been in '/home/diafour/Projects/dapp/ansible-test/debug-playbook.yml': line 6, column 7, but may
be elsewhere in the file depending on the exact syntax problem.

The offending line appears to be:

  tasks:
    - apk: msgfd
      ^ here
ACTUAL RESULTS
devel branch:

Double output

ERROR! this task 'apk' has extra params, which is only allowed in the following modules: command, win_command, shell, win_shell, script, include, include_vars, include_tasks, include_role, import_tasks, import_role, add_host, group_by, set_fact, raw, meta

The error appears to have been in '/home/diafour/Projects/dapp/ansible-test/debug-playbook.yml': line 6, column 7, but may
be elsewhere in the file depending on the exact syntax problem.

The offending line appears to be:

  tasks:
    - apk: msgfd
      ^ here


The error appears to have been in '/home/diafour/Projects/dapp/ansible-test/debug-playbook.yml': line 6, column 7, but may
be elsewhere in the file depending on the exact syntax problem.

The offending line appears to be:

  tasks:
    - apk: msgfd
      ^ here

version 2.4 (git checkout stable-2.4)

Triple output

ERROR! this task 'apk' has extra params, which is only allowed in the following modules: command, win_command, shell, win_shell, script, include, include_vars, include_tasks, include_role, import_tasks, import_role, add_host, group_by, set_fact, raw, meta

The error appears to have been in '/home/diafour/Projects/dapp/ansible-test/debug-playbook.yml': line 6, column 7, but may
be elsewhere in the file depending on the exact syntax problem.

The offending line appears to be:

  tasks:
    - apk: msgfd
      ^ here


The error appears to have been in '/home/diafour/Projects/dapp/ansible-test/debug-playbook.yml': line 6, column 7, but may
be elsewhere in the file depending on the exact syntax problem.

The offending line appears to be:

  tasks:
    - apk: msgfd
      ^ here

exception type: <class 'ansible.errors.AnsibleParserError'>
exception: this task 'apk' has extra params, which is only allowed in the following modules: command, win_command, shell, win_shell, script, include, include_vars, include_tasks, include_role, import_tasks, import_role, add_host, group_by, set_fact, raw, meta

The error appears to have been in '/home/diafour/Projects/dapp/ansible-test/debug-playbook.yml': line 6, column 7, but may
be elsewhere in the file depending on the exact syntax problem.

The offending line appears to be:

  tasks:
    - apk: msgfd
      ^ here

@ansibot
Copy link
Contributor

ansibot commented Feb 28, 2018

Files identified in the description:
None

If these files are inaccurate, please update the component name section of the description or use the !component bot command.

click here for bot help

@ansibot ansibot added affects_2.4 This issue/PR affects Ansible v2.4 bug_report needs_triage Needs a first human triage before being processed. python3 support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Feb 28, 2018
@diafour
Copy link
Contributor Author

diafour commented Feb 28, 2018

!component +lib/ansible/errors/init.py
!component +lib/ansible/parsing/mod_args.py
!component +lib/ansible/playbook/task.py

@ansibot
Copy link
Contributor

ansibot commented Feb 28, 2018

Files identified in the description:

If these files are inaccurate, please update the component name section of the description or use the !component bot command.

click here for bot help

@ansibot
Copy link
Contributor

ansibot commented Feb 28, 2018

Files identified in the description:

If these files are inaccurate, please update the component name section of the description or use the !component bot command.

click here for bot help

@jborean93 jborean93 removed the needs_triage Needs a first human triage before being processed. label Feb 28, 2018
@alikins
Copy link
Contributor

alikins commented Mar 1, 2018

The mod_args.py and task.py cases need some tweaking, since currently they catch AnsibleParserError and then raises a new AnsibleParserError() with additional args, so the final displayed exception's orig_exc has two or more instances of the same exception type with the same message. And when displayed as shown, that doesn't make a ton of sense.

@alikins alikins self-assigned this Mar 1, 2018
@ansibot ansibot added the bug This issue/PR relates to a bug. label Mar 1, 2018
@alikins
Copy link
Contributor

alikins commented Mar 1, 2018

For cases like:

https://github.com/ansible/ansible/blob/devel/lib/ansible/playbook/task.py#L187

... guess there is no particular reason to re raise a new AnsibleParserError in those cases.
As far as I can tell, we could either just do a plain (re) raise, or if we need to set/update the AnsibleParserError._obj we can do it the original Exception instance and (re) raise

try:
    whatever()
except AnsibleParserError as e:
    # if we don't need to add/change the AnsibleError._obj, just re raise
    raise

try:
    whatever(some_thing_related_to_some_yaml_object)
except AnsibleParserError as e:
    # if whatever() doesn't specify obj=some_yaml_obj when it raises AnsibleParserError()
    # we can just set/update it here and re raise
    e._obj = some_yaml_obj
    raise

@ansibot ansibot removed the bug_report label Mar 1, 2018
@abadger abadger removed the python3 label Mar 19, 2018
bcoca pushed a commit that referenced this issue Mar 19, 2018
* Fix redundant yaml error blurbs on ModArgs parse errors

Some of the AnsibleParserErrors from parsing.mod_args
are created with the obj=some_yaml_ds options but
some are not.

If they were, we don't want to add another yaml_ds to
it, because that will result in double yaml error blurbs.
And since we dont need to add info, we can just re raise it.

But if there is no ._obj, add it here so we get the extra
detail in the error message (see issue #14790) and raise
a new AnsibleParserError instance.

Fixes #36848

* cleanup existing test_tasks pep8/sanity issues
nitzmahone pushed a commit that referenced this issue Mar 29, 2018
* Fix redundant yaml error blurbs on ModArgs parse errors

Some of the AnsibleParserErrors from parsing.mod_args
are created with the obj=some_yaml_ds options but
some are not.

If they were, we don't want to add another yaml_ds to
it, because that will result in double yaml error blurbs.
And since we dont need to add info, we can just re raise it.

But if there is no ._obj, add it here so we get the extra
detail in the error message (see issue #14790) and raise
a new AnsibleParserError instance.

Fixes #36848

* cleanup existing test_tasks pep8/sanity issues

(cherry picked from commit e166946)
@ansible ansible locked and limited conversation to collaborators Apr 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.4 This issue/PR affects Ansible v2.4 bug This issue/PR relates to a bug. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants