Repository: facebookresearch/hydra

Issue:
Improve error on incorrect command line argument or composition problems
This is very verbose:
```
$ python examples/tutorial/5_composition/my_app.py foo=bar
Traceback (most recent call last):
  File "examples/tutorial/5_composition/my_app.py", line 11, in <module>
    my_app()
  File "c:\users\omry\pycharmprojects\hydra\hydra\main.py", line 31, in decorated_main
    strict=strict,
  File "c:\users\omry\pycharmprojects\hydra\hydra\_internal\utils.py", line 194, in run_hydra
    overrides=args.overrides,
  File "c:\users\omry\pycharmprojects\hydra\hydra\_internal\hydra.py", line 96, in run
    config_file=config_file, overrides=overrides, with_log_configuration=True
  File "c:\users\omry\pycharmprojects\hydra\hydra\_internal\hydra.py", line 393, in compose_config
    config_file=config_file, overrides=overrides, strict=strict
  File "c:\users\omry\pycharmprojects\hydra\hydra\_internal\config_loader.py", line 93, in load_configuration
    cfg.merge_with_dotlist(remaining_overrides)
  File "C:\tools\miniconda3\lib\site-packages\omegaconf\basecontainer.py", line 154, in merge_with_dotlist
    self.update_node(key, value)
  File "C:\tools\miniconda3\lib\site-packages\omegaconf\basecontainer.py", line 179, in update_node
    root[last] = value
  File "C:\tools\miniconda3\lib\site-packages\omegaconf\dictconfig.py", line 100, in __setitem__
    self._validate_access(key)
  File "C:\tools\miniconda3\lib\site-packages\omegaconf\dictconfig.py", line 302, in _validate_access
    raise AttributeError(msg)
AttributeError: Accessing unknown key in a struct : foo
```

Discussion:

Patch:
diff --git a/hydra/_internal/config_loader_impl.py b/hydra/_internal/config_loader_impl.py
index 0ebd5576b5..174ee10b6a 100644
--- a/hydra/_internal/config_loader_impl.py
+++ b/hydra/_internal/config_loader_impl.py
@@ -6,11 +6,13 @@ import copy
 from typing import List, Optional, Tuple
 
 from omegaconf import DictConfig, ListConfig, OmegaConf, open_dict
+from omegaconf.errors import OmegaConfBaseException
 
 from hydra._internal.config_repository import ConfigRepository
 from hydra.core.config_loader import ConfigLoader, LoadTrace
 from hydra.core.config_search_path import ConfigSearchPath
 from hydra.core.config_store import ConfigStore
+from hydra.core.errors import HydraException
 from hydra.core.object_type import ObjectType
 from hydra.core.utils import JobRuntime, get_overrides_dirname, split_key_val
 from hydra.errors import MissingConfigException
@@ -104,7 +106,12 @@ class ConfigLoaderImpl(ConfigLoader):
         # Merge all command line overrides after enabling strict flag
         all_consumed = consumed + consumed_free_job_defaults
         remaining_overrides = [x for x in overrides if x not in all_consumed]
-        cfg.merge_with_dotlist(remaining_overrides)
+        try:
+            merged = OmegaConf.merge(cfg, OmegaConf.from_dotlist(remaining_overrides))
+            assert isinstance(merged, DictConfig)
+            cfg = merged
+        except OmegaConfBaseException as ex:
+            raise HydraException("Error merging overrides") from ex
 
         remaining = consumed + consumed_free_job_defaults + remaining_overrides
 
@@ -323,34 +330,36 @@ class ConfigLoaderImpl(ConfigLoader):
     def _merge_config(
         self, cfg: DictConfig, family: str, name: str, required: bool
     ) -> DictConfig:
+        try:
+            if family != "":
+                new_cfg = "{}/{}".format(family, name)
+            else:
+                new_cfg = name
 
-        if family != "":
-            new_cfg = "{}/{}".format(family, name)
-        else:
-            new_cfg = name
-
-        loaded_cfg, _ = self._load_config_impl(new_cfg)
-        if loaded_cfg is None:
-            if required:
-                if family == "":
-                    msg = "Could not load {}".format(new_cfg)
-                    raise MissingConfigException(msg, new_cfg)
-                else:
-                    options = self.get_group_options(family)
-                    if options:
-                        msg = "Could not load {}, available options:\n{}:\n\t{}".format(
-                            new_cfg, family, "\n\t".join(options)
-                        )
-                    else:
+            loaded_cfg, _ = self._load_config_impl(new_cfg)
+            if loaded_cfg is None:
+                if required:
+                    if family == "":
                         msg = "Could not load {}".format(new_cfg)
-                    raise MissingConfigException(msg, new_cfg, options)
-            else:
-                return cfg
+                        raise MissingConfigException(msg, new_cfg)
+                    else:
+                        options = self.get_group_options(family)
+                        if options:
+                            msg = "Could not load {}, available options:\n{}:\n\t{}".format(
+                                new_cfg, family, "\n\t".join(options)
+                            )
+                        else:
+                            msg = "Could not load {}".format(new_cfg)
+                        raise MissingConfigException(msg, new_cfg, options)
+                else:
+                    return cfg
 
-        else:
-            ret = OmegaConf.merge(cfg, loaded_cfg)
-            assert isinstance(ret, DictConfig)
-            return ret
+            else:
+                ret = OmegaConf.merge(cfg, loaded_cfg)
+                assert isinstance(ret, DictConfig)
+                return ret
+        except OmegaConfBaseException as ex:
+            raise HydraException(f"Error merging {family}={name}") from ex
 
     def _merge_defaults(
         self,
diff --git a/hydra/_internal/utils.py b/hydra/_internal/utils.py
index b0d6f37bb5..75a994d08e 100644
--- a/hydra/_internal/utils.py
+++ b/hydra/_internal/utils.py
@@ -7,12 +7,15 @@ import os
 import sys
 import warnings
 from os.path import dirname, join, normpath, realpath
+from traceback import print_exc
 from typing import Any, Callable, List, Optional, Sequence, Tuple, Type, Union
 
 from omegaconf import DictConfig, OmegaConf, _utils, read_write
+from omegaconf.errors import OmegaConfBaseException
 
 from hydra._internal.config_search_path_impl import ConfigSearchPathImpl
 from hydra.core.config_search_path import ConfigSearchPath
+from hydra.core.errors import HydraException
 from hydra.core.utils import get_valid_filename, split_config_path
 from hydra.types import ObjectConf, TaskFunction
 
@@ -177,6 +180,23 @@ def create_config_search_path(search_path_dir: Optional[str]) -> ConfigSearchPat
     return search_path
 
 
+def run_and_report(func: Any) -> None:
+    try:
+        func()
+    except (HydraException, OmegaConfBaseException) as ex:
+        if "HYDRA_FULL_ERROR" in os.environ and os.environ["HYDRA_FULL_ERROR"] == "1":
+            print_exc()
+        else:
+            sys.stderr.write(str(ex) + "\n")
+            if ex.__cause__ is not None:
+                sys.stderr.write(str(ex.__cause__) + "\n")
+
+            sys.stderr.write(
+                "Set the environment variable HYDRA_FULL_ERROR=1 for a complete stack trace\n"
+            )
+        sys.exit(1)
+
+
 def run_hydra(
     args_parser: argparse.ArgumentParser,
     task_function: TaskFunction,
@@ -223,25 +243,35 @@ def run_hydra(
         if num_commands == 0:
             args.run = True
         if args.run:
-            hydra.run(
-                config_name=config_name,
-                task_function=task_function,
-                overrides=args.overrides,
+            run_and_report(
+                lambda: hydra.run(
+                    config_name=config_name,
+                    task_function=task_function,
+                    overrides=args.overrides,
+                )
             )
         elif args.multirun:
-            hydra.multirun(
-                config_name=config_name,
-                task_function=task_function,
-                overrides=args.overrides,
+            run_and_report(
+                lambda: hydra.multirun(
+                    config_name=config_name,
+                    task_function=task_function,
+                    overrides=args.overrides,
+                )
             )
         elif args.cfg:
-            hydra.show_cfg(
-                config_name=config_name, overrides=args.overrides, cfg_type=args.cfg
+            run_and_report(
+                lambda: hydra.show_cfg(
+                    config_name=config_name, overrides=args.overrides, cfg_type=args.cfg
+                )
             )
         elif args.shell_completion:
-            hydra.shell_completion(config_name=config_name, overrides=args.overrides)
+            run_and_report(
+                lambda: hydra.shell_completion(
+                    config_name=config_name, overrides=args.overrides
+                )
+            )
         else:
-            print("Command not specified")
+            sys.stderr.write("Command not specified\n")
             sys.exit(1)
     finally:
         GlobalHydra.instance().clear()
diff --git a/hydra/core/errors.py b/hydra/core/errors.py
new file mode 100644
index 0000000000..91f907b441
--- /dev/null
+++ b/hydra/core/errors.py
@@ -0,0 +1,5 @@
+# Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved
+
+
+class HydraException(Exception):
+    ...
diff --git a/news/349.feature b/news/349.feature
new file mode 100644
index 0000000000..a2a54bfbf9
--- /dev/null
+++ b/news/349.feature
@@ -0,0 +1,1 @@
+Improve command line and config composition error reporting


Test patch:
diff --git a/tests/test_compose.py b/tests/test_compose.py
index 2f17306f3b..b6c8ca2989 100644
--- a/tests/test_compose.py
+++ b/tests/test_compose.py
@@ -6,6 +6,7 @@ import pytest
 
 from hydra._internal.config_search_path_impl import ConfigSearchPathImpl
 from hydra.core.config_search_path import SearchPathQuery
+from hydra.core.errors import HydraException
 from hydra.core.global_hydra import GlobalHydra
 from hydra.experimental import compose, initialize
 from hydra.test_utils.test_utils import (
@@ -90,7 +91,7 @@ class TestCompose:
         # default strict True, call is unspecified
         overrides.append("fooooooooo=bar")
         with hydra_global_context(config_dir=config_dir, strict=True):
-            with pytest.raises(AttributeError):
+            with pytest.raises(HydraException):
                 compose(config_file, overrides)
 
     def test_strict_failure_call_is_strict(
@@ -103,7 +104,7 @@ class TestCompose:
     ) -> None:
         # default strict false, but call is strict
         with hydra_global_context(config_dir=config_dir, strict=False):
-            with pytest.raises(AttributeError):
+            with pytest.raises(HydraException):
                 compose(config_name=config_file, overrides=overrides, strict=True)
 
         # default strict true, but call is false
diff --git a/tests/test_config_loader.py b/tests/test_config_loader.py
index ecba8c7207..331fa796a3 100644
--- a/tests/test_config_loader.py
+++ b/tests/test_config_loader.py
@@ -10,6 +10,7 @@ from hydra._internal.config_loader_impl import ConfigLoaderImpl
 from hydra._internal.utils import create_config_search_path
 from hydra.core.config_loader import LoadTrace
 from hydra.core.config_store import ConfigStore, ConfigStoreWithProvider
+from hydra.core.errors import HydraException
 from hydra.errors import MissingConfigException
 from hydra.test_utils.test_utils import chdir_hydra_root
 
@@ -151,7 +152,7 @@ class TestConfigLoader:
             cfg.not_here
 
         # Test that bad overrides triggers the KeyError
-        with pytest.raises(AttributeError):
+        with pytest.raises(HydraException):
             config_loader.load_configuration(
                 config_name="compose.yaml", overrides=["f00=ZZZ"], strict=True
             )
@@ -253,7 +254,7 @@ class TestConfigLoader:
             cfg.db.port = "fail"
 
         # verify illegal override is rejected during load
-        with pytest.raises(ValidationError):
+        with pytest.raises(HydraException):
             config_loader.load_configuration(
                 config_name="db/mysql", overrides=["db.port=fail"]
             )
@@ -584,5 +585,5 @@ def test_invalid_plugin_merge(restore_singletons: Any) -> Any:
     cs.store(group="plugin", name="invalid", node=InvalidPlugin, path="plugin")
 
     cl = ConfigLoaderImpl(config_search_path=create_config_search_path(None))
-    with pytest.raises(ValidationError):
+    with pytest.raises(HydraException):
         cl.load_configuration(config_name="config", overrides=["plugin=invalid"])
diff --git a/tests/test_structured_configs_tutorial.py b/tests/test_structured_configs_tutorial.py
index b3bab642f9..8f2518854d 100644
--- a/tests/test_structured_configs_tutorial.py
+++ b/tests/test_structured_configs_tutorial.py
@@ -24,6 +24,14 @@ def test_structured_configs_1_basic_run(tmpdir: Path) -> None:
     assert result.decode("utf-8").rstrip() == "Host: localhost, port: 3306"
 
 
+def run_with_error(cmd: Any) -> str:
+    with Popen(cmd, stdout=PIPE, stderr=PIPE) as p:
+        _stdout, stderr = p.communicate()
+        err = stderr.decode("utf-8").rstrip().replace("\r\n", "\n")
+        assert p.returncode == 1
+    return err
+
+
 def test_structured_configs_1_basic_run_with_override_error(tmpdir: Path) -> None:
     cmd = [
         sys.executable,
@@ -31,14 +39,11 @@ def test_structured_configs_1_basic_run_with_override_error(tmpdir: Path) -> Non
         "hydra.run.dir=" + str(tmpdir),
     ]
 
-    expected = """omegaconf.errors.ConfigAttributeError: Key 'pork' not in 'MySQLConfig'
+    expected = """Key 'pork' not in 'MySQLConfig'
 \tfull_key: pork
 \treference_type=Optional[MySQLConfig]
 \tobject_type=MySQLConfig"""
-    p = Popen(cmd, stdout=PIPE, stderr=PIPE)
-    _stdout, stderr = p.communicate()
-    err = stderr.decode("utf-8").rstrip().replace("\r\n", "\n")
-
+    err = run_with_error(cmd)
     assert re.search(re.escape(expected), err) is not None
 
 
@@ -61,13 +66,12 @@ def test_structured_configs_1_basic_override_type_error(tmpdir: Path) -> None:
         "port=foo",
     ]
 
-    expected = """omegaconf.errors.ValidationError: Value 'foo' could not be converted to Integer
+    expected = """Value 'foo' could not be converted to Integer
 \tfull_key: port
 \treference_type=Optional[MySQLConfig]
 \tobject_type=MySQLConfig"""
-    p = Popen(cmd, stdout=PIPE, stderr=PIPE)
-    _stdout, stderr = p.communicate()
-    err = stderr.decode("utf-8").rstrip().replace("\r\n", "\n")
+
+    err = run_with_error(cmd)
     assert re.search(re.escape(expected), err) is not None