From c0829bd94afd8b2f06a911edc1455ce6706f799d Mon Sep 17 00:00:00 2001 From: Milan Lukac Date: Thu, 2 Jul 2026 14:16:14 +0200 Subject: [PATCH 1/2] fix(check-collections): report runner scan FAILED with logs on abort re-raise MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A single-contract (runner) scan runs with abort_on_first_error=True, so the first construction/verify exception re-raises out of execute_check_collections before phase 3's combined upload — the only place that otherwise ships the engine logs and marks the scan FAILED in Cloud. The exception then reaches the CLI (exit 3) and the launcher only writes it to pod logs, leaving the Cloud scan record FAILED with no logs and the failure undiagnosable. Before the abort re-raise, best-effort mark the still-PENDING runner scan FAILED with the captured logs (the shared Logs gatherer holds every construction/verify record). The re-raise contract is preserved and reporting never masks the original exception. No-op for ad-hoc runs (no scan id). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../soda_core/check_collections/session.py | 39 +++++++++++++++++ ...t_marks_scan_failed_on_connection_error.py | 43 +++++++++++++++++++ 2 files changed, 82 insertions(+) diff --git a/soda-core/src/soda_core/check_collections/session.py b/soda-core/src/soda_core/check_collections/session.py index ea576cda6..67c066bea 100644 --- a/soda-core/src/soda_core/check_collections/session.py +++ b/soda-core/src/soda_core/check_collections/session.py @@ -201,6 +201,7 @@ def execute_check_collections( constructed.append((impl, impl_class, None, yaml_source)) except Exception as exc: if abort_on_first_error: + _report_runner_scan_failed_before_reraise(soda_cloud_impl, publish_results, logs, exc) raise constructed.append((None, impl_class, exc, yaml_source)) @@ -238,6 +239,7 @@ def execute_check_collections( results.append(impl.verify()) except Exception as exc: if abort_on_first_error: + _report_runner_scan_failed_before_reraise(soda_cloud_impl, publish_results, logs, exc) raise results.append(impl_class.build_error_result(yaml_source, exc)) @@ -398,6 +400,43 @@ def execute_check_collections( return CheckCollectionSessionResult(results) +def _report_runner_scan_failed_before_reraise( + soda_cloud_impl: Optional[SodaCloud], + publish_results: bool, + logs: Optional[Logs], + exc: BaseException, +) -> None: + """Best-effort: mark the runner's (still-PENDING) Cloud scan FAILED with the captured logs + just before an ``abort_on_first_error`` re-raise. + + A single-contract (runner) scan aborts on the first construction/verify exception, which + re-raises out of ``execute_check_collections`` before phase 3's combined upload runs — the only + place that otherwise ships logs / marks the scan FAILED. Without this the exception propagates to + the CLI (exit 3) and the launcher only writes it to pod logs, so the Cloud scan record shows + FAILED with no logs and the failure is undiagnosable (SAS-13001). Reporting here keeps the + historical re-raise contract intact while making the failure visible in Cloud. + + No-op when there is no cloud client, no publish opt-in, or no runner scan id (ad-hoc runs). + Never masks the original exception: any error while reporting is swallowed so the caller still + sees the real failure. The shared ``logs`` gatherer holds every construction/verify record (each + impl is built with ``logs=logs``), so its records are exactly what should reach Cloud. + """ + if soda_cloud_impl is None or not publish_results: + return + soda_scan_id: Optional[str] = EnvConfigHelper().soda_scan_id + if not soda_scan_id: + return + try: + log_records = logs.get_log_records() if logs is not None else None + soda_cloud_impl.mark_scan_as_failed( + scan_id=soda_scan_id, + logs=log_records, + exc=exc if isinstance(exc, Exception) else None, + ) + except Exception: + logger.debug("Could not report runner scan as failed before re-raising", exc_info=True) + + def _parse_data_timestamp(value: Optional[Union[str, datetime]]) -> Optional[datetime]: """Coerce a ``data_timestamp`` value to ``Optional[datetime]``. diff --git a/soda-tests/tests/unit/test_contract_marks_scan_failed_on_connection_error.py b/soda-tests/tests/unit/test_contract_marks_scan_failed_on_connection_error.py index 8a71822f6..4ea081b4f 100644 --- a/soda-tests/tests/unit/test_contract_marks_scan_failed_on_connection_error.py +++ b/soda-tests/tests/unit/test_contract_marks_scan_failed_on_connection_error.py @@ -12,8 +12,10 @@ from unittest.mock import patch +import pytest from helpers.mock_soda_cloud import MockSodaCloud from soda_core.common.data_source_impl import DataSourceImpl +from soda_core.common.logging_constants import soda_logger from soda_core.common.yaml import ContractYamlSource, DataSourceYamlSource from soda_core.contracts.contract_verification import ContractVerificationSession from soda_core.contracts.impl.contract_verification_impl import ContractImpl @@ -145,3 +147,44 @@ def test_ad_hoc_run_without_scan_id_still_uploads_results(monkeypatch): "Ad-hoc run has no scan id to mark failed; mark_scan_as_failed must not be used. " f"Requests seen: {request_types}" ) + + +def test_uncaught_exception_during_verify_marks_scan_failed_with_logs(monkeypatch): + """A single-contract (runner) scan that raises an *uncaught* exception during verify aborts + and re-raises before phase 3's combined upload runs. The engine must still report the runner + scan as FAILED with the captured logs, otherwise the Cloud scan record shows no logs and the + failure is undiagnosable (SAS-13001).""" + monkeypatch.setenv("SODA_SCAN_ID", "scan-under-test") + + def _boom(self, *args, **kwargs): + soda_logger.error("Boom: could not build check collection") + raise RuntimeError("verify exploded") + + monkeypatch.setattr(ContractImpl, "verify", _boom) + + data_source_impl = DataSourceImpl.from_yaml_source(DataSourceYamlSource.from_str(_DATA_SOURCE_YAML)) + mock_cloud = MockSodaCloud() + mock_cloud._upload_contract_yaml_file = lambda *args, **kwargs: "contract-file-id" + + # The abort-on-first-error contract path re-raises the exception verbatim. + with pytest.raises(RuntimeError, match="verify exploded"): + ContractVerificationSession.execute( + contract_yaml_sources=[ContractYamlSource.from_str(_CONTRACT_YAML)], + data_source_impls=[data_source_impl], + soda_cloud_impl=mock_cloud, + soda_cloud_publish_results=True, + ) + + request_types = [r.json.get("type") for r in mock_cloud.requests if isinstance(r.json, dict)] + mark_requests = [ + r.json + for r in mock_cloud.requests + if isinstance(r.json, dict) and r.json.get("type") == "sodaCoreMarkScanFailed" + ] + assert mark_requests, ( + "A scan that raised an uncaught exception during verify must be marked FAILED so the " + f"failure is visible in Cloud. Requests seen: {request_types}" + ) + assert mark_requests[0].get("scanId") == "scan-under-test" + # The captured engine logs must be shipped, not an empty payload — that is the whole point. + assert mark_requests[0].get("logs"), "mark-scan-failed must carry the captured engine logs, not an empty payload" From 60306898964bcfe5e65c96618c77aa58b9266aa2 Mon Sep 17 00:00:00 2001 From: Milan Lukac Date: Mon, 6 Jul 2026 16:06:04 +0200 Subject: [PATCH 2/2] review: warn (not debug) on report failure; test construction abort path Addresses review on #2779: - The best-effort "could not report scan as failed" fallback logged at debug, which is easy to miss; if it fires the scan stays FAILED-with-no-logs in Cloud (the exact state this avoids), so bump it to warning for pod/Datadog visibility. Original exception is still re-raised. - The fix reports on both abort pathways (construction + verify) but only verify was tested; add a construction-phase test (exception in ContractImpl.__init__). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../soda_core/check_collections/session.py | 5 ++- ...t_marks_scan_failed_on_connection_error.py | 39 +++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/soda-core/src/soda_core/check_collections/session.py b/soda-core/src/soda_core/check_collections/session.py index 67c066bea..c04bd83ea 100644 --- a/soda-core/src/soda_core/check_collections/session.py +++ b/soda-core/src/soda_core/check_collections/session.py @@ -434,7 +434,10 @@ def _report_runner_scan_failed_before_reraise( exc=exc if isinstance(exc, Exception) else None, ) except Exception: - logger.debug("Could not report runner scan as failed before re-raising", exc_info=True) + # Warning, not debug: if this best-effort report fails the scan stays FAILED-with-no-logs in + # Cloud — the exact state this helper exists to avoid — so it must be visible in the pod logs + # / Datadog rather than silently swallowed. (The original exception is still re-raised.) + logger.warning("Could not report runner scan as failed before re-raising", exc_info=True) def _parse_data_timestamp(value: Optional[Union[str, datetime]]) -> Optional[datetime]: diff --git a/soda-tests/tests/unit/test_contract_marks_scan_failed_on_connection_error.py b/soda-tests/tests/unit/test_contract_marks_scan_failed_on_connection_error.py index 4ea081b4f..3627595ae 100644 --- a/soda-tests/tests/unit/test_contract_marks_scan_failed_on_connection_error.py +++ b/soda-tests/tests/unit/test_contract_marks_scan_failed_on_connection_error.py @@ -188,3 +188,42 @@ def _boom(self, *args, **kwargs): assert mark_requests[0].get("scanId") == "scan-under-test" # The captured engine logs must be shipped, not an empty payload — that is the whole point. assert mark_requests[0].get("logs"), "mark-scan-failed must carry the captured engine logs, not an empty payload" + + +def test_uncaught_exception_during_construction_marks_scan_failed_with_logs(monkeypatch): + """The other abort pathway: an uncaught exception during contract *construction* (phase 1, before + verify) must likewise mark the runner scan FAILED with the captured logs before re-raising. Both + the construction and verify abort points call the same reporting helper (SAS-13001).""" + monkeypatch.setenv("SODA_SCAN_ID", "scan-under-test") + + def _boom_init(self, *args, **kwargs): + soda_logger.error("Boom: could not construct contract") + raise RuntimeError("construction exploded") + + # Fail inside ContractImpl.__init__ so the exception escapes phase-1 construction, not verify(). + monkeypatch.setattr(ContractImpl, "__init__", _boom_init) + + data_source_impl = DataSourceImpl.from_yaml_source(DataSourceYamlSource.from_str(_DATA_SOURCE_YAML)) + mock_cloud = MockSodaCloud() + mock_cloud._upload_contract_yaml_file = lambda *args, **kwargs: "contract-file-id" + + with pytest.raises(RuntimeError, match="construction exploded"): + ContractVerificationSession.execute( + contract_yaml_sources=[ContractYamlSource.from_str(_CONTRACT_YAML)], + data_source_impls=[data_source_impl], + soda_cloud_impl=mock_cloud, + soda_cloud_publish_results=True, + ) + + request_types = [r.json.get("type") for r in mock_cloud.requests if isinstance(r.json, dict)] + mark_requests = [ + r.json + for r in mock_cloud.requests + if isinstance(r.json, dict) and r.json.get("type") == "sodaCoreMarkScanFailed" + ] + assert mark_requests, ( + "A scan that raised an uncaught exception during construction must be marked FAILED. " + f"Requests seen: {request_types}" + ) + assert mark_requests[0].get("scanId") == "scan-under-test" + assert mark_requests[0].get("logs"), "mark-scan-failed must carry the captured engine logs, not an empty payload"