test(wac): pin the failure record with one corpus both SDKs read (#10385)

* test(wac): pin the failure record with one corpus both SDKs read

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test(wac): add the behaviour matrix that verified the failure record

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs(wac): record how to exercise an unreleased SDK change

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(wac): guard the whole extra pair, not just its value

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(wac): never rehash an untrusted extra key

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(wac): walk only a real __dict__ when collecting extra

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs(wac): name the divergence the corpus cannot pin

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* chore(wac): state the extra-encoding constraints in four lines

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

python-client/wmill/tests/test_workflow.py

@@ -2,6 +2,7 @@
 
 import asyncio
 import json
+import pathlib
 import pytest
 
 from datetime import datetime, timezone
@@ -9,6 +10,16 @@ from datetime import datetime, timezone
 from wmill.client import WorkflowCtx, _StepSuspend, TaskError, workflow, task, step, sleep, parallel, wait_for_approval, _run_workflow, _run_workflow_async
 
 
+class _FakeItems(dict):
+    """A mapping whose ``items()`` yields something that is not a key/value pair."""
+
+    def __bool__(self):
+        return True
+
+    def items(self):
+        return [None]
+
+
 class _StubInlineClient:
     """Stands in for the httpx client the inline fast path POSTs with.
 
@@ -968,6 +979,59 @@ class TestErrorPropagation:
         assert "step failed" in r["result"]["caught"]
 
 
+class TestFailureRecordCorpus:
+    """The cases both SDKs must agree on, from the corpus the typescript suite
+    also reads. Its `_readme` states the contract and why it is shared."""
+
+    CORPUS = json.loads(
+        (
+            pathlib.Path(__file__).resolve().parents[3]
+            / "backend/windmill-common/src/wac_failure_corpus.json"
+        ).read_text()
+    )
+
+    @staticmethod
+    def _construct(spec: dict) -> BaseException:
+        exc = type(spec["name"], (Exception,), {})(spec["message"])
+        for key, value in (spec.get("props") or {}).items():
+            setattr(exc, key, value)
+        if spec.get("circular_prop"):
+            cyclic: dict = {}
+            cyclic["self"] = cyclic
+            setattr(exc, spec["circular_prop"], cyclic)
+        return exc
+
+    @pytest.mark.parametrize("case", CORPUS["cases"], ids=lambda c: c["case"])
+    def test_marker_matches_the_shared_contract(self, case):
+        exc = self._construct(case["thrown"])
+
+        def raiser():
+            raise exc
+
+        # through the real step path, so the stack is the one a failure actually
+        # records rather than one this test happens to construct
+        async def run():
+            ctx = WorkflowCtx({})
+            try:
+                await ctx._run_inline_step("k", raiser)
+            except _StepSuspend as suspended:
+                return suspended.dispatch_info["result"]
+            raise AssertionError("a raising step did not suspend")
+
+        error = asyncio.run(run())["result"]["error"]
+
+        expect = case["expect"]
+        assert error["name"] == expect["name"]
+        assert error["message"] == expect["message"]
+        assert ("stack" in error) == (expect["stack"] == "present")
+        if "extra" in expect:
+            assert error["extra"] == expect["extra"]
+        for absent in expect.get("absent", []):
+            assert absent not in error
+        # whatever it kept has to survive the trip to the checkpoint
+        json.dumps(error, allow_nan=False)
+
+
 class TestRaisingInlineStepIsCheckpointed:
     """A ``step()`` whose body raises must still land in ``completed_steps``.
 
@@ -1092,6 +1156,52 @@ class TestRaisingInlineStepIsCheckpointed:
         _json.dumps(marker)
         assert marker["result"]["error"]["name"] == "HostileDict"
 
+        # ...or an overridden __dict__, whatever shape it takes
+        class NotAMapping(Exception):
+            @property
+            def __dict__(self):
+                return "definitely not a mapping"
+
+        marker = _step_error_marker("k", NotAMapping())
+        _json.dumps(marker)
+        assert marker["result"]["error"]["name"] == "NotAMapping"
+
+        class YieldsNonPairs(Exception):
+            @property
+            def __dict__(self):
+                return _FakeItems()
+
+        marker = _step_error_marker("k", YieldsNonPairs())
+        _json.dumps(marker)
+        assert marker["result"]["error"]["name"] == "YieldsNonPairs"
+
+        class OddKey(Exception):
+            def __init__(self):
+                super().__init__("boom")
+                self.code = 429
+
+        odd = OddKey()
+        odd.__dict__[(1, 2)] = "tuple key"
+        marker = _step_error_marker("k", odd)
+        _json.dumps(marker)  # a surviving tuple key would raise here
+        assert marker["result"]["error"]["extra"] == {"code": 429}
+
+        class ExplodingKey:
+            def __init__(self):
+                self.hashed = 0
+
+            def __hash__(self):
+                self.hashed += 1
+                if self.hashed > 1:
+                    raise RuntimeError("second hash")
+                return 1
+
+        exploding = OddKey()
+        exploding.__dict__[ExplodingKey()] = "x"
+        marker = _step_error_marker("k", exploding)
+        _json.dumps(marker)
+        assert marker["result"]["error"]["extra"] == {"code": 429}
+
         # A float is serializable, so `default=` never sees NaN — it would go out
         # as a bare `NaN` literal, which is not JSON and which the backend
         # rejects, so the step could not be checkpointed at all.

python-client/wmill/wmill/client.py

@@ -2772,33 +2772,36 @@ def _step_error_marker(key: str, exc: BaseException) -> dict:
     # there is — ``resp.raise_for_status()``, whose ``__dict__`` holds a request
     # and a response object — would otherwise fail to serialize and silently
     # drop every such failure onto the slow suspend-and-replay path.
-    # ``getattr``'s default only swallows ``AttributeError``; an exception
-    # overriding ``__getattribute__`` raises whatever it likes from this read.
+    # Everything about the failing exception can fight back, and this runs inside
+    # the ``except`` reporting it, so an escape replaces the user's error and
+    # skips the checkpoint. Only a genuine ``dict`` is walked: an overridden
+    # ``__dict__`` can raise on access, on ``.items()``, or yield non-pairs.
     try:
-        extra = getattr(exc, "__dict__", None)
+        _raw_extra = getattr(exc, "__dict__", None)
     except Exception:
-        extra = None
-    if extra:
-        # ``default`` is where json hands back the objects it cannot represent,
-        # and ``str()`` on a detached ORM row or a proxy over a closed
-        # connection raises in turn. This runs inside the ``except`` that is
-        # reporting the user's failure, so an escape here would replace their
-        # error with an unrelated one and skip the checkpoint entirely.
-        # Narrow: what json raises for something it cannot represent. A broader
-        # catch would hide a mistake in this function as a silently missing
-        # field, which is how it read before.
-        try:
-            # ``parse_constant`` catches the one thing ``default`` cannot: a
-            # float is serializable, so ``NaN``/``Infinity`` pass through as
-            # bare literals that are not JSON and that the backend's extractor
-            # rejects — taking the whole checkpoint down with them. Kept as
-            # their text rather than dropped, so the attribute still says
-            # something.
-            error["extra"] = json.loads(
-                json.dumps(extra, default=_safe_str), parse_constant=lambda c: c
-            )
-        except (TypeError, ValueError, RecursionError):
-            pass
+        _raw_extra = None
+    if type(_raw_extra) is dict and _raw_extra:
+        safe_extra = {}
+        for _k, _v in _raw_extra.items():
+            # Rebuilding the pair hashes the key again, so only the types json
+            # can represent, and exactly those: a subclass may define __hash__.
+            if type(_k) not in (str, int, float, bool, type(None)):
+                continue
+            # Per attribute so one bad value cannot take the rest, and as a pair
+            # so an int/bool/None key arrives as the string a replay reads.
+            # ``parse_constant`` catches what ``default`` cannot: a float is
+            # serializable, so NaN/Infinity would go out as invalid JSON.
+            try:
+                safe_extra.update(
+                    json.loads(
+                        json.dumps({_k: _v}, default=_safe_str),
+                        parse_constant=lambda c: c,
+                    )
+                )
+            except (TypeError, ValueError, RecursionError):
+                pass
+        if safe_extra:
+            error["extra"] = safe_extra
     return {
         "__wmill_error": True,
         "message": _safe_str(exc),