Repository: dbt-labs/dbt-core
Pull request: Add a better error for undefined macros
resolves #2999 

<!---
  Include the number of the issue addressed by this PR above if applicable.
  PRs for code changes without an associated issue *will not be merged*.
  See CONTRIBUTING.md for more information.

  Example:
    resolves #1234
-->


### Description

Adds a new `UndefinedMacroException` and raises this when an undefined macro is caught.


### Checklist
 - [X] I have signed the [CLA](https://docs.getdbt.com/docs/contributor-license-agreements)
 - [X] I have run this code in development and it appears to resolve the stated issue
 - [X] This PR includes tests, or tests are not required/relevant for this PR
 - [X] I have updated the `CHANGELOG.md` and added information about my change to the "dbt next" section.


Problem statement:
Raise a warning for required, but uninstalled packages
### Describe the feature
The error message for an uninstalled package comes straight out of the Jinja library:
```
Compilation Error in analysis scratch (analysis/scratch.sql)
    'dbt_utils' is undefined
```

This error message _is_ strictly true, but I think it might be worth taking the time to write a more explicit warning message for uninstalled packages

```
Warning: The following packages are listed as required in `packages.yml` but have not been installed
- dbt_utils

Run `dbt deps` to install these packages
```

Since we have the expected packages in `packages.yml` and the installed packages in `dbt_modules`, this should be possible, right? And not that hard?

Note: I've only got it as a warning so that the places where the package is referenced still causes an error, but I'm open to having this as an error instead

### Describe alternatives you've considered
Keep as is

### Who will this benefit?
Confused users

### Are you interested in contributing this feature?
No ty :)

Hints from issue discussion:
Love it. I'll take a look at some of the code paths involved, and see if this can be a good first issue
FYI, I finally stumbled upon this issue when googling this error message, so yes, updated error message would be very much appreciated.
The message for this is indeed coming from Jinja. The simplest version could just raise a more helpful compilation exception when Jinja returns an `UndefinedError` here:
https://github.com/fishtown-analytics/dbt/blob/749f87397ec1e0a270b2e09bd8dbeb71862fdb81/core/dbt/clients/jinja.py#L505-L506

For instance:
```python
    except jinja2.exceptions.UndefinedError as e:
        raise CompilationException(
            f'{e}. This can happen when calling a macro that is missing. ' \
            'Do you need to install a package dependency with `dbt deps`?'
        , node) from e
```
```
Encountered an error:
Runtime Error
  Compilation Error in model model_b (models/model_b.sql)
    'dbt_utils' is undefined. This can happen when calling a macro that is missing. Do you need to install a package dependency with `dbt deps`?
```

The downside of an approach this simple is that it isn't possible to distinguish between an undefined value that corresponds to a missing namespace/package, versus just a typo in the macro call (`{{ my_marco() }}` when you've defined `{% macro my_macro() %}`). So the message has to be a bit vague. The upside is that we don't need to make the Jinja context aware of the information in `packages.yml`.

I'm going to mark this a good first issue! And a nice chance to add a new [dbt exception](https://github.com/fishtown-analytics/dbt/blob/develop/core/dbt/exceptions.py) :)
@jtcohen6 I'd love to jump onto this one in the next day or so.

Just to clarify, regarding:

> I'm going to mark this a good first issue! And a nice chance to add a new dbt exception :)

Will catching and reraising the exception be enough, or do I also need to modify `exceptions.py`?
@jaypeedevlin Nice! Would love to have you contribute this :)

I think a new exception in `exceptions.py` is warranted, to wrap the error message above (rather than burying it deep in `clients/jinja.py`), and to name that we're doing something a little funky: `PossiblyUninstalledPackageError`?

After rereading this issue, the _absolute best_ version of this probably does the thing Claire mentioned in the original description, and checks to (as a very first step) to see if the contents of `dbt_modules` matches what's specified in `packages.yml`. That would involve touching more code-paths than this approach, which I think is good enough for 90% of times users see this `UndefinedError`.
@jaypeedevlin I don't know why I thought this would be so tricky! At the prompting of your PR comment, I took a look down this code path, and I think it could be as simple as sticking a check right in `load_dependencies`:
https://github.com/fishtown-analytics/dbt/blob/1f33b6a74a614aa346824f16c5629e4d910d2106/core/dbt/config/runtime.py#L327-L334

```python
    def load_dependencies(self) -> Mapping[str, 'RuntimeConfig']:
        if self.dependencies is None:
            all_projects = {self.project_name: self}
            internal_packages = get_include_paths(self.credentials.type)
            # raise exception if fewer installed packages than specified packages
            package_dirs = self._get_project_directories()
            count_packages_specified = len(self.packages.packages)
            count_packages_installed = len(tuple(package_dirs))
            if count_packages_specified > count_packages_installed:
                raise_compiler_error(
                    f'dbt found {count_packages_specified} package(s) specified in packages.yml, '
                    f'but only {count_packages_installed} package(s) installed in {self.modules_path}. '
                    f'Run "dbt deps" to install package dependencies.'
                )
            project_paths = itertools.chain(internal_packages, package_dirs)
```

We can't match on names, since `self.packages.packages` is just the info from `packages.yml` (which may or may not match the package name, if it's a git repo name or local directory). We also can't check lengths for perfect equality, since it's possible that `count_packages_installed` will be greater than `count_packages_installed`, if the specified packages have package dependencies of their own. But I'd say this is a _lot_ better than nothing, and I'd be excited about pairing it with the kinder `UndefinedMacroException` you've got over in #3343.

Commit message: Fix issue with null packages

Patch to review:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index f3ebcf1a5c3..6c15c62f1d5 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,7 @@
 - Fix references to macros with package names when rendering schema tests ([#3324](https://github.com/fishtown-analytics/dbt/issues/3324), [#3345](https://github.com/fishtown-analytics/dbt/pull/3345)
 - Stop clobbering default keyword arguments for jinja test definitions ([#3329](https://github.com/fishtown-analytics/dbt/issues/3329), [#3340](https://github.com/fishtown-analytics/dbt/pull/3340))
 - Update the snowflake adapter to only comment on a column if it exists when using the persist_docs config ([#3039](https://github.com/fishtown-analytics/dbt/issues/3039), [#3149](https://github.com/fishtown-analytics/dbt/pull/3149))
+- Add a better error messages for undefined macros and when there are less packages installed than specified in `packages.yml`. ([#2999](https://github.com/fishtown-analytics/dbt/issues/2999))
 
 
 ### Under the hood
@@ -16,6 +17,7 @@ Contributors:
 - [@panasenco](https://github.com/panasenco) ([#3315](https://github.com/fishtown-analytics/dbt/pull/3315))
 - [@peiwangdb](https://github.com/peiwangdb) ([#3344](https://github.com/fishtown-analytics/dbt/pull/3344))
 - [@elikastelein](https://github.com/elikastelein) ([#3149](https://github.com/fishtown-analytics/dbt/pull/3149))
+- [@jaypeedevlin](https://github.com/jaypeedevlin) ([#2999](https://github.com/fishtown-analytics/dbt/issues/2999))
 
 ## dbt 0.20.0b1 (May 03, 2021)
 
diff --git a/core/dbt/clients/jinja.py b/core/dbt/clients/jinja.py
index 19004fec7c4..ddc2b92936d 100644
--- a/core/dbt/clients/jinja.py
+++ b/core/dbt/clients/jinja.py
@@ -29,7 +29,8 @@
 from dbt.contracts.graph.parsed import ParsedSchemaTestNode
 from dbt.exceptions import (
     InternalException, raise_compiler_error, CompilationException,
-    invalid_materialization_argument, MacroReturn, JinjaRenderingException
+    invalid_materialization_argument, MacroReturn, JinjaRenderingException,
+    UndefinedMacroException
 )
 from dbt import flags
 from dbt.logger import GLOBAL_LOGGER as logger  # noqa
@@ -518,7 +519,7 @@ def catch_jinja(node=None) -> Iterator[None]:
         e.translated = False
         raise CompilationException(str(e), node) from e
     except jinja2.exceptions.UndefinedError as e:
-        raise CompilationException(str(e), node) from e
+        raise UndefinedMacroException(str(e), node) from e
     except CompilationException as exc:
         exc.add_node(node)
         raise
diff --git a/core/dbt/config/runtime.py b/core/dbt/config/runtime.py
index c914766bb91..be51611d4a0 100644
--- a/core/dbt/config/runtime.py
+++ b/core/dbt/config/runtime.py
@@ -328,10 +328,19 @@ def load_dependencies(self) -> Mapping[str, 'RuntimeConfig']:
         if self.dependencies is None:
             all_projects = {self.project_name: self}
             internal_packages = get_include_paths(self.credentials.type)
-            project_paths = itertools.chain(
-                internal_packages,
-                self._get_project_directories()
-            )
+            # raise exception if fewer installed packages than in packages.yml
+            package_dirs = self._get_project_directories()
+            count_packages_specified = len(self.packages.get('packages', 0))
+            count_packages_installed = len(tuple(package_dirs))
+            if count_packages_specified > count_packages_installed:
+                raise_compiler_error(
+                    f'dbt found {count_packages_specified} package(s) '
+                    f'specified in packages.yml, but only '
+                    f'{count_packages_installed} package(s) installed '
+                    f'in {self.modules_path}. Run "dbt deps" to '
+                    f'install package dependencies.'
+                )
+            project_paths = itertools.chain(internal_packages, package_dirs)
             for project_name, project in self.load_projects(project_paths):
                 if project_name in all_projects:
                     raise_compiler_error(
diff --git a/core/dbt/exceptions.py b/core/dbt/exceptions.py
index 82cd9267bc4..7fb07934127 100644
--- a/core/dbt/exceptions.py
+++ b/core/dbt/exceptions.py
@@ -289,6 +289,15 @@ class JinjaRenderingException(CompilationException):
     pass
 
 
+class UndefinedMacroException(CompilationException):
+
+    def __str__(self, prefix='! ') -> str:
+        msg = super().__str__(prefix)
+        return f'{msg}. This can happen when calling a macro that does ' \
+            'not exist. Check for typos and/or install package dependencies ' \
+            'with "dbt deps".'
+
+
 class UnknownAsyncIDException(Exception):
     CODE = 10012
     MESSAGE = 'RPC server got an unknown async ID'
diff --git a/test/integration/029_docs_generate_tests/test_docs_generate.py b/test/integration/029_docs_generate_tests/test_docs_generate.py
index e2471155472..afba151e654 100644
--- a/test/integration/029_docs_generate_tests/test_docs_generate.py
+++ b/test/integration/029_docs_generate_tests/test_docs_generate.py
@@ -171,6 +171,7 @@ def run_and_generate(self, extra=None, seed_count=1, model_count=2, alternate_db
 
         vars_arg = '--vars={{test_schema: {}}}'.format(self.unique_schema())
 
+        self.run_dbt(["deps"])
         self.assertEqual(len(self.run_dbt(["seed", vars_arg])), seed_count)
         self.assertEqual(len(self.run_dbt(['run', vars_arg])), model_count)
         os.remove(normalize('target/manifest.json'))
@@ -3284,6 +3285,7 @@ def models(self):
         return self.dir("trivial_models")
 
     def run_and_generate(self):
+        self.run_dbt(["deps"])
         self.assertEqual(len(self.run_dbt(['run'])), 1)
         os.remove(normalize('target/manifest.json'))
         os.remove(normalize('target/run_results.json'))
diff --git a/test/integration/033_event_tracking_test/test_events.py b/test/integration/033_event_tracking_test/test_events.py
index f09b7ce611e..e4a0277706a 100644
--- a/test/integration/033_event_tracking_test/test_events.py
+++ b/test/integration/033_event_tracking_test/test_events.py
@@ -46,6 +46,7 @@ def run_event_test(
         expect_pass=True,
         expect_raise=False
     ):
+        self.run_dbt(["deps"])
         track_fn.reset_mock()
 
         project_id = hashlib.md5(
@@ -541,6 +542,7 @@ def test__postgres_event_tracking_model_error(self):
     @use_profile("postgres")
     def test__postgres_event_tracking_tests(self):
         # TODO: dbt does not track events for tests, but it should!
+        self.run_dbt(["deps"])
         self.run_dbt(["run", "--model", "example", "example_2"])
 
         expected_calls = [
diff --git a/test/integration/100_rpc_test/test_rpc.py b/test/integration/100_rpc_test/test_rpc.py
index 9487c8bb739..9005db56749 100644
--- a/test/integration/100_rpc_test/test_rpc.py
+++ b/test/integration/100_rpc_test/test_rpc.py
@@ -722,8 +722,8 @@ def test_invalid_requests_postgres(self):
             task_tags={'some_tag': True, 'another_tag': 'blah blah blah'}
         ).json()
         error_data = self.assertIsErrorWith(data, 10004, 'Compilation Error', {
-            'type': 'CompilationException',
-            'message': "Compilation Error in rpc mymodel (from remote system)\n  'reff' is undefined",
+            'type': 'UndefinedMacroException',
+            'message': "Compilation Error in rpc mymodel (from remote system)\n  'reff' is undefined. This can happen when calling a macro that does not exist. Check for typos and/or install package dependencies with \"dbt deps\".",
             'compiled_sql': None,
             'raw_sql': 'select * from {{ reff("nonsource_descendant") }}',
             'tags': {'some_tag': True, 'another_tag': 'blah blah blah'}


Code review comments:
diff_hunk: @@ -328,10 +328,19 @@ def load_dependencies(self) -> Mapping[str, 'RuntimeConfig']:
         if self.dependencies is None:
             all_projects = {self.project_name: self}
             internal_packages = get_include_paths(self.credentials.type)
-            project_paths = itertools.chain(
-                internal_packages,
-                self._get_project_directories()
-            )
+            # raise exception if fewer installed packages than in packages.yml
+            package_dirs = self._get_project_directories()
+            count_packages_specified = len(self.packages.get('packages', 0))
+            count_packages_installed = len(tuple(package_dirs))
+            if count_packages_specified > count_packages_installed:
+                raise_compiler_error(
+                    f'dbt found {count_packages_specified} package(s) '
+                    f'specified in packages.yml, but only '
+                    f'{count_packages_installed} package(s) installed '
+                    f'in {self.modules_path}. Run "dbt deps" to '
+                    f'install package dependencies.'
+                )
+            project_paths = itertools.chain(internal_packages, package_dirs)
path: core/dbt/config/runtime.py
text: @user1:
...if we just need to call it again down here
```suggestion
            project_paths = itertools.chain(
                internal_packages,
                self._get_project_directories()
            )
```

diff_hunk: @@ -328,10 +328,19 @@ def load_dependencies(self) -> Mapping[str, 'RuntimeConfig']:
         if self.dependencies is None:
             all_projects = {self.project_name: self}
             internal_packages = get_include_paths(self.credentials.type)
-            project_paths = itertools.chain(
-                internal_packages,
-                self._get_project_directories()
-            )
+            # raise exception if fewer installed packages than in packages.yml
+            package_dirs = self._get_project_directories()
+            count_packages_specified = len(self.packages.get('packages', 0))
+            count_packages_installed = len(tuple(package_dirs))
path: core/dbt/config/runtime.py
text: @user1:
Ah, I think (at least some of) the failing tests are because `_get_project_directories()` returns a generator object, so we can only access it once. No sense setting it to a var...
```suggestion
            count_packages_specified = len(self.packages.packages)
            count_packages_installed = len(tuple(self._get_project_directories()))
```