From 39f79367992dd6ced14556f3f792f96718928604 Mon Sep 17 00:00:00 2001 From: Emily Rockman Date: Tue, 15 Mar 2022 11:57:07 -0500 Subject: [PATCH] catch all requests exceptions to retry (#4865) * catch all requests exceptions to retry * add changelog --- .../unreleased/Fixes-20220315-105331.yaml | 8 ++++ core/dbt/events/types.py | 12 +++++- core/dbt/utils.py | 14 +++---- test/unit/test_core_dbt_utils.py | 41 ++++++------------- test/unit/test_events.py | 3 +- 5 files changed, 40 insertions(+), 38 deletions(-) create mode 100644 .changes/unreleased/Fixes-20220315-105331.yaml diff --git a/.changes/unreleased/Fixes-20220315-105331.yaml b/.changes/unreleased/Fixes-20220315-105331.yaml new file mode 100644 index 00000000000..9e21f48768d --- /dev/null +++ b/.changes/unreleased/Fixes-20220315-105331.yaml @@ -0,0 +1,8 @@ +kind: Fixes +body: Catch all Requests Exceptions on deps install to attempt retries. Also log + the exceptions hit. +time: 2022-03-15T10:53:31.637963-05:00 +custom: + Author: emmyoop + Issue: "4849" + PR: "4865" diff --git a/core/dbt/events/types.py b/core/dbt/events/types.py index c022fdd89c0..9ebb65ccf81 100644 --- a/core/dbt/events/types.py +++ b/core/dbt/events/types.py @@ -2381,7 +2381,7 @@ def message(self) -> str: class RetryExternalCall(DebugLevel, Cli, File): attempt: int max: int - code: str = "Z045" + code: str = "M020" def message(self) -> str: return f"Retrying external call. Attempt: {self.attempt} Max attempts: {self.max}" @@ -2419,6 +2419,15 @@ def message(self) -> str: return "Internal event buffer full. Earliest events will be dropped (FIFO)." +@dataclass +class RecordRetryException(DebugLevel): + exc: Exception + code: str = "M021" + + def message(self) -> str: + return f"External call exception: {self.exc}" + + # since mypy doesn't run on every file we need to suggest to mypy that every # class gets instantiated. But we don't actually want to run this code. # making the conditional `if False` causes mypy to skip it as dead code so @@ -2774,3 +2783,4 @@ def message(self) -> str: GeneralWarningMsg(msg='', log_fmt='') GeneralWarningException(exc=Exception(''), log_fmt='') EventBufferFull() + RecordRetryException(exc=Exception("")) diff --git a/core/dbt/utils.py b/core/dbt/utils.py index 2f115a0ba74..9ea0d426007 100644 --- a/core/dbt/utils.py +++ b/core/dbt/utils.py @@ -16,7 +16,7 @@ from contextlib import contextmanager from dbt.exceptions import ConnectionException from dbt.events.functions import fire_event -from dbt.events.types import RetryExternalCall +from dbt.events.types import RetryExternalCall, RecordRetryException from enum import Enum from typing_extensions import Protocol from typing import ( @@ -599,19 +599,19 @@ def __contains__(self, name) -> bool: def _connection_exception_retry(fn, max_attempts: int, attempt: int = 0): """Attempts to run a function that makes an external call, if the call fails - on a connection error, timeout or decompression issue, it will be tried up to 5 more times. - See https://github.com/dbt-labs/dbt-core/issues/4579 for context on this decompression issues - specifically. + on a Requests exception or decompression issue (ReadError), it will be tried + up to 5 more times. All exceptions that Requests explicitly raises inherit from + requests.exceptions.RequestException. See https://github.com/dbt-labs/dbt-core/issues/4579 + for context on this decompression issues specifically. """ try: return fn() except ( - requests.exceptions.ConnectionError, - requests.exceptions.Timeout, - requests.exceptions.ContentDecodingError, + requests.exceptions.RequestException, ReadError, ) as exc: if attempt <= max_attempts - 1: + fire_event(RecordRetryException(exc=exc)) fire_event(RetryExternalCall(attempt=attempt, max=max_attempts)) time.sleep(1) _connection_exception_retry(fn, max_attempts, attempt + 1) diff --git a/test/unit/test_core_dbt_utils.py b/test/unit/test_core_dbt_utils.py index 2c6ec182b94..1deb8a77552 100644 --- a/test/unit/test_core_dbt_utils.py +++ b/test/unit/test_core_dbt_utils.py @@ -12,27 +12,17 @@ def test_connection_exception_retry_none(self): connection_exception_retry(lambda: Counter._add(), 5) self.assertEqual(1, counter) + def test_connection_exception_retry_success_requests_exception(self): + Counter._reset() + connection_exception_retry(lambda: Counter._add_with_requests_exception(), 5) + self.assertEqual(2, counter) # 2 = original attempt returned None, plus 1 retry + def test_connection_exception_retry_max(self): Counter._reset() with self.assertRaises(ConnectionException): connection_exception_retry(lambda: Counter._add_with_exception(), 5) self.assertEqual(6, counter) # 6 = original attempt plus 5 retries - def test_connection_exception_retry_success(self): - Counter._reset() - connection_exception_retry(lambda: Counter._add_with_limited_exception(), 5) - self.assertEqual(2, counter) # 2 = original attempt plus 1 retry - - def test_connection_timeout(self): - Counter._reset() - connection_exception_retry(lambda: Counter._add_with_timeout(), 5) - self.assertEqual(2, counter) # 2 = original attempt plus 1 retry - - def test_connection_exception_retry_success_none_response(self): - Counter._reset() - connection_exception_retry(lambda: Counter._add_with_none_exception(), 5) - self.assertEqual(2, counter) # 2 = original attempt returned None, plus 1 retry - def test_connection_exception_retry_success_failed_untar(self): Counter._reset() connection_exception_retry(lambda: Counter._add_with_untar_exception(), 5) @@ -44,25 +34,18 @@ class Counter(): def _add(): global counter counter+=1 - def _add_with_exception(): - global counter - counter+=1 - raise requests.exceptions.ConnectionError - def _add_with_limited_exception(): - global counter - counter+=1 - if counter < 2: - raise requests.exceptions.ConnectionError - def _add_with_timeout(): + # All exceptions that Requests explicitly raises inherit from + # requests.exceptions.RequestException so we want to make sure that raises plus one exception + # that inherit from it for sanity + def _add_with_requests_exception(): global counter counter+=1 if counter < 2: - raise requests.exceptions.Timeout - def _add_with_none_exception(): + raise requests.exceptions.RequestException + def _add_with_exception(): global counter counter+=1 - if counter < 2: - raise requests.exceptions.ContentDecodingError + raise requests.exceptions.ConnectionError def _add_with_untar_exception(): global counter counter+=1 diff --git a/test/unit/test_events.py b/test/unit/test_events.py index be9d3f88ba8..352c1bb5915 100644 --- a/test/unit/test_events.py +++ b/test/unit/test_events.py @@ -401,7 +401,8 @@ def MockNode(): IntegrationTestError(''), IntegrationTestException(''), EventBufferFull(), - UnitTestInfo('') + RecordRetryException(exc=Exception('')), + UnitTestInfo(''), ]