diff --git a/cloudinit/url_helper.py b/cloudinit/url_helper.py index ebb6bc4a0ef..04643895e76 100644 --- a/cloudinit/url_helper.py +++ b/cloudinit/url_helper.py @@ -413,6 +413,7 @@ def dual_stack( return_exception = future.exception() if return_exception: last_exception = return_exception + exceptions.append(last_exception) else: return_result = future.result() if return_result: @@ -426,9 +427,9 @@ def dual_stack( # debugging if last_exception: LOG.warning( - "Exception(s) %s during request to %s, " - "raising last exception", - " ".join(exceptions), + "Exception(s) %s during request to " + "%s, raising last exception", + exceptions, returned_address, ) raise last_exception diff --git a/tests/unittests/test_url_helper.py b/tests/unittests/test_url_helper.py index 5e09221938c..a9b9a85fbaf 100644 --- a/tests/unittests/test_url_helper.py +++ b/tests/unittests/test_url_helper.py @@ -282,43 +282,18 @@ class TestDualStack: """ @pytest.mark.parametrize( - "func," - "addresses," - "stagger_delay," - "timeout," - "expected_val," - "expected_exc", + "func," "addresses," "stagger_delay," "timeout," "expected_val,", [ # Assert order based on timeout - (lambda x, _: x, ("one", "two"), 1, 1, "one", None), + (lambda x, _: x, ("one", "two"), 1, 1, "one"), # Assert timeout results in (None, None) - (lambda _a, _b: event.wait(1), ("one", "two"), 1, 0, None, None), - # Assert that exception in func is raised if all threads - # raise exception - # currently if all threads experience exception - # dual_stack() logs an error containing all exceptions - # but only raises the last exception to occur - ( - lambda _a, _b: 1 / 0, - ("one", "two"), - 0, - 1, - None, - ZeroDivisionError, - ), - # Verify "best effort behavior" - # dual_stack will temporarily ignore an exception in any of the - # request threads in hopes that a later thread will succeed - # this behavior is intended to allow a requests.ConnectionError - # exception from on endpoint to occur without preventing another - # thread from succeeding + (lambda _a, _b: event.wait(1), ("one", "two"), 1, 0, None), ( lambda a, _b: 1 / 0 if a == "one" else a, ("one", "two"), 0, 1, "two", - None, ), # Assert that exception in func is only raised # if neither thread gets a valid result @@ -328,7 +303,6 @@ class TestDualStack: 0, 1, "one", - None, ), # simulate a slow response to verify correct order ( @@ -337,7 +311,6 @@ class TestDualStack: 0, 1, "two", - None, ), # simulate a slow response to verify correct order ( @@ -346,7 +319,6 @@ class TestDualStack: 0, 1, "tri", - None, ), ], ) @@ -357,7 +329,6 @@ def test_dual_stack( stagger_delay, timeout, expected_val, - expected_exc, ): """Assert various failure modes behave as expected""" event.clear() @@ -369,13 +340,103 @@ def test_dual_stack( stagger_delay=stagger_delay, timeout=timeout, ) - if expected_exc: - with pytest.raises(expected_exc): - _, result = assert_time(gen) - assert expected_val == result - else: - _, result = assert_time(gen) - assert expected_val == result + _, result = assert_time(gen) + assert expected_val == result + + event.set() + + @pytest.mark.parametrize( + "func," + "addresses," + "stagger_delay," + "timeout," + "message," + "expected_exc", + [ + ( + lambda _a, _b: 1 / 0, + ("¯\\_(ツ)_/¯", "(╯°□°)╯︵ ┻━┻"), + 0, + 1, + "division by zero", + ZeroDivisionError, + ), + ( + lambda _a, _b: 1 / 0, + ("it", "really", "doesn't"), + 0, + 1, + "division by zero", + ZeroDivisionError, + ), + ( + lambda _a, _b: [][0], + ("matter", "these"), + 0, + 1, + "list index out of range", + IndexError, + ), + ( + lambda _a, _b: (_ for _ in ()).throw( + Exception("soapstone is not effective soap") + ), + ("are", "ignored"), + 0, + 1, + "soapstone is not effective soap", + Exception, + ), + ], + ) + def test_dual_stack_exceptions( + self, + func, + addresses, + stagger_delay, + timeout, + message, + expected_exc, + caplog, + ): + # Context: + # + # currently if all threads experience exception + # dual_stack() logs an error containing all exceptions + # but only raises the last exception to occur + # Verify "best effort behavior" + # dual_stack will temporarily ignore an exception in any of the + # request threads in hopes that a later thread will succeed + # this behavior is intended to allow a requests.ConnectionError + # exception from on endpoint to occur without preventing another + # thread from succeeding + event.clear() + + # Note: python3.6 repr(Exception("test")) produces different output + # than later versions, so we cannot match exact message without + # some ugly manual exception repr() function, which I'd rather not do + # in dual_stack(), so we recreate expected messages manually here + # in a version-independant way for testing, the extra comma on old + # versions won't hurt anything + exc_list = str([expected_exc(message) for _ in addresses]) + expected_msg = f"Exception(s) {exc_list} during request" + gen = partial( + dual_stack, + func, + addresses, + stagger_delay=stagger_delay, + timeout=timeout, + ) + with pytest.raises(expected_exc): + gen() # 1 + with caplog.at_level(logging.DEBUG): + try: + gen() # 2 + except expected_exc: + pass + finally: + assert 2 == len(caplog.records) + assert 2 == caplog.text.count(expected_msg) event.set() def test_dual_stack_staggered(self):