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

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

backend/windmill-common/src/wac_failure_corpus.json

@@ -0,0 +1,80 @@
+{
+  "_readme": [
+    "Cases the python and the typescript SDK must both satisfy when they serialize",
+    "a failed step() body into a `__wmill_error` marker.",
+    "",
+    "The two drifted apart twice while #10368 was in review: `name` was taken from",
+    "the constructor in one client and from `e.name` in the other, and `stack` was",
+    "written in two different formats. Each was caught by a reviewer reading the",
+    "diff, not by a test, because each suite only ever checked its own language.",
+    "A shared corpus turns the next divergence into a failure in both suites.",
+    "",
+    "It sits next to `wac_failure_record`, the function that decides the record's",
+    "final shape, because that is the contract these markers are the raw material",
+    "for. The SDK test suites read it by relative path.",
+    "",
+    "`thrown` describes a value each language constructs natively:",
+    "  name           the name the record must report",
+    "  message        the message the record must report",
+    "  props          own properties/attributes that must reach `extra`",
+    "  circular_prop  an own property holding a cycle, which must be dropped",
+    "                 without taking the rest of `extra` with it",
+    "`expect.stack` is only \"present\" or \"absent\": its text is language-specific",
+    "(each client matches its own executor's format) and is asserted there.",
+    "",
+    "Known divergence, deliberately not covered here: a non-finite float reaches",
+    "`extra` as the string \"NaN\" in python and as null in typescript, because",
+    "JSON.stringify has no hook for it. The executors differ the same way."
+  ],
+  "cases": [
+    {
+      "case": "a named error keeps its name and message",
+      "thrown": { "name": "HttpError", "message": "429 too many requests" },
+      "expect": {
+        "name": "HttpError",
+        "message": "429 too many requests",
+        "stack": "present",
+        "absent": ["extra", "extra_omitted"]
+      }
+    },
+    {
+      "case": "custom properties reach extra",
+      "thrown": {
+        "name": "HttpError",
+        "message": "429",
+        "props": { "code": 429, "retry_after": 5, "endpoint": "/v1/jobs" }
+      },
+      "expect": {
+        "name": "HttpError",
+        "message": "429",
+        "stack": "present",
+        "extra": { "code": 429, "retry_after": 5, "endpoint": "/v1/jobs" }
+      }
+    },
+    {
+      "case": "a property that cannot be serialized is dropped on its own",
+      "thrown": {
+        "name": "RequestError",
+        "message": "socket hang up",
+        "props": { "code": "ECONNRESET" },
+        "circular_prop": "request"
+      },
+      "expect": {
+        "name": "RequestError",
+        "message": "socket hang up",
+        "stack": "present",
+        "extra": { "code": "ECONNRESET" }
+      }
+    },
+    {
+      "case": "an error carrying nothing of its own has no extra",
+      "thrown": { "name": "ValueError", "message": "nope" },
+      "expect": {
+        "name": "ValueError",
+        "message": "nope",
+        "stack": "present",
+        "absent": ["extra"]
+      }
+    }
+  ]
+}

python-client/wmill/tests/test_workflow.py

@@ -2,6 +2,7 @@
 
 import asyncio
 import json
+import pathlib
 import pytest
 
 from datetime import datetime, timezone
@@ -968,6 +969,64 @@ class TestErrorPropagation:
         assert "step failed" in r["result"]["caught"]
 
 
+class TestFailureRecordCorpus:
+    """The cases both SDKs must agree on, read from one shared file.
+
+    `name` and `stack` drifted between the two clients twice while the record
+    was being unified, and each time only a reviewer noticed: a suite that only
+    knows its own language cannot see a divergence. The corpus is the same file
+    the typescript suite reads.
+    """
+
+    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``.
 

python-client/wmill/wmill/client.py

@@ -2784,21 +2784,27 @@ def _step_error_marker(key: str, exc: BaseException) -> dict:
         # 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
+        #
+        # Per attribute, so one that cannot be represented does not take the
+        # rest with it — an ``AxiosError``-shaped exception carrying both a
+        # ``code`` and a cyclic ``request`` keeps the ``code``. The typescript
+        # client admits properties one at a time for the same reason.
+        safe_extra = {}
+        for _k, _v in extra.items():
+            # Narrow: what json raises for something it cannot represent. A
+            # broader catch would hide a mistake here as a silently missing
+            # field, which is how it read before. ``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.
+            try:
+                safe_extra[_k] = json.loads(
+                    json.dumps(_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),

typescript-client/tests/workflow.test.ts

@@ -10,6 +10,38 @@ import { expect, test, describe } from "bun:test";
 // backend's shape before, so a copy of them here would guard nothing.
 import { isSuspendSignal, stepErrorMarker, taskErrorFromMarker } from "../wacError";
 
+// The cases both SDKs must agree on, read from the same file the python suite
+// reads. `name` and `stack` drifted between the two clients twice while the
+// record was being unified, and a suite that only knows its own language cannot
+// see that.
+import corpus from "../../backend/windmill-common/src/wac_failure_corpus.json";
+
+describe("shared failure-record corpus", () => {
+  const construct = (spec: any): any => {
+    const e: any = Object.assign(new Error(spec.message), { name: spec.name });
+    for (const [k, v] of Object.entries(spec.props ?? {})) e[k] = v;
+    if (spec.circular_prop) {
+      const cyclic: any = {};
+      cyclic.self = cyclic;
+      e[spec.circular_prop] = cyclic;
+    }
+    return e;
+  };
+
+  for (const c of (corpus as any).cases) {
+    test(c.case, () => {
+      const error = stepErrorMarker("k", construct(c.thrown)).result.error;
+      expect(error.name).toBe(c.expect.name);
+      expect(error.message).toBe(c.expect.message);
+      expect("stack" in error).toBe(c.expect.stack === "present");
+      if (c.expect.extra) expect(error.extra).toEqual(c.expect.extra);
+      for (const absent of c.expect.absent ?? []) expect(absent in error).toBe(false);
+      // whatever it kept has to survive the trip to the checkpoint
+      expect(() => JSON.stringify(error)).not.toThrow();
+    });
+  }
+});
+
 // --- Inline SDK (mirrors client.ts implementation) ---
 
 class StepSuspend extends Error {