airbytehq / airbytehq/airbyte-python-cdk

CompositeErrorHandler swallows REFRESH_TOKEN_THEN_RETRY matched by earlier handlers

Open Beginner friendly
#1,080 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
26
Forks
53
Avg merge
2d 6h
Merged PRs (30d)
10

Description

## 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)

Contributor guide

Open the contributing guide

Research direction

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.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
api, backend
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.