Var does not exist exception not being caught

[jpmmcneill]

Describe the bug

Var does not exist exception not being caught when var being directly referenced in a loop

Steps To Reproduce

For a given example model
Example 1

select {{ var('var_that_doesnt_exist') }} as example_field

will raise an exception: Required var 'var_that_doesnt_exist' not found in config:,
followed by a list of all the vars supplied to that model.

Example 2

{% for items in var('var_that_doesnt_exist') %}
select {{loop.index}} as example_field_{{loop.index}}
{% endfor %}

will raise an exception: 'NoneType' object is not iterable. This is typically found in YAML config issues and such can be confusing in many cases as it's not a typical var style exception.

Expected behavior

Examples 1 and 2 to raise the same error (Required var 'var_that_doesnt_exist' not found in config: etc). Example 2 is currently failing during the dbt compile.

I'm happy to fix this up if someone points me in the right direction!

System information

Which database are you using dbt with?

• postgres
• redshift
• bigquery
• snowflake
• other (specify: ____________)

The output of dbt --version:

installed version: 0.18.1
   latest version: 0.19.1

Your version of dbt is out of date! You can find instructions for upgrading here:
https://docs.getdbt.com/docs/installation

Plugins:
  - snowflake: 0.18.1
  - bigquery: 0.18.1
  - postgres: 0.18.1
  - redshift: 0.18.1

The operating system you're using: Mac

The output of python --version: Python 3.7.4

[jtcohen6]

Hey @jpmmcneill, nice catch, and thanks for opening this issue! I'm all for improving this error message, since Jinja errors like 'NoneType' object is not iterable are just not very helpful at all.

Here's where that exception should be raised:

https://github.com/fishtown-analytics/dbt/blob/26fb58bd1b08218781b182918f5ed4dec3f735d9/core/dbt/context/base.py#L134-L140

https://github.com/fishtown-analytics/dbt/blob/26fb58bd1b08218781b182918f5ed4dec3f735d9/core/dbt/context/base.py#L153-L159

Even though the code in your reproduction case definitely goes down the else in that conditional, return self.get_missing_var(var_name) is not appropriately raising the compilation error at parse time—it's returning None instead, allowing the code path to continue and wind up with the Jinja not iterable error.

If instead I move the get_missing_var logic to live directly inside the __call__ conditional code:

    def __call__(self, var_name, default=_VAR_NOTSET):
        if self.has_var(var_name):
            return self.get_rendered_var(var_name)
        elif default is not self._VAR_NOTSET:
            return default
        else:
            dct = {k: self._merged[k] for k in self._merged}
            pretty_vars = json.dumps(dct, sort_keys=True, indent=4)
            msg = self.UndefinedVarError.format(
                var_name, self.node_name, pretty_vars
            )
            raise_compiler_error(msg, self._node)

I manage to get:

Encountered an error:
Compilation Error in model my_model (models/my_model.sql)
  Required var 'var_that_doesnt_exist' not found in config:
  Vars supplied to my_model = {}

I'm not enough of a python whiz to know why it works when stepping through this logic directly, and it doesn't work when calling a function with equivalent logic. I tried removing just the return in front of self.get_missing_var(var_name) with no luck.

In any case, I'd definitely welcome a fix for this! :)

[jpmmcneill]

Thanks @jtcohen6 for all the detail! Very much appreciate the pointers 😃

Is there a document that recommends a good way of doing local dev on dbt to test this kind of thing? In my company we have a dockerised CLI dbt so it'd be v awkward to switch dbt to a local version.

Would it be worth it for me to set up a jaffle shop that runs off some local DBT version?

[jtcohen6]

@jpmmcneill Yes! Check out the contributing guide. I'd recommend cloning and installing dbt from source in a virtualenv, though it's also possible to run it from docker.

[jpmmcneill]

🙏 thank you! I'll update here if I have a PR ready to go

[jpmmcneill]

Hey @jtcohen6, I think I've narrowed down the behaviour

It's a bit of a weird one - basically when that self.get_missing_var() gets called, self is actually a ParseVar class.

I'm not sure of the order of operations i'm guessing that it's simply that this is during the compile stage of DBT and is desired behaviour. However, that class actually has a get_missing_var argument itself so it's overriding the class function as you shared in context/base.py:

https://github.com/fishtown-analytics/dbt/blob/8ac5cdd2e1b5de14b6e341188a2d3498af305687/core/dbt/context/providers.py#L572:L575

Getting rid of this seems to get everything working for me (ie. get more verbose errors showing up - I have tested this in jaffle_shop). I have no context as to why this odd return None function was added here so there might be an some conditional logic to make sure that the original case isn't being blown out of the water here.

#3358

[jtcohen6]

Thanks for your work here @jpmmcneill! The failing unit test on that PR is indeed very informative, since it points us here:

https://github.com/fishtown-analytics/dbt/blob/8ac5cdd2e1b5de14b6e341188a2d3498af305687/test/unit/test_context.py#L97-L99

It actually looks like we made an opinionated choice about this a few years ago: #1429

I mostly agree with the rationale there, with the big caveat that returning None for missing variables can cause Jinja type errors that are raised at parsing. This issue is very similar to the discussion over in #3353, where we slowly shuffled toward a different tack: Raise a more helpful error message (i.e. one that says where this error is being raised) by catching a Jinja TypeError and returning it along with the node name.

So I see two potential approaches:

1. Raise compilation error at parse time for all missing vars. Pro: really helpful error message. Con: bad var calls in disabled models can brick your run. (We can't know which models are disabled until all configs have been processed.)
2. Keep returning None for missing vars at parse time, but catch Jinja type errors. Pro: most similar to current behavior, doesn't revert previous choices around var-error-at-runtime. Con: significantly less helpful error message (though still more helpful than status quo).

What do you think? dbt is a different beast today than it was two years ago. Does it make sense for dbt to raise more aggressive errors about missing variables, wherever and whenever it may find them?

[jpmmcneill]

Hey @jtcohen6 sorry for slow response on this one. I'm not going quiet deliberately, i'll try to follow up with some context of how I'm seeing users getting this issue (hint - i'm developing packages)

[github-actions]

This issue has been marked as Stale because it has been open for 180 days with no activity. If you would like the issue to remain open, please remove the stale label or comment on the issue, or it will be closed in 7 days.