diff --git a/sdks/python/apache_beam/utils/retry.py b/sdks/python/apache_beam/utils/retry.py index 03e42829b843..063446472fe5 100644 --- a/sdks/python/apache_beam/utils/retry.py +++ b/sdks/python/apache_beam/utils/retry.py @@ -183,7 +183,7 @@ def retry_on_server_errors_timeout_or_quota_issues_filter(exception): if not isinstance(content, dict): content = json.loads(exception.content) return content["error"]["errors"][0]["reason"] in _RETRYABLE_REASONS - except (KeyError, IndexError, TypeError) as e: + except (KeyError, IndexError, TypeError, ValueError) as e: _LOGGER.warning( "Could not determine if HttpError is transient. " "Will not retry: %s", diff --git a/sdks/python/apache_beam/utils/retry_test.py b/sdks/python/apache_beam/utils/retry_test.py index 05cc6e6a7f40..8d9e006e00c5 100644 --- a/sdks/python/apache_beam/utils/retry_test.py +++ b/sdks/python/apache_beam/utils/retry_test.py @@ -141,6 +141,24 @@ def test_with_http_error_that_should_be_retried(self): 500) self.assertEqual(len(self.clock.calls), 10) + @unittest.skipIf(HttpError is None, 'google-apitools is not installed') + def test_quota_filter_preserves_http_error_with_non_json_content(self): + error = HttpError({'status': '403'}, + 'upstream error', + 'https://example.test') + + @retry.with_exponential_backoff( + clock=self.clock, + retry_filter=retry.retry_on_server_errors_timeout_or_quota_issues_filter + ) + def fail(): + raise error + + with self.assertRaises(HttpError) as raised: + fail() + self.assertIs(raised.exception, error) + self.assertEqual(self.clock.calls, []) + def test_with_explicit_initial_delay(self): self.assertRaises( NotImplementedError,