airbytehq / airbytehq/airbyte-python-cdk

CompositeErrorHandler swallows REFRESH_TOKEN_THEN_RETRY matched by earlier handlers

Đang mở Phù hợp với người mới
#1,080 0 bình luận 0 reaction 0 người được giao Xem trên GitHub
Ngôn ngữ chính
Python
Star
26
Fork
53
Merge trung bình
2 ngày 6 giờ
Pull request đã merge (30 ngày)
10

Mô tả

## Summary

`CompositeErrorHandler.interpret_response` swallows `REFRESH_TOKEN_THEN_RETRY` (and `RATE_LIMITED`) resolutions matched by an earlier handler in the chain: the loop only short-circuits on `SUCCESS`, `RETRY`, `IGNORE`, and `RESET_PAGINATION`, so the matched resolution is overwritten by the next handler's result (typically `FAIL` from the default 401 mapping).

## Reproduction

```python
import requests
from unittest.mock import MagicMock
from airbyte_cdk.sources.declarative.requesters.error_handlers.composite_error_handler import CompositeErrorHandler
from airbyte_cdk.sources.declarative.requesters.error_handlers.default_error_handler import DefaultErrorHandler
from airbyte_cdk.sources.declarative.requesters.error_handlers.http_response_filter import HttpResponseFilter

config = {}
refresh_filter = HttpResponseFilter(
action="REFRESH_TOKEN_THEN_RETRY",
predicate="{{ response.get('code') == 124 }}",
config=config,
parameters={},
)
h1 = DefaultErrorHandler(config=config, parameters={}, response_filters=[refresh_filter])
h2 = DefaultErrorHandler(config=config, parameters={})
composite = CompositeErrorHandler(error_handlers=[h1, h2], parameters={})

resp = MagicMock(spec=requests.Response)
resp.status_code = 401
resp.json.return_value = {"code": 124, "message": "Access token is expired."}
resp.headers = {}
resp.ok = False

print(composite.interpret_response(resp).response_action)
# Actual: ResponseAction.FAIL
# Expected: ResponseAction.REFRESH_TOKEN_THEN_RETRY
```

Verified on CDK 7.23.6 and current main.

## Root cause

In `airbyte_cdk/sources/declarative/requesters/error_handlers/composite_error_handler.py`, `interpret_response` only returns early for:

```python
if matched_error_resolution.response_action in [
ResponseAction.SUCCESS,
ResponseAction.RETRY,
ResponseAction.IGNORE,
ResponseAction.RESET_PAGINATION,
]:
return matched_error_resolution
```

A `REFRESH_TOKEN_THEN_RETRY` match falls through, `matched_error_resolution` is overwritten by the next handler in the loop, and the last handler's resolution (e.g. `FAIL` for a 401 via the default mapping) wins.

## Impact

Any manifest that declares a `REFRESH_TOKEN_THEN_RETRY` response filter inside a `CompositeErrorHandler` (a very common pattern, since composites are the documented way to layer custom filters over the default handler) silently never refreshes/retries — long-running syncs fail with a fatal 401 at the token-expiry boundary. This was observed in production with source-zoom (see airbytehq/airbyte#72784).

Workaround: place the `REFRESH_TOKEN_THEN_RETRY` filter inside every `DefaultErrorHandler` in the composite (including the fallback handler), so the last-evaluated handler also returns it.

## Suggested fix

Add `ResponseAction.REFRESH_TOKEN_THEN_RETRY` (and likely `ResponseAction.RATE_LIMITED`) to the early-return list in `CompositeErrorHandler.interpret_response`.

Reported on behalf of Ryan Waskewich.

---
[Devin session](https://app.devin.ai/sessions/d3994b352075445db4298c4cf7649b18)

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Hướng nghiên cứu

Read airbyte_cdk/sources/declarative/requesters/error_handlers/composite_error_handler.py and run the supplied reproduction to observe the overwritten response action. Confirm that matched REFRESH_TOKEN_THEN_RETRY and RATE_LIMITED resolutions are preserved through the handler chain while existing terminal actions remain unchanged.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Đánh giá

Công nghệ
python
Lĩnh vực
api, backend
Loại issue
Lỗi
Độ khó
2/5
Thời gian dự kiến
1-3 giờ
Mức độ hoạt động
Ít trao đổi
Độ rõ ràng
Đặc tả rõ ràng
Mức phù hợp với người mới
76/100

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.