Skip to content

Commit 8e92512

Browse files
fix(file-based): surface actionable errors instead of full Python tracebacks during connection check (#1012)
Co-authored-by: devin-ai-integration[bot] <devin-ai-integration[bot]@users.noreply.github.com>
1 parent 886fcf8 commit 8e92512

2 files changed

Lines changed: 84 additions & 10 deletions

File tree

airbyte_cdk/sources/file_based/availability_strategy/default_file_based_availability_strategy.py

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
from __future__ import annotations
66

77
import logging
8-
import traceback
98
from typing import TYPE_CHECKING, Optional, Tuple
109

1110
from airbyte_cdk import AirbyteTracedException
@@ -39,12 +38,18 @@ def check_availability( # type: ignore[override] # Signature doesn't match bas
3938
"""
4039
Perform a connection check for the stream (verify that we can list files from the stream).
4140
42-
Returns (True, None) if successful, otherwise (False, <error message>).
41+
Returns `(True, None)` if successful, otherwise `(False, <error message>)`.
42+
43+
The returned reason is the actionable error string from the underlying
44+
`CheckAvailabilityError`. Connector-raised `AirbyteTracedException`s are
45+
re-raised so that their actionable `message` propagates to the platform
46+
unchanged. The full traceback is still logged for debugging.
4347
"""
4448
try:
4549
self._check_list_files(stream)
46-
except CheckAvailabilityError:
47-
return False, "".join(traceback.format_exc())
50+
except CheckAvailabilityError as exc:
51+
logger.exception("Stream availability check failed")
52+
return False, str(exc)
4853

4954
return True, None
5055

@@ -83,25 +88,33 @@ def check_availability_and_parsability(
8388
# If the parser is set to not check parsability, we still want to check that we can open the file.
8489
handle = stream.stream_reader.open_file(file, parser.file_read_mode, None, logger)
8590
handle.close()
86-
except AirbyteTracedException as ate:
87-
raise ate
88-
except CheckAvailabilityError:
89-
return False, "".join(traceback.format_exc())
91+
except AirbyteTracedException:
92+
raise
93+
except CheckAvailabilityError as exc:
94+
logger.exception("Stream availability check failed")
95+
return False, str(exc)
9096

9197
return True, None
9298

9399
def _check_list_files(self, stream: AbstractFileBasedStream) -> RemoteFile:
94100
"""
95101
Check that we can list files from the stream.
96102
97-
Returns the first file if successful, otherwise raises a CheckAvailabilityError.
103+
Returns the first file if successful, otherwise raises a `CheckAvailabilityError`.
104+
105+
`AirbyteTracedException`s raised by the underlying stream reader are
106+
re-raised unchanged so that connector-specific actionable messages
107+
(e.g. invalid credentials, missing permissions) reach the platform
108+
without being wrapped in a generic `ERROR_LISTING_FILES` reason.
98109
"""
99110
try:
100111
file = next(iter(stream.get_files()))
101112
except StopIteration:
102113
raise CheckAvailabilityError(FileBasedSourceError.EMPTY_STREAM, stream=stream.name)
103114
except CustomFileBasedException as exc:
104115
raise CheckAvailabilityError(str(exc), stream=stream.name) from exc
116+
except AirbyteTracedException:
117+
raise
105118
except Exception as exc:
106119
raise CheckAvailabilityError(
107120
FileBasedSourceError.ERROR_LISTING_FILES, stream=stream.name

unit_tests/sources/file_based/availability_strategy/test_default_file_based_availability_strategy.py

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@
66
from datetime import datetime
77
from unittest.mock import Mock, PropertyMock
88

9+
import pytest
10+
11+
from airbyte_cdk import AirbyteTracedException, FailureType
912
from airbyte_cdk.sources.file_based.availability_strategy.default_file_based_availability_strategy import (
1013
DefaultFileBasedAvailabilityStrategy,
1114
)
@@ -61,7 +64,9 @@ def test_given_file_extension_does_not_match_when_check_availability_and_parsabi
6164

6265
def test_not_available_given_no_files(self) -> None:
6366
"""
64-
If no files are returned, then the stream is not available.
67+
If no files are returned, then the stream is not available, and the
68+
reason is the actionable `EMPTY_STREAM` message rather than a Python
69+
traceback.
6570
"""
6671
self._stream.get_files.return_value = []
6772

@@ -71,6 +76,62 @@ def test_not_available_given_no_files(self) -> None:
7176

7277
assert not is_available
7378
assert "No files were identified in the stream" in reason
79+
assert "Traceback" not in reason
80+
assert "raise CheckAvailabilityError" not in reason
81+
82+
def test_check_availability_returns_actionable_reason_when_no_files(self) -> None:
83+
"""
84+
`check_availability` (used by file-transfer / permissions-transfer modes)
85+
must also return the actionable reason rather than a traceback string.
86+
"""
87+
self._stream.get_files.return_value = []
88+
89+
is_available, reason = self._strategy.check_availability(self._stream, Mock(), Mock())
90+
91+
assert not is_available
92+
assert "No files were identified in the stream" in reason
93+
assert "Traceback" not in reason
94+
assert "raise CheckAvailabilityError" not in reason
95+
96+
def test_airbyte_traced_exception_from_stream_reader_propagates(self) -> None:
97+
"""
98+
When the underlying stream reader raises an `AirbyteTracedException`
99+
(e.g. invalid credentials), the actionable message must propagate
100+
unchanged instead of being wrapped in a generic `ERROR_LISTING_FILES`
101+
reason.
102+
"""
103+
self._stream.get_files.side_effect = AirbyteTracedException(
104+
internal_message="raw provider error",
105+
message="Could not authenticate with Google drive. Please check your credentials.",
106+
failure_type=FailureType.config_error,
107+
)
108+
109+
with pytest.raises(AirbyteTracedException) as exc_info:
110+
self._strategy.check_availability_and_parsability(self._stream, Mock(), Mock())
111+
112+
assert (
113+
exc_info.value.message
114+
== "Could not authenticate with Google drive. Please check your credentials."
115+
)
116+
assert exc_info.value.failure_type == FailureType.config_error
117+
118+
def test_airbyte_traced_exception_propagates_in_check_availability(self) -> None:
119+
"""
120+
Same propagation guarantee as above for `check_availability`.
121+
"""
122+
self._stream.get_files.side_effect = AirbyteTracedException(
123+
internal_message="raw provider error",
124+
message="Could not authenticate with Google drive. Please check your credentials.",
125+
failure_type=FailureType.config_error,
126+
)
127+
128+
with pytest.raises(AirbyteTracedException) as exc_info:
129+
self._strategy.check_availability(self._stream, Mock(), Mock())
130+
131+
assert (
132+
exc_info.value.message
133+
== "Could not authenticate with Google drive. Please check your credentials."
134+
)
74135

75136
def test_parse_records_is_not_called_with_parser_max_n_files_for_parsability_set(self) -> None:
76137
"""

0 commit comments

Comments
 (0)