feat(sync_report): emit failure_class on ResourceOutcome for HTTP errors#587
Merged
Merged
Conversation
Extend ResourceOutcome with an optional failure_class field that carries
a structured error taxonomy. _sanitize_reason returns a (reason,
failure_class) tuple; every emit call site forwards both values through.
The field is omitempty in to_dict() so consumers that don't know it see
no schema change.
Today the only signal an NDJSON consumer has for why an outcome failed
is the freetext reason string ("HTTP 500", "HTTP 403", "TimeoutError",
...). Consumers that want to count, alert, or branch on failure modes
must pattern-match the string, which is fragile and duplicates
classification logic that sync-cli already has at exception time. This
PR exposes that classification directly as a structured enum on the
outcome record.
Schema change (additive, optional):
{"type": "outcome", "resource_type": "...", "status": "failure",
"reason": "HTTP 403", "failure_class": "http_4xx_403"}
Canonical failure_class values:
http_4xx_403 CustomClientHTTPError(403)
http_4xx_404 CustomClientHTTPError(404)
http_4xx_429 CustomClientHTTPError(429)
http_4xx_other CustomClientHTTPError(other 4xx)
http_5xx CustomClientHTTPError(5xx)
http_timeout TimeoutError
http_connection ResourceConnectionError
unknown generic Exception fallback
The reason string format is unchanged — every value sync-cli emits today
is still emitted in the same shape, so consumers that don't adopt the
new field see no behavioral change.
Tests: tests/unit/test_failure_class.py covers every _sanitize_reason
branch, omitempty round-trips on to_dict, JSON serialisation, and the
_emit call-site wiring including the dedicated inline except TimeoutError
branch in _import_get_resources_cb.
nathantournant
force-pushed
the
nathan.tournant/failure-class-on-outcomes
branch
from
June 3, 2026 15:01
b45d869 to
54dba13
Compare
nathantournant
marked this pull request as ready for review
June 3, 2026 15:09
heyronhay
approved these changes
Jun 3, 2026
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Extends
ResourceOutcomewith an optionalfailure_classfield thatcarries a structured error taxonomy for HTTP failures.
_sanitize_reasonis updated to return a
(reason, failure_class)tuple, and every emitcall site forwards both values through. The field is
omitempty—absent from the NDJSON record when empty — preserving backward
compatibility for all existing consumers.
Motivation
Today the only signal an NDJSON consumer has for why an outcome failed
is the freetext
reasonstring ("HTTP 500","HTTP 403","TimeoutError", etc.). Consumers that want to count, alert, or branchon failure modes must pattern-match the string, which is fragile and
duplicates classification logic that sync-cli already has at exception
time.
This PR exposes that classification directly as a structured enum on
the outcome record, so consumers can route on
failure_classwithoutparsing.
Schema change
New optional field on
type:"outcome"NDJSON records:{"type": "outcome", "resource_type": "...", "status": "failure", "reason": "HTTP 403", "failure_class": "http_4xx_403"}The field is absent when empty (omitempty). Success / skip / filtered
outcomes never set
failure_class.Canonical values
CustomClientHTTPError(403)HTTP 403http_4xx_403CustomClientHTTPError(404)HTTP 404http_4xx_404CustomClientHTTPError(429)HTTP 429http_4xx_429CustomClientHTTPError(4xx)HTTP <code>http_4xx_otherCustomClientHTTPError(5xx)HTTP <code>http_5xxTimeoutErrorTimeoutErrorhttp_timeoutResourceConnectionErrorconnection_errorhttp_connectionGeneric
Exceptionsubclasses produce(type(err).__name__, "unknown"),unchanged from current behavior on the
reasonside.Backward compatibility
reasonstring format is unchanged — every value sync-cli emitstoday is still emitted in the same shape.
failure_classfield isomitempty, so consumers usingjson.loadsand ignoring unknown keys see no schema change when thefield is absent.
no upgrade lockstep is required.
Test plan
tox -e py312 -- tests/unit/— 675 passed (existing + 30 new).tox -e ruff— clean.tests/unit/test_failure_class.pycovers:_sanitize_reason(HTTP 403/404/429, 4xx-other,5xx, TimeoutError, ResourceConnectionError, generic Exception).
omitemptyround-trips: field absent when empty, present when set._emitwiring: both tuple values forwarded to NDJSON output.except TimeoutError:branch in_import_get_resources_cb(verified consistent with_sanitize_reason).Out of scope
reasonstring format is preserved as-is for backwardcompatibility; classification is purely additive.