fix(oauth): preserve provider error detail on refresh token failure - #1138
fix(oauth): preserve provider error detail on refresh token failure#1138Anatolii Yatsuk (tolik0) wants to merge 2 commits into
Conversation
When an OAuth refresh request is rejected, the CDK replaced the provider's own diagnostic with one fixed sentence. `_wrap_refresh_token_exception` already parsed the error body to decide whether the failure was a refresh token failure and then discarded it. For Microsoft Entra that body carries an AADSTS code which separates completely different root causes: AADSTS50173 (grant revoked, e.g. the user changed their password), AADSTS7000218 / AADSTS700025 (client type or secret misconfiguration) and AADSTS50076 / AADSTS50078 / AADSTS700082 (Conditional Access requiring an interactive sign-in). All of them collapsed into the same string, and no AADSTS code reached production failure summaries. The parsed body is now reused instead of being parsed a second time, the full provider response goes to `internal_message` so it lands in the logs, and a short single-line `error` / `error_description` detail is appended after the existing actionable guidance in the user-facing message. Bodies that are empty, non-JSON or not a JSON object degrade to the previous behaviour without raising. Everything surfaced is run through secret redaction, and the authenticator's own refresh token and client secret are redacted explicitly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. 💡 Show Tips and TricksTesting This CDK VersionYou can test this version of the CDK using the following: # Run the CLI from this branch:
uvx 'git+https://github.com/airbytehq/airbyte-python-cdk.git@tolik0/oauth-preserve-provider-error-detail#egg=airbyte-python-cdk[dev]' --help
# Update a connector to use the CDK from this branch ref:
cd airbyte-integrations/connectors/source-example
poe use-cdk-branch tolik0/oauth-preserve-provider-error-detailPR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
|
PyTest Results (Fast)4 373 tests +10 4 361 ✅ +9 9m 29s ⏱️ +42s Results for commit 6d5e37c. ± Comparison against base commit 4855c2d. This pull request skips 1 test.♻️ This comment has been updated with latest results. |
|
/prerelease
|
Baz (bazarnov)
left a comment
Baz (bazarnov)
left a comment
There was a problem hiding this comment.
Thanks for the change, and for the end-to-end validation on a revoked Bing Ads token. Review only, nothing pushed to the branch; the inline suggestions below are validated as a whole on commit de26718 (merge of main + the suggested edits, CI run on #1144).
Verified:
- Adequacy: source-bing-ads sets no
refresh_token_error_*, so it runs on the declarative defaults(400,) / "error" / ("invalid_grant", "invalid_permissions")(model_to_component_factory.py#L3068), which is exactly the path Microsoft's400 invalid_grantwithAADSTS50173takes. The code reaches the failure summary, the full body reachesinternal_message. - Compatibility:
_wrap_refresh_token_exceptiononly gains an optional kwarg; no connector in airbytehq/airbyte overrides it or_make_handled_request, and nothing asserts on the exact previous message. Not a breaking change. - Redaction order is sound (
filter_secretsfirst, then the authenticator's own refresh token and client secret, before truncation), and only the response body is ever read. - The new tests are discriminating: 6 of the 9 cases fail on
main, including the JSON-array body that previously raisedAttributeErrorinside the error handler.
Suggested changes (inline):
- Keep only the first line of
error_descriptionand cap at 120. Entra appendsTrace ID,Correlation ID,Timestampand issue dates to the description, so today the same failure produces a different user-facing message on every attempt (up to 375 characters, GUIDs inside), which defeats grouping by failure summary - the reason this PR exists. With the change it is one string per failure class, about 295 characters at most, and the full body is still ininternal_message. - Docstring:
AADSTS700082is refresh-token expiry due to inactivity, not Conditional Access (reference-error-codes); the PR description carries the same grouping. - Tests: the redaction test uses credential values equal to their field names and is satisfied by a secret leaked from an earlier test (
add_to_secrets("token")); thetrailing_markercomment is not true for theAADSTS7000218fixture (181 characters, under the cap). Worth adding the declarative-authenticator path that manifest connectors actually run; a ready-made test is in the commit above (unit_tests/sources/declarative/auth/test_oauth.py).
Notes for merge and follow-up:
- Under the defaults,
invalid_client(AADSTS7000218 / AADSTS700025, the latter arriving as HTTP 401) andinteraction_required(AADSTS50078) do not enter this path for source-bing-ads; they surface as the raw HTTP error. If those need the same treatment, setrefresh_token_error_status_codes/refresh_token_error_valuesin the connector manifest. - source-bing-ads pins
source-declarative-manifest:7.25.1; this fix and #1130 (v7.28.3) reach users only after the next CDK patch release and a base-image bump of the connector. - The "300 sampled production failure summaries" figure in the description is not in the oncall thread; worth citing the query or softening.
- The PR is still a draft.
| # How much of the provider's error detail is appended to the user-facing message. Long enough to | ||
| # keep a provider error code and the start of its description (e.g. Microsoft Entra puts the | ||
| # `AADSTS<code>` at the front of `error_description`), short enough to keep the message readable. | ||
| _PROVIDER_ERROR_DETAIL_MAX_LENGTH = 200 |
There was a problem hiding this comment.
With the first-line rule suggested below, 120 still keeps the whole AADSTS50173 sentence (113 characters with the invalid_grant: prefix) but stops before the two issue timestamps Entra puts later on that same line, so the message is identical for every occurrence of a code.
| # How much of the provider's error detail is appended to the user-facing message. Long enough to | |
| # keep a provider error code and the start of its description (e.g. Microsoft Entra puts the | |
| # `AADSTS<code>` at the front of `error_description`), short enough to keep the message readable. | |
| _PROVIDER_ERROR_DETAIL_MAX_LENGTH = 200 | |
| # How much of the provider's error detail is appended to the user-facing message. Long enough to | |
| # keep a provider error code and the start of its description (e.g. Microsoft Entra puts the | |
| # `AADSTS<code>` at the front of `error_description`), short enough to keep the message readable | |
| # and to stop before the per-request values (issue dates, trace ids) that some providers embed | |
| # further into the description, so the same failure produces the same message on every attempt. | |
| _PROVIDER_ERROR_DETAIL_MAX_LENGTH = 120 |
There was a problem hiding this comment.
☑️ Resolved in 6d5e37c. Applied the suggestion verbatim: _PROVIDER_ERROR_DETAIL_MAX_LENGTH = 120 with the updated comment. Combined with the first-line rule, an AADSTS50173 payload with issue dates, Trace ID, Correlation ID and Timestamp now yields a 295-character user-facing message ending at ...a fresh auth token is needed. The us..., identical on every attempt.
Ported from de26718 (#1144) on behalf of ZaneHyattAB while Anatolii Yatsuk (@tolik0) is out.
There was a problem hiding this comment.
Following up in this thread rather than opening a new one, because the resolution above holds for AADSTS50173 but not for every code the docstring now groups with it.
AADSTS50173 is safe, and the arithmetic above checks out. In f"{error}: {error_description}" with error="invalid_grant", the first per-request value (the '<issued-on>' timestamp) starts at offset 191, so the 120 cut lands mid-word at ...a fresh auth token is needed. The us... — identical on every attempt.
AADSTS700082 is not. Entra puts the issue timestamp in the first sentence of that description, not on a later line:
AADSTS700082: The refresh token has expired due to inactivity. The token was issued on <ISO timestamp> and was inactive for <duration>.
The prefix up to the timestamp is 102 characters, so .splitlines()[0] does not remove it and the 120 cap admits ~18 characters of it. Two attempts on the same error code:
... Provider error: invalid_grant: AADSTS700082: The refresh token has expired due to inactivity. The token was issued on 2026-06-11T03:14:0...
... Provider error: invalid_grant: AADSTS700082: The refresh token has expired due to inactivity. The token was issued on 2026-07-29T22:01:5...
Both are ~295 characters, matching the estimate above, but they are different strings. On main those same two responses produced one byte-identical message, so for this code the change moves failure-summary cardinality from exactly 1 to unbounded — the grouping regression this thread set out to fix, on the code that #1138 (comment) deliberately regrouped into the expired/revoked bucket.
And no test can catch it. Mutating the file one change at a time against the 86 tests in the two touched modules:
- delete
.splitlines()[0]→ 86 passed. Every fixture's trace id sits past character 120, so the cap alone removes it, which makes bothtrailing_marker not in exc_info.value.messageand"Trace ID" not in ...messagevacuous. _PROVIDER_ERROR_DETAIL_MAX_LENGTH = 120→2000→ 86 passed. The truncation test assertslen(provider_detail) == _PROVIDER_ERROR_DETAIL_MAX_LENGTH + len("...")against the imported constant, so it can never pin its value, and its"x" * 5000fixture passes for any cap below ~5028.
Only the combined mutant fails, so the two guards mask each other and the realistic future regression — raising the cap to fit more description while keeping the first-line rule — ships green.
Options, in the order I'd prefer them:
- Append the provider code, not the prose.
^[A-Za-z]{3,}\d{4,}against the description yieldsAADSTS700082, which is the entire grouping key. Immune to sentence shape by construction, and it also bounds how much provider-controlled text lands in the persisted failure summary. - Keep the prose, strip per-request values before truncating —
re.sub(r"\d{4}-\d{2}-\d{2}T[\d:.]+Z?", ...)plus a GUID pattern. - Cut at the last sentence boundary at or before the cap, so a partial timestamp can never terminate the string.
Whichever you pick, two fixtures would pin the property this thread is about, each verified to fail on exactly one of the mutants above:
- both timestamps in the first sentence and no later lines (first line 335 chars): assert
"AADSTS50173" in messageandre.search(r"\d{4}-\d{2}-\d{2}T", message) is None. Passes at 120, fails at 2000 regardless ofsplitlines. error_description = "AADSTS50173: grant revoked.\r\nTrace ID: 00000000-0000-0000-0000-000000000000"(collapsed length 89, under the cap): assert"Trace ID" not in message. Passes withsplitlines, fails without it at any cap.
Plus the property itself, asserted directly: the same AADSTS700082 payload twice with different issue dates, asserting the two message values are equal.
Reviewed at 6d5e37c9. The offsets are against Microsoft's published error_description wording; the mechanism — a character cap cannot separate stable text from per-request text — does not depend on it.
There was a problem hiding this comment.
Confirmed on 6d5e37c9: two AADSTS700082 payloads differing only in the issue timestamp produce two distinct user-facing messages, both ending mid-timestamp (...The token was issued on 2026-06-11T03:14:0... vs ...2026-07-29T22:01:5...). The first-line rule plus the 120 cap is not sufficient for that code, as you describe.
I was authorized only to port Baz (@bazarnov)'s already-validated edits from #1144, so I am not going to pick among options 1-3 unilaterally; that is a design choice for you / Baz (@bazarnov) / ZaneHyattAB. For what it's worth, option 1 (append the extracted ^[A-Za-z]{3,}\d{4,} code rather than the prose) is the only one that is stable by construction and also bounds provider-controlled text in the persisted summary; options 2 and 3 keep depending on the sentence shape. Whichever is chosen, I agree the three fixtures you list (first-sentence timestamps with no later lines, short description with a \r\n trace id, and the same code twice with different dates asserting equal message) should land with it so the cap and the first-line rule cannot mask each other again.
Happy to implement once one option is picked.
There was a problem hiding this comment.
🙋 Human Input Needed: Anatolii Yatsuk (@tolik0) / Baz (@bazarnov) / ZaneHyattAB — please pick option 1, 2 or 3 above (repro confirmed, see previous reply); I will implement it with the three pinning fixtures once decided.
| Only the standard OAuth 2.0 error fields (`error` and `error_description`) are used: they | ||
| are where providers put the actionable diagnostic, and they are not expected to carry | ||
| credentials. For Microsoft Entra this is what distinguishes a revoked grant | ||
| (`AADSTS50173`) from a client-type or secret misconfiguration (`AADSTS7000218`, | ||
| `AADSTS700025`) from Conditional Access requiring interactive sign-in (`AADSTS50076`, | ||
| `AADSTS50078`, `AADSTS700082`) - all of which otherwise collapse into the same message. |
There was a problem hiding this comment.
AADSTS700082 is ExpiredOrRevokedGrantInactiveToken - the refresh token expired after inactivity (reference-error-codes), so it belongs with the expired/revoked group, not with Conditional Access (the PR description has the same grouping). Also worth stating that reachability depends on refresh_token_error_*: on the declarative defaults (400,) / "error" / ("invalid_grant", "invalid_permissions") that source-bing-ads runs on, invalid_client (AADSTS7000218, AADSTS700025) and interaction_required (AADSTS50078) never get here.
| Only the standard OAuth 2.0 error fields (`error` and `error_description`) are used: they | |
| are where providers put the actionable diagnostic, and they are not expected to carry | |
| credentials. For Microsoft Entra this is what distinguishes a revoked grant | |
| (`AADSTS50173`) from a client-type or secret misconfiguration (`AADSTS7000218`, | |
| `AADSTS700025`) from Conditional Access requiring interactive sign-in (`AADSTS50076`, | |
| `AADSTS50078`, `AADSTS700082`) - all of which otherwise collapse into the same message. | |
| Only the standard OAuth 2.0 `error` and `error_description` fields (RFC 6749 section 5.2) | |
| are used, and only the first line of the description: providers put the actionable code | |
| there (Microsoft Entra leads with `AADSTS<code>`, which tells a revoked or expired grant | |
| such as `AADSTS50173` / `AADSTS700082` apart from a client misconfiguration such as | |
| `AADSTS7000218`), while per-request trace ids and timestamps follow on later lines. Which | |
| provider errors reach this path at all is set by the authenticator's `refresh_token_error_*` | |
| configuration. |
There was a problem hiding this comment.
☑️ Resolved in 6d5e37c. Docstring replaced with your suggested text: AADSTS700082 now sits with the expired/revoked grants next to AADSTS50173, and the docstring states that reachability is set by the authenticator's refresh_token_error_* configuration. The PR description was updated with the same regrouping and spells out that on the declarative defaults invalid_client (AADSTS7000218, AADSTS700025) and interaction_required (AADSTS50078) never reach this path.
| parts = [ | ||
| str(response_content[key]) | ||
| for key in ("error", "error_description") | ||
| if response_content.get(key) | ||
| ] | ||
| if not parts: | ||
| return None | ||
| # Collapse newlines and repeated whitespace so the detail stays on a single line. | ||
| detail = " ".join(": ".join(parts).split()) |
There was a problem hiding this comment.
Entra's error_description is AADSTS<code>: <sentence>\r\nTrace ID: <guid>\r\nCorrelation ID: <guid>\r\nTimestamp: ... (token endpoint error response). Collapsing whitespace pulls those onto the user-facing line: for AADSTS7000218 the whole detail is 181 characters, so both GUIDs land in message; for AADSTS50173 the 200 cut lands after the first issue timestamp. Every attempt then carries a distinct 375-character message, which defeats grouping by failure summary (the 4,074-row count in the oncall issue keys on exactly that). First line only plus the 120 cap gives one string per code, about 295 characters in total, with the full body still in internal_message. The isinstance guard also avoids rendering a dict repr when a provider nests the error object.
| parts = [ | |
| str(response_content[key]) | |
| for key in ("error", "error_description") | |
| if response_content.get(key) | |
| ] | |
| if not parts: | |
| return None | |
| # Collapse newlines and repeated whitespace so the detail stays on a single line. | |
| detail = " ".join(": ".join(parts).split()) | |
| parts = [ | |
| response_content[key].strip() | |
| for key in ("error", "error_description") | |
| if isinstance(response_content.get(key), str) and response_content[key].strip() | |
| ] | |
| if not parts: | |
| return None | |
| # Keep the first line only and collapse its whitespace: the actionable code leads the | |
| # description, while trace ids and timestamps that differ on every attempt follow on later | |
| # lines and would make the same failure read differently each time. | |
| detail = " ".join(": ".join(parts).splitlines()[0].split()) |
There was a problem hiding this comment.
☑️ Resolved in 6d5e37c. Applied your suggested block verbatim: isinstance(..., str) guard with .strip() on error / error_description, and detail = " ".join(": ".join(parts).splitlines()[0].split()) so only the first line reaches the user-facing message. Verified locally with an Entra-style AADSTS50173 body: message carries the code and no Trace ID / Correlation ID; internal_message still carries the full body.
| # Sits at the very end of each provider payload below, past the point where the user-facing | ||
| # message is truncated, so it can only be found in the internal message. |
There was a problem hiding this comment.
This is not true for the AADSTS7000218 fixture: its collapsed detail is 181 characters, under the 200 cap, so the marker lands in the user-facing message as well, and nothing catches it because the test only asserts the marker is in internal_message. With the first-line change the comment becomes accurate; either way, assert TestOauth2Authenticator.trailing_marker not in exc_info.value.message next to the internal_message assertion pins the user-facing side (it fails on the current code, passes with the change).
| # Sits at the very end of each provider payload below, past the point where the user-facing | |
| # message is truncated, so it can only be found in the internal message. | |
| # Sits on a later line of each provider payload below, like the trace ids and timestamps | |
| # Microsoft Entra appends, so it must reach the internal message and never the user-facing one. |
There was a problem hiding this comment.
☑️ Resolved in 6d5e37c. Replaced the trailing_marker comment with your wording and added assert TestOauth2Authenticator.trailing_marker not in exc_info.value.message next to the internal_message assertion in test_refresh_access_token_surfaces_provider_error_code. Also took the fixture-text and == _PROVIDER_ERROR_DETAIL_MAX_LENGTH + len("...") truncation tweaks from de26718.
| oauth = self._entra_style_authenticator() | ||
| requests_mock.post( | ||
| f"https://{TestOauth2Authenticator.refresh_endpoint}", | ||
| status_code=400, | ||
| json={ | ||
| "error": "invalid_grant", | ||
| "error_description": ( | ||
| f"AADSTS50173: rejected refresh_token={TestOauth2Authenticator.refresh_token} " | ||
| f"client_secret={TestOauth2Authenticator.client_secret}" | ||
| ), | ||
| }, | ||
| ) | ||
|
|
||
| with pytest.raises(AirbyteTracedException) as exc_info: | ||
| oauth.refresh_access_token() | ||
|
|
||
| for message in (exc_info.value.message, exc_info.value.internal_message): | ||
| assert TestOauth2Authenticator.refresh_token not in message | ||
| assert TestOauth2Authenticator.client_secret not in message | ||
| assert "****" in message |
There was a problem hiding this comment.
TestOauth2Authenticator.refresh_token is the literal "refresh_token" and client_secret is "client_secret", so these assertions cannot tell a redacted value from a redacted field name. On top of that, test_refresh_access_token_retry runs earlier and calls add_to_secrets("token") on its successful refresh, after which filter_secrets turns refresh_token into refresh_**** on its own: with the explicit pair redaction disabled, refresh_token not in message and "****" in message still pass. Distinct values plus a reset of the config-level secrets make the test exercise the redaction this PR adds (needs from airbyte_cdk.utils.airbyte_secrets_utils import update_secrets at the top).
| oauth = self._entra_style_authenticator() | |
| requests_mock.post( | |
| f"https://{TestOauth2Authenticator.refresh_endpoint}", | |
| status_code=400, | |
| json={ | |
| "error": "invalid_grant", | |
| "error_description": ( | |
| f"AADSTS50173: rejected refresh_token={TestOauth2Authenticator.refresh_token} " | |
| f"client_secret={TestOauth2Authenticator.client_secret}" | |
| ), | |
| }, | |
| ) | |
| with pytest.raises(AirbyteTracedException) as exc_info: | |
| oauth.refresh_access_token() | |
| for message in (exc_info.value.message, exc_info.value.internal_message): | |
| assert TestOauth2Authenticator.refresh_token not in message | |
| assert TestOauth2Authenticator.client_secret not in message | |
| assert "****" in message | |
| # Start from no config-level secrets: earlier tests register values such as "token" through | |
| # add_to_secrets, which would otherwise mask the credentials on their own. | |
| update_secrets([]) | |
| refresh_token = "0.AXoA-rt-9f3ZqW7kP2" | |
| client_secret = "s3cr3t~Xyz-1Qp" | |
| oauth = Oauth2Authenticator( | |
| f"https://{TestOauth2Authenticator.refresh_endpoint}", | |
| TestOauth2Authenticator.client_id, | |
| client_secret, | |
| refresh_token, | |
| refresh_token_error_status_codes=(400,), | |
| refresh_token_error_key="error", | |
| refresh_token_error_values=("invalid_grant",), | |
| ) | |
| requests_mock.post( | |
| f"https://{TestOauth2Authenticator.refresh_endpoint}", | |
| status_code=400, | |
| json={ | |
| "error": "invalid_grant", | |
| "error_description": f"AADSTS50173: rejected rt={refresh_token} cs={client_secret}", | |
| }, | |
| ) | |
| with pytest.raises(AirbyteTracedException) as exc_info: | |
| oauth.refresh_access_token() | |
| for message in (exc_info.value.message, exc_info.value.internal_message): | |
| assert refresh_token not in message | |
| assert client_secret not in message | |
| assert message.count("****") == 2 |
There was a problem hiding this comment.
☑️ Resolved in 6d5e37c. Redaction test now uses distinct refresh_token / client_secret values, calls update_secrets([]) first, and asserts message.count("****") == 2. Also ported your DeclarativeOauth2Authenticator test into unit_tests/sources/declarative/auth/test_oauth.py for the manifest-connector path.
Sabotage check: with the for get_credential in (self.get_refresh_token, self.get_client_secret) loop in _redact_credentials emptied, both redaction tests fail; with it restored, all 216 tests in the two directories pass.
Apply bazarnov's review on #1138, ported from de26718 (#1144): - cap provider detail at 120 chars and keep only the first line of error_description, so Entra trace ids / timestamps stay out of the user-facing message and the same failure yields the same summary - guard error/error_description on isinstance(str) - docstring: AADSTS700082 is an expired/revoked grant; reachability depends on refresh_token_error_* - tests: distinct credential values + update_secrets([]) so the redaction test exercises the explicit pair redaction; assert trailing marker is absent from the user-facing message; add declarative authenticator test Co-authored-by: bazarnov <bazarnov@users.noreply.github.com> Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
3 participants
What
When an OAuth refresh request is rejected,
AbstractOauth2Authenticator._make_handled_requestraised anAirbyteTracedExceptionwhose user-facing message was one fixed sentence, and it threw away the provider's own diagnostic._wrap_refresh_token_exceptionhad already parsed the error body withexception.response.json()in order to decide whether the failure was a refresh-token failure, and then discarded that parsed body.That matters most for Microsoft Entra, where the body carries an AADSTS code that separates root causes needing completely different fixes:
AADSTS50173— the grant was revoked, typically because the user changed or reset their password. Re-authenticating is the fix.AADSTS7000218/AADSTS700025— the app registration's client type or client credential is misconfigured. Re-authenticating does not help; the app registration has to change.AADSTS50076/AADSTS50078/AADSTS700082— Conditional Access requires an interactive sign-in or MFA. Again a different fix, and one the workspace admin has to make.Today all three collapse into the same string. Across 300 sampled production failure summaries for source-bing-ads, zero contain an AADSTS code, so support has no way to tell these apart from the failure summary. See airbytehq/oncall#12835.
How
_make_handled_request, and passed into_wrap_refresh_token_exceptionthrough a new optionalresponse_contentargument, so the response is no longer parsed twice. The argument defaults toNoneand the method parses on demand when it is absent, so existing callers keep working.internal_messagecarries the full provider response (HTTP <status>: <body>, truncated at 1000 characters), so the whole payload lands in the logs.messagedeliberately keeps the existing actionable sentence as its lead: "Refresh token was rejected by the OAuth provider (invalid, expired, or already used). Re-authenticate this source's credentials in its connection settings." Only after that is a short provider detail appended, asProvider error: <error>: <error_description>, built from the standard OAuth 2.0erroranderror_descriptionfields, collapsed to a single line and truncated at 200 characters. Two hundred characters is enough to keep theAADSTS<code>and the beginning of its description, since Entra puts the code at the front oferror_description, while staying short enough that the failure summary is still readable. No raw provider blob becomes the primary message, and when the body has no usableerror/error_descriptionthe message is byte-for-byte what it was before.AttributeErroron.get(...)inside the already-failing error path.filter_secrets, and the authenticator's own refresh token and client secret are redacted explicitly on top of that, in case a provider echoes submitted credentials back in its payload. Only response bodies are read, so request headers, includingAuthorization, are never echoed.No changelog entry or version bump is included:
CONTRIBUTING.mdstates releases are drafted automatically bysemantic-pr-release-drafterfrom the PR title, and the package version is computed bypoetry-dynamic-versioning.CHANGELOG.mdis frozen and points at GitHub Release Notes.Test plan
New tests in
unit_tests/sources/streams/http/requests_native_auth/test_requests_native_auth.py:internal_messagewhilemessagestill starts with the re-authenticate guidance and then carries the provider code on a single line;error_description;RequestExceptioninstead of raising something new;error/error_description, asserting the user-facing message is exactly the unchanged sentence.The existing
test_refresh_access_token_wrappedassertion onmessagewas relaxed from equality tostartswith, since the wrapped case now appendsProvider error: invalid_grant.This repo is Poetry-managed, so
uv run pytestcannot resolve the dev dependencies; the tests were run with the project's Poetry virtualenv:🤖 Generated with Claude Code
Update — 2026-09-03 (added by ZaneHyattAB on Anatolii Yatsuk (@tolik0)'s behalf)
Anatolii Yatsuk (@tolik0) is out, and this PR now carries commit
6d5e37c9applyingBaz (@bazarnov)'s review feedback. Everything above is Anatolii Yatsuk (@tolik0)'s original text,
left as written. The points below supersede it where they conflict; the
original wording is kept for context rather than rewritten.
The user-facing detail is now the first line of
error_descriptiononly, capped at 120 characters (was: single-line collapse, 200
characters). Entra appends
Trace ID,Correlation IDandTimestamponlater lines and embeds per-request issue dates in the first sentence, so
collapsing whitespace pulled per-request GUIDs into the message. Every
attempt then produced a distinct ~375-character string, which defeats
grouping by failure summary — the capability this PR exists to provide.
After the change the message is a stable ~295 characters per error code,
with the full body still in
internal_message.AADSTS700082is regrouped. It isExpiredOrRevokedGrantInactiveToken— refresh-token expiry afterinactivity — so it belongs with the expired/revoked group above, not with
Conditional Access.
Reachability caveat added. On the declarative defaults
source-bing-adsruns ((400,)/"error"/("invalid_grant", "invalid_permissions")),invalid_client(
AADSTS7000218,AADSTS700025) andinteraction_required(
AADSTS50078) never reach this path and surface as raw HTTP errors.Only string values are read, so a provider nesting the error object no
longer renders a dict repr into the message.
The redaction test now actually tests the redaction. It uses distinct
credential values and calls
update_secrets([])first, because secretsregistered by earlier tests were masking the credentials on their own —
the previous test passed even with the explicit pair redaction disabled.
Verified by disabling that redaction and confirming the test fails.
New declarative-authenticator test at
unit_tests/sources/declarative/auth/test_oauth.py, covering the pathmanifest-only connectors such as
source-bing-adsactually run.Retracted: the "300 sampled production failure summaries" figure above is
not sourced in airbytehq/oncall#12835 and should not be relied on. The
verified claim is narrower: a deliberately revoked Bing Ads token on
3.0.7produced a 95-line
checklog containing zero AADSTS codes, and the sametoken on a preview build of this PR surfaced the code.
Validation on
6d5e37c9: 216 targeted tests pass,ruffandmypyclean.Additionally validated end to end in Airbyte Cloud on a genuinely revoked
token — see airbytehq/oncall#12835.
Anatolii Yatsuk (@tolik0), please adjust or fold this in as you see fit when you're back.