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

Print Jinja error context on UndefinedError #61553

Merged
merged 4 commits into from
Feb 4, 2022

Conversation

jfindlay
Copy link
Contributor

@jfindlay jfindlay commented Jan 31, 2022

What does this PR do?

Fix authored by @frebib.

All other Jinja exceptions raised in salt/utils/templates.py are raised with source file context except for jinja2.exceptions.UndefinedError. This change provides both the line number on which the error occurred in the Jinja template, and also several lines of context around the error in the Jinja template source for UndefinedError.

Previous Behavior

Traces/errors before were vague and ambiguous:

File "/usr/lib/python2.7/dist-packages/salt/utils/templates.py", line 387, in render_jinja_tmpl
  buf=tmplstr)
SaltRenderError: Jinja variable 'str object' has no attribute 'items'

New Behavior

This change gives a line number and multiple lines of context:

File "/usr/lib/python2.7/dist-packages/salt/utils/templates.py", line 387, in render_jinja_tmpl
  tmplstr)
SaltRenderError: Jinja variable 'str object' has no attribute 'items'; line 5

---
# {{ salt.pillar.get('managed_by_salt_message', '') }}
# salt template: {{ source }}

{%- for section in sections %}
  {%- for name, config in section.items() %}    <======================
[{{ name }}]
    {%- for line in config %}
{{ line }}
    {%- endfor %}
  {%- endfor %}
[...]
---

Merge requirements satisfied?

Commits signed with GPG?

Yes

@jfindlay jfindlay requested a review from a team as a code owner January 31, 2022 15:25
@jfindlay jfindlay requested review from Ch3LL and removed request for a team January 31, 2022 15:25
@jfindlay jfindlay force-pushed the jfindlay/SALT-469/jinja-context branch from 5b99401 to 19e3b06 Compare January 31, 2022 15:31
@jfindlay jfindlay changed the title Jfindlay/salt 469/jinja context Print jinja error context on UndefinedError Jan 31, 2022
@jfindlay jfindlay changed the title Print jinja error context on UndefinedError Print Jinja error context on UndefinedError Jan 31, 2022
@jfindlay jfindlay force-pushed the jfindlay/SALT-469/jinja-context branch from 19e3b06 to e3afb28 Compare January 31, 2022 20:46
@jfindlay
Copy link
Contributor Author

@Ch3LL, the test pipeline is searching for the master branch of the kitchen-docker repo. Perhaps that branch was recently deleted?

[2022-01-31T20:50:07.216Z] Fetching https://github.com/test-kitchen/kitchen-docker.git
[2022-01-31T20:50:07.471Z] fatal: Needed a single revision
[2022-01-31T20:50:07.471Z] Revision master does not exist in the repository
[2022-01-31T20:50:07.471Z] https://github.com/test-kitchen/kitchen-docker.git. Maybe you misspelled it?

@jfindlay jfindlay force-pushed the jfindlay/SALT-469/jinja-context branch from e3afb28 to 3f13e31 Compare February 1, 2022 15:25
Ch3LL
Ch3LL previously approved these changes Feb 1, 2022
@Ch3LL Ch3LL added the Phosphorus v3005.0 Release code name and version label Feb 1, 2022
frebib and others added 3 commits February 2, 2022 09:18
This provides both the line number on which the error occurred in the
Jinja template, and also several lines of context around the error in
the Jinja template source.

The one caveat to this change is that if paired with jinja2<2.11 it
could possibly report incorrect line numbers for some errors. This was
fixed in pallets/jinja#1109

Traces/errors before were vague and ambiguous:

    File "/usr/lib/python2.7/dist-packages/salt/utils/templates.py", line 387, in render_jinja_tmpl
      buf=tmplstr)
    SaltRenderError: Jinja variable 'str object' has no attribute 'items'

This gives a line number and multiple lines of context:

    File "/usr/lib/python2.7/dist-packages/salt/utils/templates.py", line 387, in render_jinja_tmpl
      tmplstr)
    SaltRenderError: Jinja variable 'str object' has no attribute 'items'; line 5

    ---
    # {{ salt.pillar.get('managed_by_salt_message', '') }}
    # salt template: {{ source }}

    {%- for section in sections %}
      {%- for name, config in section.items() %}    <======================
    [{{ name }}]
        {%- for line in config %}
    {{ line }}
        {%- endfor %}
      {%- endfor %}
    [...]
    ---

Signed-off-by: Joe Groocock <jgroocock@cloudflare.com>
Add a test to reinforce that salt will provide context when
`jinja2.exceptions.UndefinedError` is raised.
Some functions in salt/utils/templates.py were undocumented.
@github-actions
Copy link

github-actions bot commented Feb 3, 2022

Hi! I'm your friendly PR bot!

You might be wondering what I'm doing commenting here on your PR.

Yes, as a matter of fact, I am...

I'm just here to help us improve the documentation. I can't respond to
questions or anything, but what I can do, I do well!

Okay... so what do you do?

I detect modules that are missing docstrings or "CLI Example" on existing docstrings!
When I was created we had a lot of these. The documentation for these
modules need some love and attention to make Salt better for our users.

So what does that have to do with my PR?

I noticed that in this PR there are some files changed that have some of these
issues. So I'm leaving this comment to let you know your options.

Okay, what are they?

Well, my favorite, is that since you were making changes here I'm hoping that
you would be the most familiar with this module and be able to add some other
examples or fix any of the reported issues.

If I can, then what?

Well, you can either add them to this PR or add them to another PR. Either way is fine!

Well... what if I can't, or don't want to?

That's also fine! We appreciate all contributions to the Salt Project. If you
can't add those other examples, either because you're too busy, or unfamiliar,
or you just aren't interested, we still appreciate the contributions that
you've made already.

Whatever approach you decide to take, just drop a comment here letting us know!

Detected Issues (click me)
Check Known Missing Docstrings...........................................Failed
- hook id: invoke
- duration: 1.51s
- exit code: 1

The function 'wrap_tmpl_func' on 'salt/utils/templates.py' does not have a docstring
The function 'render_jinja_tmpl' on 'salt/utils/templates.py' does not have a docstring
The function 'render_mako_tmpl' on 'salt/utils/templates.py' does not have a docstring
The function 'render_wempy_tmpl' on 'salt/utils/templates.py' does not have a docstring
Found 4 errors


Thanks again!

@Ch3LL Ch3LL merged commit 95fb516 into saltstack:master Feb 4, 2022
@max-arnold
Copy link
Contributor

This is super useful, thanks a lot!

Last year I did some research on how to turn the dreaded

jinja2.exceptions.UndefinedError: 'dict object' has no attribute 'baz'

into (note the myobj variable name)

jinja2.exceptions.UndefinedError: 'dict object' (myobj) has no attribute 'baz'

It turned out to be very hairy: https://gist.github.com/max-arnold/94b0abf877475f3344f6f20135c7b224
I believe it should be handled in Jinja itself, but unfortunately the issue was closed pallets/jinja#313

Your fix solves the problem quite nicely without introducing much complexity

@Ch3LL Do you think these issues could be closed? #57852 #59243

@max-arnold
Copy link
Contributor

@jfindlay Does it print the same error multiple times for you as well?

salt-call state.apply phosphorus.foo
[ERROR   ] Rendering exception occurred
Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/salt-3003rc1+2286.g03015f1d55-py3.8.egg/salt/utils/templates.py", line 469, in render_jinja_tmpl
    output = template.render(**decoded_context)
  File "/usr/lib/python3/dist-packages/jinja2/asyncsupport.py", line 76, in render
    return original_render(self, *args, **kwargs)
  File "/usr/lib/python3/dist-packages/jinja2/environment.py", line 1008, in render
    return self.environment.handle_exception(exc_info, True)
  File "/usr/lib/python3/dist-packages/jinja2/environment.py", line 780, in handle_exception
    reraise(exc_type, exc_value, tb)
  File "/usr/lib/python3/dist-packages/jinja2/_compat.py", line 37, in reraise
    raise value.with_traceback(tb)
  File "<template>", line 6, in <module>
jinja2.exceptions.UndefinedError: 'dict object' has no attribute 'foo'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/salt-3003rc1+2286.g03015f1d55-py3.8.egg/salt/utils/templates.py", line 216, in render_tmpl
    output = render_str(tmplstr, context, tmplpath)
  File "/usr/lib/python3/dist-packages/salt-3003rc1+2286.g03015f1d55-py3.8.egg/salt/utils/templates.py", line 475, in render_jinja_tmpl
    raise SaltRenderError("Jinja variable {}{}".format(exc, out), line, tmplstr)
salt.exceptions.SaltRenderError: Jinja variable 'dict object' has no attribute 'foo'; line 6

---
{% set myobj = {'baz': 'qux'} %}

foo:
  test.configurable_test_state:
    - changes: True
    - comment: {{ myobj['foo'] }}    <======================
---
[CRITICAL] Rendering SLS 'base:phosphorus.foo' failed: Jinja variable 'dict object' has no attribute 'foo'; line 6

---
{% set myobj = {'baz': 'qux'} %}

foo:
  test.configurable_test_state:
    - changes: True
    - comment: {{ myobj['foo'] }}    <======================
---
[WARNING ] Unable to compress state output by ID! Returning output normally.
local:
    Data failed to compile:
----------
    Rendering SLS 'base:phosphorus.foo' failed: Jinja variable 'dict object' has no attribute 'foo'; line 6

---
{% set myobj = {'baz': 'qux'} %}

foo:
  test.configurable_test_state:
    - changes: True
    - comment: {{ myobj['foo'] }}    <======================
---

@OrangeDog
Copy link
Contributor

This doesn't appear to work properly: #62620

@frebib
Copy link
Contributor

frebib commented Sep 6, 2022

@OrangeDog Looking at the report in that issue, I see an attached context but it appears to be the wrong file. I've never personally seen this do the wrong thing, but I could be missing something. I would expect that if the error is from an included file then you'd see that context instead. If we could get a basic reproducer I can fix it

@OrangeDog
Copy link
Contributor

@frebib turns out they're in the same file, OP is just not providing the whole file. It's only the line that's wrong.

@frebib
Copy link
Contributor

frebib commented Sep 6, 2022

Sounds like a Jinja issue. Maybe they're using an old Jinja version

@OrangeDog
Copy link
Contributor

OrangeDog commented Sep 6, 2022

Jinja2: 2.11.3

just upgraded to onedir and there is still the same error.

@frebib
Copy link
Contributor

frebib commented Sep 6, 2022

Curious. Can we get a basic reproducer?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Phosphorus v3005.0 Release code name and version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants