From 78f43cfb1d71bda8ddb6668a107b3ee9e8c070bf Mon Sep 17 00:00:00 2001 From: Alexandre Girard Date: Wed, 27 Jul 2022 06:53:52 -0700 Subject: [PATCH] [low-code connectors] Handle 200 responses with error (#15055) * Handle 200 responses with error * missing file --- .../airbyte_cdk/sources/declarative/read_exception.py | 7 +++++++ .../requesters/error_handlers/default_error_handler.py | 6 +++--- .../requesters/error_handlers/http_response_filter.py | 3 ++- .../sources/declarative/retrievers/simple_retriever.py | 3 ++- .../error_handlers/test_default_error_handler.py | 9 +++++++++ 5 files changed, 23 insertions(+), 5 deletions(-) create mode 100644 airbyte-cdk/python/airbyte_cdk/sources/declarative/read_exception.py diff --git a/airbyte-cdk/python/airbyte_cdk/sources/declarative/read_exception.py b/airbyte-cdk/python/airbyte_cdk/sources/declarative/read_exception.py new file mode 100644 index 0000000000000..3c9a0cc0f43e8 --- /dev/null +++ b/airbyte-cdk/python/airbyte_cdk/sources/declarative/read_exception.py @@ -0,0 +1,7 @@ +# +# Copyright (c) 2022 Airbyte, Inc., all rights reserved. +# +class ReadException(Exception): + """ + Raise when there is an error reading data from an API Source + """ diff --git a/airbyte-cdk/python/airbyte_cdk/sources/declarative/requesters/error_handlers/default_error_handler.py b/airbyte-cdk/python/airbyte_cdk/sources/declarative/requesters/error_handlers/default_error_handler.py index dea51dfc6d668..5bcda8231c0f9 100644 --- a/airbyte-cdk/python/airbyte_cdk/sources/declarative/requesters/error_handlers/default_error_handler.py +++ b/airbyte-cdk/python/airbyte_cdk/sources/declarative/requesters/error_handlers/default_error_handler.py @@ -112,13 +112,11 @@ def max_retries(self) -> Union[int, None]: def should_retry(self, response: requests.Response) -> ResponseStatus: request = response.request - if response.ok: - return response_status.SUCCESS + if request not in self._last_request_to_attempt_count: self._last_request_to_attempt_count = {request: 1} else: self._last_request_to_attempt_count[request] += 1 - for response_filter in self._response_filters: filter_action = response_filter.matches(response) if filter_action is not None: @@ -126,6 +124,8 @@ def should_retry(self, response: requests.Response) -> ResponseStatus: return ResponseStatus(ResponseAction.RETRY, self._backoff_time(response, self._last_request_to_attempt_count[request])) else: return ResponseStatus(filter_action) + if response.ok: + return response_status.SUCCESS # Fail if the response matches no filters return response_status.FAIL diff --git a/airbyte-cdk/python/airbyte_cdk/sources/declarative/requesters/error_handlers/http_response_filter.py b/airbyte-cdk/python/airbyte_cdk/sources/declarative/requesters/error_handlers/http_response_filter.py index 1c576f6181562..69790dad9f912 100644 --- a/airbyte-cdk/python/airbyte_cdk/sources/declarative/requesters/error_handlers/http_response_filter.py +++ b/airbyte-cdk/python/airbyte_cdk/sources/declarative/requesters/error_handlers/http_response_filter.py @@ -61,4 +61,5 @@ def _response_contains_error_message(self, response: requests.Response) -> bool: if not self._error_message_contains: return False else: - return self._error_message_contains in HttpStream.parse_response_error_message(response) + error_message = HttpStream.parse_response_error_message(response) + return error_message and self._error_message_contains in error_message diff --git a/airbyte-cdk/python/airbyte_cdk/sources/declarative/retrievers/simple_retriever.py b/airbyte-cdk/python/airbyte_cdk/sources/declarative/retrievers/simple_retriever.py index d915ac78e10c2..367f12c1e0568 100644 --- a/airbyte-cdk/python/airbyte_cdk/sources/declarative/retrievers/simple_retriever.py +++ b/airbyte-cdk/python/airbyte_cdk/sources/declarative/retrievers/simple_retriever.py @@ -7,6 +7,7 @@ import requests from airbyte_cdk.models import SyncMode from airbyte_cdk.sources.declarative.extractors.http_selector import HttpSelector +from airbyte_cdk.sources.declarative.read_exception import ReadException from airbyte_cdk.sources.declarative.requesters.error_handlers.response_action import ResponseAction from airbyte_cdk.sources.declarative.requesters.paginators.no_pagination import NoPagination from airbyte_cdk.sources.declarative.requesters.paginators.paginator import Paginator @@ -263,7 +264,7 @@ def parse_response( # else -> delegate to record selector response_status = self._requester.should_retry(response) if response_status.action == ResponseAction.FAIL: - response.raise_for_status() + raise ReadException(f"Request {response.request} failed with response {response}") elif response_status.action == ResponseAction.IGNORE: self.logger.info(f"Ignoring response for failed request with error message {HttpStream.parse_response_error_message(response)}") return [] diff --git a/airbyte-cdk/python/unit_tests/sources/declarative/requesters/error_handlers/test_default_error_handler.py b/airbyte-cdk/python/unit_tests/sources/declarative/requesters/error_handlers/test_default_error_handler.py index 1ae8ee880b652..b7167f7dfcd81 100644 --- a/airbyte-cdk/python/unit_tests/sources/declarative/requesters/error_handlers/test_default_error_handler.py +++ b/airbyte-cdk/python/unit_tests/sources/declarative/requesters/error_handlers/test_default_error_handler.py @@ -101,6 +101,15 @@ response_status.FAIL, None, ), + ( + "test_200_fail_with_predicate", + HTTPStatus.OK, + HttpResponseFilter(action=ResponseAction.FAIL, error_message_contain="found"), + None, + {}, + response_status.FAIL, + None, + ), ( "test_retry_403", HTTPStatus.FORBIDDEN,