From 42eca879a7b3eef12b3a89a55f639be8d06fe604 Mon Sep 17 00:00:00 2001 From: Jasha <8935917+Jasha10@users.noreply.github.com> Date: Fri, 30 Jul 2021 17:49:00 -0500 Subject: [PATCH 01/18] handle frame in stack trace that doesn't have a module --- hydra/_internal/utils.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hydra/_internal/utils.py b/hydra/_internal/utils.py index 674beabe516..4ce4ed5523f 100644 --- a/hydra/_internal/utils.py +++ b/hydra/_internal/utils.py @@ -248,7 +248,8 @@ def run_and_report(func: Any) -> Any: while end is not None: frame = end.tb_frame mdl = inspect.getmodule(frame) - assert mdl is not None + if mdl is None: + break name = mdl.__name__ if name.startswith("omegaconf."): break From fc1f24fd24bbeb45504194a3d9cf6be3e640e67f Mon Sep 17 00:00:00 2001 From: Jasha <8935917+Jasha10@users.noreply.github.com> Date: Sat, 31 Jul 2021 09:46:37 -0500 Subject: [PATCH 02/18] revert to `assert mdl is not None` (don't use break) --- hydra/_internal/utils.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/hydra/_internal/utils.py b/hydra/_internal/utils.py index 4ce4ed5523f..674beabe516 100644 --- a/hydra/_internal/utils.py +++ b/hydra/_internal/utils.py @@ -248,8 +248,7 @@ def run_and_report(func: Any) -> Any: while end is not None: frame = end.tb_frame mdl = inspect.getmodule(frame) - if mdl is None: - break + assert mdl is not None name = mdl.__name__ if name.startswith("omegaconf."): break From 1d07372560a4456f613dbf6744337c18115836d3 Mon Sep 17 00:00:00 2001 From: Jasha <8935917+Jasha10@users.noreply.github.com> Date: Sat, 31 Jul 2021 10:05:30 -0500 Subject: [PATCH 03/18] stricter typing in `run_and_report` Previously `typing.Any` was used for the type of some Optional tracebacks in `hydra._internal.run_and_report`. This commit makes the typing more precise, using `types.TracebackType` and `Optional[types.TracebackType]` where appropriate. Additionally, a new type assertion is introduced based on feedback from mypy. --- hydra/_internal/utils.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/hydra/_internal/utils.py b/hydra/_internal/utils.py index 674beabe516..10c2e5c68e3 100644 --- a/hydra/_internal/utils.py +++ b/hydra/_internal/utils.py @@ -7,7 +7,7 @@ from dataclasses import dataclass from os.path import dirname, join, normpath, realpath from traceback import print_exc, print_exception -from types import FrameType +from types import FrameType, TracebackType from typing import Any, Callable, List, Optional, Sequence, Tuple, Union from omegaconf.errors import OmegaConfBaseException @@ -223,7 +223,7 @@ def run_and_report(func: Any) -> Any: # It is possible to add additional libraries to sanitize from the bottom later, # maybe even make it configurable. - tb: Any = ex.__traceback__ + tb = ex.__traceback__ search_max = 10 # strip Hydra frames from start of stack # will strip until it hits run_job() @@ -243,7 +243,7 @@ def run_and_report(func: Any) -> Any: sys.exit(1) # strip OmegaConf frames from bottom of stack - end = tb + end: Optional[TracebackType] = tb num_frames = 0 while end is not None: frame = end.tb_frame @@ -276,6 +276,7 @@ class FakeTracebackType: added = added + 1 cur.tb_next = FakeTracebackType() cur = cur.tb_next + assert iter_tb.tb_next is not None iter_tb = iter_tb.tb_next print_exception(etype=None, value=ex, tb=final_tb) # type: ignore From 81e0c75a84a050905aef22520708e00139ee4fc2 Mon Sep 17 00:00:00 2001 From: Jasha <8935917+Jasha10@users.noreply.github.com> Date: Sat, 31 Jul 2021 11:16:37 -0500 Subject: [PATCH 04/18] tests for current run_and_report_behavior --- tests/test_utils.py | 106 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 106 insertions(+) diff --git a/tests/test_utils.py b/tests/test_utils.py index 60e5a16fdc9..6cdb323f1cb 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,14 +1,19 @@ # Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved +import io import os from pathlib import Path +from textwrap import dedent from typing import Any +from unittest.mock import patch from omegaconf import DictConfig, OmegaConf from pytest import mark, raises from hydra import utils +from hydra._internal.utils import run_and_report from hydra.conf import HydraConf, RuntimeConf from hydra.core.hydra_config import HydraConfig +from hydra.test_utils.test_utils import assert_regex_match def test_get_original_cwd(hydra_restore_singletons: Any) -> None: @@ -60,3 +65,104 @@ def test_to_absolute_path_without_hydra( path = str(Path(path)) expected = str(Path(expected).absolute()) assert utils.to_absolute_path(path) == expected + + +class TestRunAndReport: + def test_success(self) -> None: + def func() -> Any: + return 123 + + assert run_and_report(func) == 123 + + def test_simple_failure(self) -> None: + """Full traceback is printed for simple `run_and_report` failure.""" + + def simple_error() -> None: + assert False, "simple_err_msg" + + mock_stderr = io.StringIO() + with raises(SystemExit, match="1"), patch("sys.stderr", new=mock_stderr): + run_and_report(simple_error) + mock_stderr.seek(0) + stderr_output = mock_stderr.read() + assert_regex_match( + dedent( + r""" + Traceback \(most recent call last\):$ + File "[^"]+", line \d+, in run_and_report$ + return func\(\)$ + File "[^"]+", line \d+, in simple_error$ + assert False, "simple_err_msg"$ + AssertionError: simple_err_msg$ + assert False$ + """ + ), + stderr_output, + ) + + def test_run_job_failure(self) -> None: + """ + If a function named `run_job` appears in the traceback, the top of the + stack is stripped away until after the `run_job` frame. + """ + + def run_job_wrapper() -> None: + def run_job() -> None: + def nested_error() -> None: + assert False, "nested_err" + + nested_error() + + run_job() + + mock_stderr = io.StringIO() + with raises(SystemExit, match="1"), patch("sys.stderr", new=mock_stderr): + run_and_report(run_job_wrapper) + mock_stderr.seek(0) + stderr_output = mock_stderr.read() + assert_regex_match( + dedent( + r""" + Traceback \(most recent call last\):$ + File "[^"]+", line \d+, in nested_error$ + assert False, "nested_err"$ + AssertionError: nested_err$ + assert False$ + Set the environment variable HYDRA_FULL_ERROR=1 for a complete stack trace\.$ + """ + ), + stderr_output, + ) + + def test_run_job_omegaconf_failure(self) -> None: + """ + If a function named `run_job` appears in the traceback, the bottom of the + stack is stripped away until an `omegaconf` frame is found. + """ + + def run_job() -> None: + def job_calling_omconf() -> None: + from omegaconf import OmegaConf + + OmegaConf.resolve(123) # type: ignore + + job_calling_omconf() + + mock_stderr = io.StringIO() + with raises(SystemExit, match="1"), patch("sys.stderr", new=mock_stderr): + run_and_report(run_job) + mock_stderr.seek(0) + stderr_output = mock_stderr.read() + print(f"stderr_output:\n{stderr_output}\n--------") + assert_regex_match( + dedent( + r""" + Traceback \(most recent call last\):$ + File "[^"]+", line \d+, in job_calling_omconf$ + OmegaConf.resolve\(123\) # type: ignore$ + ValueError: Invalid config type \(int\), expected an OmegaConf Container$ + Set the environment variable HYDRA_FULL_ERROR=1 for a complete stack trace\.$ + """ + ), + stderr_output, + ) From 953fc550c3ddf75c908747f774711c67a450373b Mon Sep 17 00:00:00 2001 From: Jasha <8935917+Jasha10@users.noreply.github.com> Date: Sat, 31 Jul 2021 11:27:33 -0500 Subject: [PATCH 05/18] refactor tests with @pytest.mark.parametrize --- tests/test_utils.py | 148 +++++++++++++++++++++----------------------- 1 file changed, 72 insertions(+), 76 deletions(-) diff --git a/tests/test_utils.py b/tests/test_utils.py index 6cdb323f1cb..ba311d9594a 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -7,7 +7,7 @@ from unittest.mock import patch from omegaconf import DictConfig, OmegaConf -from pytest import mark, raises +from pytest import mark, raises, param from hydra import utils from hydra._internal.utils import run_and_report @@ -68,44 +68,18 @@ def test_to_absolute_path_without_hydra( class TestRunAndReport: - def test_success(self) -> None: - def func() -> Any: - return 123 + class DemoFunctions: + """Demo inputs for `run_and_report`""" - assert run_and_report(func) == 123 - - def test_simple_failure(self) -> None: - """Full traceback is printed for simple `run_and_report` failure.""" + @staticmethod + def success_func() -> Any: + return 123 + @staticmethod def simple_error() -> None: assert False, "simple_err_msg" - mock_stderr = io.StringIO() - with raises(SystemExit, match="1"), patch("sys.stderr", new=mock_stderr): - run_and_report(simple_error) - mock_stderr.seek(0) - stderr_output = mock_stderr.read() - assert_regex_match( - dedent( - r""" - Traceback \(most recent call last\):$ - File "[^"]+", line \d+, in run_and_report$ - return func\(\)$ - File "[^"]+", line \d+, in simple_error$ - assert False, "simple_err_msg"$ - AssertionError: simple_err_msg$ - assert False$ - """ - ), - stderr_output, - ) - - def test_run_job_failure(self) -> None: - """ - If a function named `run_job` appears in the traceback, the top of the - stack is stripped away until after the `run_job` frame. - """ - + @staticmethod def run_job_wrapper() -> None: def run_job() -> None: def nested_error() -> None: @@ -115,54 +89,76 @@ def nested_error() -> None: run_job() - mock_stderr = io.StringIO() - with raises(SystemExit, match="1"), patch("sys.stderr", new=mock_stderr): - run_and_report(run_job_wrapper) - mock_stderr.seek(0) - stderr_output = mock_stderr.read() - assert_regex_match( - dedent( - r""" - Traceback \(most recent call last\):$ - File "[^"]+", line \d+, in nested_error$ - assert False, "nested_err"$ - AssertionError: nested_err$ - assert False$ - Set the environment variable HYDRA_FULL_ERROR=1 for a complete stack trace\.$ - """ - ), - stderr_output, - ) + @staticmethod + def omegaconf_job_wrapper() -> None: + def run_job() -> None: + def job_calling_omconf() -> None: + from omegaconf import OmegaConf - def test_run_job_omegaconf_failure(self) -> None: - """ - If a function named `run_job` appears in the traceback, the bottom of the - stack is stripped away until an `omegaconf` frame is found. - """ + OmegaConf.resolve(123) # type: ignore - def run_job() -> None: - def job_calling_omconf() -> None: - from omegaconf import OmegaConf + job_calling_omconf() - OmegaConf.resolve(123) # type: ignore + run_job() - job_calling_omconf() + def test_success(self) -> None: + assert run_and_report(self.DemoFunctions.success_func) == 123 + + @mark.parametrize( + "demo_func, expected_traceback_regex", + [ + param( + DemoFunctions.simple_error, + dedent( + r""" + Traceback \(most recent call last\):$ + File "[^"]+", line \d+, in run_and_report$ + return func\(\)$ + File "[^"]+", line \d+, in simple_error$ + assert False, "simple_err_msg"$ + AssertionError: simple_err_msg$ + assert False$ + """ + ), + id="simple_failure" + ), + param( + DemoFunctions.run_job_wrapper, + dedent( + r""" + Traceback \(most recent call last\):$ + File "[^"]+", line \d+, in nested_error$ + assert False, "nested_err"$ + AssertionError: nested_err$ + assert False$ + Set the environment variable HYDRA_FULL_ERROR=1 for a complete stack trace\.$ + """ + ), + id="run_job_failure" + ), + param( + DemoFunctions.omegaconf_job_wrapper, + dedent( + r""" + Traceback \(most recent call last\):$ + File "[^"]+", line \d+, in job_calling_omconf$ + OmegaConf.resolve\(123\) # type: ignore$ + ValueError: Invalid config type \(int\), expected an OmegaConf Container$ + Set the environment variable HYDRA_FULL_ERROR=1 for a complete stack trace\.$ + """ + ), + id="run_job_failure_with_omegaconf" + ), + ], + ) + def test_failure( + self, demo_func: Any, expected_traceback_regex: str + ) -> None: + """Full traceback is printed for simple `run_and_report` failure.""" mock_stderr = io.StringIO() with raises(SystemExit, match="1"), patch("sys.stderr", new=mock_stderr): - run_and_report(run_job) + run_and_report(demo_func) mock_stderr.seek(0) stderr_output = mock_stderr.read() - print(f"stderr_output:\n{stderr_output}\n--------") - assert_regex_match( - dedent( - r""" - Traceback \(most recent call last\):$ - File "[^"]+", line \d+, in job_calling_omconf$ - OmegaConf.resolve\(123\) # type: ignore$ - ValueError: Invalid config type \(int\), expected an OmegaConf Container$ - Set the environment variable HYDRA_FULL_ERROR=1 for a complete stack trace\.$ - """ - ), - stderr_output, - ) + assert_regex_match(expected_traceback_regex, stderr_output) From ad6be1fd74c845b3dbad86465a3adde8e447ad2b Mon Sep 17 00:00:00 2001 From: Jasha <8935917+Jasha10@users.noreply.github.com> Date: Sat, 31 Jul 2021 11:28:41 -0500 Subject: [PATCH 06/18] remove outdated test docstring --- tests/test_utils.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/test_utils.py b/tests/test_utils.py index ba311d9594a..8ad5f9fede0 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -154,8 +154,6 @@ def test_success(self) -> None: def test_failure( self, demo_func: Any, expected_traceback_regex: str ) -> None: - """Full traceback is printed for simple `run_and_report` failure.""" - mock_stderr = io.StringIO() with raises(SystemExit, match="1"), patch("sys.stderr", new=mock_stderr): run_and_report(demo_func) From e7218d100cb49ee84e8ac40e27ddf826235a6948 Mon Sep 17 00:00:00 2001 From: Jasha <8935917+Jasha10@users.noreply.github.com> Date: Sat, 31 Jul 2021 11:34:29 -0500 Subject: [PATCH 07/18] black formatting --- tests/test_utils.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/test_utils.py b/tests/test_utils.py index 8ad5f9fede0..e7dd2a25736 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -151,9 +151,7 @@ def test_success(self) -> None: ), ], ) - def test_failure( - self, demo_func: Any, expected_traceback_regex: str - ) -> None: + def test_failure(self, demo_func: Any, expected_traceback_regex: str) -> None: mock_stderr = io.StringIO() with raises(SystemExit, match="1"), patch("sys.stderr", new=mock_stderr): run_and_report(demo_func) From a9a02d43bf09880d762ec2e005ca446b37b4ce3f Mon Sep 17 00:00:00 2001 From: Jasha <8935917+Jasha10@users.noreply.github.com> Date: Sat, 31 Jul 2021 11:34:36 -0500 Subject: [PATCH 08/18] improve pytest param ids --- tests/test_utils.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_utils.py b/tests/test_utils.py index e7dd2a25736..f61b192f65b 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -120,7 +120,7 @@ def test_success(self) -> None: assert False$ """ ), - id="simple_failure" + id="simple_failure_full_traceback", ), param( DemoFunctions.run_job_wrapper, @@ -134,7 +134,7 @@ def test_success(self) -> None: Set the environment variable HYDRA_FULL_ERROR=1 for a complete stack trace\.$ """ ), - id="run_job_failure" + id="strip_run_job_from_top_of_stack", ), param( DemoFunctions.omegaconf_job_wrapper, @@ -147,7 +147,7 @@ def test_success(self) -> None: Set the environment variable HYDRA_FULL_ERROR=1 for a complete stack trace\.$ """ ), - id="run_job_failure_with_omegaconf" + id="strip_omegaconf_from_bottom_of_stack", ), ], ) From 80f1bb285af0a3d81d13502bc6718a3e92bafaf8 Mon Sep 17 00:00:00 2001 From: Jasha <8935917+Jasha10@users.noreply.github.com> Date: Sat, 31 Jul 2021 11:45:28 -0500 Subject: [PATCH 09/18] isort --- tests/test_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_utils.py b/tests/test_utils.py index f61b192f65b..3d3cbbd5e2c 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -7,7 +7,7 @@ from unittest.mock import patch from omegaconf import DictConfig, OmegaConf -from pytest import mark, raises, param +from pytest import mark, param, raises from hydra import utils from hydra._internal.utils import run_and_report From 6a6e665f34f183a2e60e2c3ba5662e6dd6ae0d61 Mon Sep 17 00:00:00 2001 From: Jasha <8935917+Jasha10@users.noreply.github.com> Date: Sat, 31 Jul 2021 11:45:36 -0500 Subject: [PATCH 10/18] add failing test --- tests/test_utils.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/test_utils.py b/tests/test_utils.py index 3d3cbbd5e2c..492225ee9d7 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -158,3 +158,23 @@ def test_failure(self, demo_func: Any, expected_traceback_regex: str) -> None: mock_stderr.seek(0) stderr_output = mock_stderr.read() assert_regex_match(expected_traceback_regex, stderr_output) + + def test_getmodule_returns_none(self) -> None: + demo_func = self.DemoFunctions.omegaconf_job_wrapper + expected_traceback_regex = dedent( + r""" + .* + .* + .* + .* + .* + """ + ) + mock_stderr = io.StringIO() + with raises(SystemExit, match="1"), patch("sys.stderr", new=mock_stderr), patch( + "inspect.getmodule", new=lambda *args: None + ): + run_and_report(demo_func) + mock_stderr.seek(0) + stderr_output = mock_stderr.read() + assert_regex_match(expected_traceback_regex, stderr_output) From 90a74518c22aacbfaa77ca90447c4f14bdfde2c3 Mon Sep 17 00:00:00 2001 From: Jasha <8935917+Jasha10@users.noreply.github.com> Date: Sat, 31 Jul 2021 12:23:41 -0500 Subject: [PATCH 11/18] wrap simplified traceback logic in a try/except block --- hydra/_internal/utils.py | 143 ++++++++++++++++++++------------------- tests/test_utils.py | 32 ++++++--- 2 files changed, 95 insertions(+), 80 deletions(-) diff --git a/hydra/_internal/utils.py b/hydra/_internal/utils.py index 10c2e5c68e3..3fe662c3451 100644 --- a/hydra/_internal/utils.py +++ b/hydra/_internal/utils.py @@ -213,76 +213,79 @@ def run_and_report(func: Any) -> Any: if _is_env_set("HYDRA_FULL_ERROR") or is_under_debugger(): raise ex else: - if isinstance(ex, CompactHydraException): - sys.stderr.write(str(ex) + os.linesep) - if isinstance(ex.__cause__, OmegaConfBaseException): - sys.stderr.write(str(ex.__cause__) + os.linesep) - else: - # Custom printing that strips the Hydra related stack frames from the top - # And any omegaconf frames from the bottom. - # It is possible to add additional libraries to sanitize from the bottom later, - # maybe even make it configurable. - - tb = ex.__traceback__ - search_max = 10 - # strip Hydra frames from start of stack - # will strip until it hits run_job() - while search_max > 0: - if tb is None: - break - frame = tb.tb_frame - tb = tb.tb_next - search_max = search_max - 1 - if inspect.getframeinfo(frame).function == "run_job": - break - - if search_max == 0 or tb is None: - # could not detect run_job, probably a runtime exception before we got there. - # do not sanitize the stack trace. - print_exc() - sys.exit(1) - - # strip OmegaConf frames from bottom of stack - end: Optional[TracebackType] = tb - num_frames = 0 - while end is not None: - frame = end.tb_frame - mdl = inspect.getmodule(frame) - assert mdl is not None - name = mdl.__name__ - if name.startswith("omegaconf."): - break - end = end.tb_next - num_frames = num_frames + 1 - - @dataclass - class FakeTracebackType: - tb_next: Any = None # Optional["FakeTracebackType"] - tb_frame: Optional[FrameType] = None - tb_lasti: Optional[int] = None - tb_lineno: Optional[int] = None - - iter_tb = tb - final_tb = FakeTracebackType() - cur = final_tb - added = 0 - while True: - cur.tb_lasti = iter_tb.tb_lasti - cur.tb_lineno = iter_tb.tb_lineno - cur.tb_frame = iter_tb.tb_frame - - if added == num_frames - 1: - break - added = added + 1 - cur.tb_next = FakeTracebackType() - cur = cur.tb_next - assert iter_tb.tb_next is not None - iter_tb = iter_tb.tb_next - - print_exception(etype=None, value=ex, tb=final_tb) # type: ignore - sys.stderr.write( - "\nSet the environment variable HYDRA_FULL_ERROR=1 for a complete stack trace.\n" - ) + try: + if isinstance(ex, CompactHydraException): + sys.stderr.write(str(ex) + os.linesep) + if isinstance(ex.__cause__, OmegaConfBaseException): + sys.stderr.write(str(ex.__cause__) + os.linesep) + else: + # Custom printing that strips the Hydra related stack frames from the top + # And any omegaconf frames from the bottom. + # It is possible to add additional libraries to sanitize from the bottom later, + # maybe even make it configurable. + + tb = ex.__traceback__ + search_max = 10 + # strip Hydra frames from start of stack + # will strip until it hits run_job() + while search_max > 0: + if tb is None: + break + frame = tb.tb_frame + tb = tb.tb_next + search_max = search_max - 1 + if inspect.getframeinfo(frame).function == "run_job": + break + + if search_max == 0 or tb is None: + # could not detect run_job, probably a runtime exception before we got there. + # do not sanitize the stack trace. + print_exc() + sys.exit(1) + + # strip OmegaConf frames from bottom of stack + end: Optional[TracebackType] = tb + num_frames = 0 + while end is not None: + frame = end.tb_frame + mdl = inspect.getmodule(frame) + assert mdl is not None + name = mdl.__name__ + if name.startswith("omegaconf."): + break + end = end.tb_next + num_frames = num_frames + 1 + + @dataclass + class FakeTracebackType: + tb_next: Any = None # Optional["FakeTracebackType"] + tb_frame: Optional[FrameType] = None + tb_lasti: Optional[int] = None + tb_lineno: Optional[int] = None + + iter_tb = tb + final_tb = FakeTracebackType() + cur = final_tb + added = 0 + while True: + cur.tb_lasti = iter_tb.tb_lasti + cur.tb_lineno = iter_tb.tb_lineno + cur.tb_frame = iter_tb.tb_frame + + if added == num_frames - 1: + break + added = added + 1 + cur.tb_next = FakeTracebackType() + cur = cur.tb_next + assert iter_tb.tb_next is not None + iter_tb = iter_tb.tb_next + + print_exception(etype=None, value=ex, tb=final_tb) # type: ignore + sys.stderr.write( + "\nSet the environment variable HYDRA_FULL_ERROR=1 for a complete stack trace.\n" + ) + except Exception: + print_exception(etype=None, value=ex, tb=ex.__traceback__) sys.exit(1) diff --git a/tests/test_utils.py b/tests/test_utils.py index 492225ee9d7..6863e711ab5 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -159,22 +159,34 @@ def test_failure(self, demo_func: Any, expected_traceback_regex: str) -> None: stderr_output = mock_stderr.read() assert_regex_match(expected_traceback_regex, stderr_output) - def test_getmodule_returns_none(self) -> None: + def test_simplified_traceback_failure(self) -> None: + """ + Test that a full traceback is printed in the case where an exception + occurs during the simplified traceback logic. + """ demo_func = self.DemoFunctions.omegaconf_job_wrapper expected_traceback_regex = dedent( r""" - .* - .* - .* - .* - .* + Traceback \(most recent call last\):$ + File "[^"]+", line \d+, in run_and_report$ + return func\(\)$ + File "[^"]+", line \d+, in omegaconf_job_wrapper$ + run_job\(\)$ + File "[^"]+", line \d+, in run_job$ + job_calling_omconf\(\)$ + File "[^"]+", line \d+, in job_calling_omconf$ + OmegaConf.resolve(123) # type: ignore + File "[^"]+", line \d+, in resolve$ + raise ValueError( + ValueError: Invalid config type \(int\), expected an OmegaConf Container$ """ ) mock_stderr = io.StringIO() - with raises(SystemExit, match="1"), patch("sys.stderr", new=mock_stderr), patch( - "inspect.getmodule", new=lambda *args: None - ): - run_and_report(demo_func) + with raises(SystemExit, match="1"), patch("sys.stderr", new=mock_stderr): + # patch `inspect.getmodule` so that an exception will occur in the + # simplified traceback logic: + with patch("inspect.getmodule", new=lambda *args: None): + run_and_report(demo_func) mock_stderr.seek(0) stderr_output = mock_stderr.read() assert_regex_match(expected_traceback_regex, stderr_output) From 69379e0618beb0421cfbde84ab959b39390b28f8 Mon Sep 17 00:00:00 2001 From: Jasha <8935917+Jasha10@users.noreply.github.com> Date: Mon, 2 Aug 2021 16:18:55 -0500 Subject: [PATCH 12/18] regex patterns: backslash excape parentheses --- tests/test_utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_utils.py b/tests/test_utils.py index 6863e711ab5..b117a7c949a 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -175,9 +175,9 @@ def test_simplified_traceback_failure(self) -> None: File "[^"]+", line \d+, in run_job$ job_calling_omconf\(\)$ File "[^"]+", line \d+, in job_calling_omconf$ - OmegaConf.resolve(123) # type: ignore + OmegaConf.resolve\(123\) # type: ignore File "[^"]+", line \d+, in resolve$ - raise ValueError( + raise ValueError\( ValueError: Invalid config type \(int\), expected an OmegaConf Container$ """ ) From 7de5282b18c33f511d4c15f174120c0ea0ce6cb5 Mon Sep 17 00:00:00 2001 From: Jasha <8935917+Jasha10@users.noreply.github.com> Date: Mon, 2 Aug 2021 16:43:47 -0500 Subject: [PATCH 13/18] raise original exception after printing a warning --- hydra/_internal/utils.py | 14 ++++++++++++-- tests/test_utils.py | 24 ++++++++---------------- 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/hydra/_internal/utils.py b/hydra/_internal/utils.py index 3fe662c3451..b36bc9a267e 100644 --- a/hydra/_internal/utils.py +++ b/hydra/_internal/utils.py @@ -284,8 +284,18 @@ class FakeTracebackType: sys.stderr.write( "\nSet the environment variable HYDRA_FULL_ERROR=1 for a complete stack trace.\n" ) - except Exception: - print_exception(etype=None, value=ex, tb=ex.__traceback__) + except Exception as ex2: + sys.stderr.write( + "An error occurred during Hydra's exception formatting:" + + os.linesep + ) + if ex2.__traceback__ is not None: + fname = ex2.__traceback__.tb_frame.f_code.co_filename + lineno = ex2.__traceback__.tb_lineno + sys.stderr.write(repr(ex2) + f", {fname}:{lineno}" + os.linesep) + else: # pragma: no cover + sys.stderr.write(repr(ex2)) + raise ex sys.exit(1) diff --git a/tests/test_utils.py b/tests/test_utils.py index b117a7c949a..b07f43ab740 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -161,28 +161,20 @@ def test_failure(self, demo_func: Any, expected_traceback_regex: str) -> None: def test_simplified_traceback_failure(self) -> None: """ - Test that a full traceback is printed in the case where an exception - occurs during the simplified traceback logic. + Test that a warning is printed and the original exception is re-raised + when an exception occurs during the simplified traceback logic. """ - demo_func = self.DemoFunctions.omegaconf_job_wrapper + demo_func = self.DemoFunctions.run_job_wrapper expected_traceback_regex = dedent( r""" - Traceback \(most recent call last\):$ - File "[^"]+", line \d+, in run_and_report$ - return func\(\)$ - File "[^"]+", line \d+, in omegaconf_job_wrapper$ - run_job\(\)$ - File "[^"]+", line \d+, in run_job$ - job_calling_omconf\(\)$ - File "[^"]+", line \d+, in job_calling_omconf$ - OmegaConf.resolve\(123\) # type: ignore - File "[^"]+", line \d+, in resolve$ - raise ValueError\( - ValueError: Invalid config type \(int\), expected an OmegaConf Container$ + An error occurred during Hydra's exception formatting:$ + AssertionError\(.*\), [^\s\d]+\.py:\d+$ """ ) mock_stderr = io.StringIO() - with raises(SystemExit, match="1"), patch("sys.stderr", new=mock_stderr): + with raises(AssertionError, match="nested_err"), patch( + "sys.stderr", new=mock_stderr + ): # patch `inspect.getmodule` so that an exception will occur in the # simplified traceback logic: with patch("inspect.getmodule", new=lambda *args: None): From 7d74b9a1bec103256bd9a3595d8f0c66be235edc Mon Sep 17 00:00:00 2001 From: Jasha <8935917+Jasha10@users.noreply.github.com> Date: Mon, 2 Aug 2021 17:13:01 -0500 Subject: [PATCH 14/18] more flexible regex pattern for filenames --- tests/test_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_utils.py b/tests/test_utils.py index b07f43ab740..f2d6fb20b10 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -168,7 +168,7 @@ def test_simplified_traceback_failure(self) -> None: expected_traceback_regex = dedent( r""" An error occurred during Hydra's exception formatting:$ - AssertionError\(.*\), [^\s\d]+\.py:\d+$ + AssertionError\(.*\), \S+\.py:\d+$ """ ) mock_stderr = io.StringIO() From 3bf3e48ae2bf44ce7f9a98aab17d73aa3dfa605e Mon Sep 17 00:00:00 2001 From: Jasha <8935917+Jasha10@users.noreply.github.com> Date: Mon, 2 Aug 2021 17:54:04 -0500 Subject: [PATCH 15/18] newline after warning message --- hydra/_internal/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hydra/_internal/utils.py b/hydra/_internal/utils.py index b36bc9a267e..dfd22b89bce 100644 --- a/hydra/_internal/utils.py +++ b/hydra/_internal/utils.py @@ -294,7 +294,7 @@ class FakeTracebackType: lineno = ex2.__traceback__.tb_lineno sys.stderr.write(repr(ex2) + f", {fname}:{lineno}" + os.linesep) else: # pragma: no cover - sys.stderr.write(repr(ex2)) + sys.stderr.write(repr(ex2) + os.linesep) raise ex sys.exit(1) From cf7bbbfcfeab1d754fb9823f1292033c12004150 Mon Sep 17 00:00:00 2001 From: Jasha <8935917+Jasha10@users.noreply.github.com> Date: Tue, 3 Aug 2021 14:32:37 -0500 Subject: [PATCH 16/18] less info when printing warning --- hydra/_internal/utils.py | 8 ++------ tests/test_utils.py | 2 +- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/hydra/_internal/utils.py b/hydra/_internal/utils.py index dfd22b89bce..18f556bacdb 100644 --- a/hydra/_internal/utils.py +++ b/hydra/_internal/utils.py @@ -288,13 +288,9 @@ class FakeTracebackType: sys.stderr.write( "An error occurred during Hydra's exception formatting:" + os.linesep + + repr(ex2) + + os.linesep ) - if ex2.__traceback__ is not None: - fname = ex2.__traceback__.tb_frame.f_code.co_filename - lineno = ex2.__traceback__.tb_lineno - sys.stderr.write(repr(ex2) + f", {fname}:{lineno}" + os.linesep) - else: # pragma: no cover - sys.stderr.write(repr(ex2) + os.linesep) raise ex sys.exit(1) diff --git a/tests/test_utils.py b/tests/test_utils.py index f2d6fb20b10..aba1e6dbaab 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -168,7 +168,7 @@ def test_simplified_traceback_failure(self) -> None: expected_traceback_regex = dedent( r""" An error occurred during Hydra's exception formatting:$ - AssertionError\(.*\), \S+\.py:\d+$ + AssertionError\(.*\)$ """ ) mock_stderr = io.StringIO() From 33d471dd065c5bdbcb7cec467cbe5362f9477d89 Mon Sep 17 00:00:00 2001 From: Jasha <8935917+Jasha10@users.noreply.github.com> Date: Thu, 5 Aug 2021 15:09:04 -0500 Subject: [PATCH 17/18] add documentation to the TestRunAndReport class --- tests/test_utils.py | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/tests/test_utils.py b/tests/test_utils.py index aba1e6dbaab..f8e40002c51 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -68,8 +68,27 @@ def test_to_absolute_path_without_hydra( class TestRunAndReport: + """ + Test the `hydra._internal.utils.run_and_report` function. + + def run_and_report(func: Any) -> Any: ... + + This class defines several test methods: + test_success: + a simple test case where `run_and_report(func)` succeeds. + test_failure: + test when `func` raises an exception, and `run_and_report(func)` + prints a nicely-formatted error message + test_simplified_traceback_failure: + test when printing a nicely-formatted error message fails, so + `run_and_report` falls back to re-raising the exception from `func`. + """ + class DemoFunctions: - """Demo inputs for `run_and_report`""" + """ + The methods of this `DemoFunctions` class are passed to + `run_and_report` as the func argument. + """ @staticmethod def success_func() -> Any: @@ -81,6 +100,12 @@ def simple_error() -> None: @staticmethod def run_job_wrapper() -> None: + """ + Trigger special logic in `run_and_report` that looks for a function + called "run_job" in the stack and strips away the leading stack + frames. + """ + def run_job() -> None: def nested_error() -> None: assert False, "nested_err" @@ -91,10 +116,17 @@ def nested_error() -> None: @staticmethod def omegaconf_job_wrapper() -> None: + """ + Trigger special logic in `run_and_report` that looks for the + `omegaconf` module in the stack and strips away the bottom stack + frames. + """ + def run_job() -> None: def job_calling_omconf() -> None: from omegaconf import OmegaConf + # The below causes an exception: OmegaConf.resolve(123) # type: ignore job_calling_omconf() From a48a21bf53e87c6ac1f9950f6f2c14e640b74526 Mon Sep 17 00:00:00 2001 From: Jasha <8935917+Jasha10@users.noreply.github.com> Date: Thu, 5 Aug 2021 19:13:00 -0500 Subject: [PATCH 18/18] add news fragment --- news/1739.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/1739.bugfix diff --git a/news/1739.bugfix b/news/1739.bugfix new file mode 100644 index 00000000000..2874d956143 --- /dev/null +++ b/news/1739.bugfix @@ -0,0 +1 @@ +Fix failure when sanitizing stack traces resulting from job exceptions