Do not rebuild a warning whose first argument is not its message

[dprada]

Rewritten 2026-09-05. This description used to argue the first approach, which @RonnyPfannschmidt's review correctly rejected. The code has implemented the review's direction since 2026-08-17; the text had not caught up, so anyone returning to this PR was reading a design that is no longer in it. Apologies for the wasted read.

On the controller, unserialize_warning_message rebuilt a warning as cls(*message_args). serialize_warning_message sends only args, but args alone does not describe a warning: BaseException.__reduce__ carries the instance dictionary as a third element and BaseException.__setstate__ applies it, and we transferred neither.

So a warning keeping anything beside its args arrived without it, and one deriving its message from that state arrived silently wrong. -n0 serializes nothing and is the reference for what the warning actually says:

class CodeWarning(UserWarning):
    def __init__(self, code=None):
        self.code = code
        super().__init__()
    def __str__(self):
        return "code {} tripped".format(self.code or "unknown")

-n0:  code 42 tripped
-n1:  code unknown tripped

What this does

Transfer the state, and rebuild without re-running __init__. _serializable_reduce_state returns what __reduce__ carries, and the controller applies it with cls.__new__(cls) + BaseException.__init__ + __setstate__. args and state are each gated on what execnet.dumps can carry, and the (dict, slots) two-tuple a __slots__ class reduces to is handled.

Drop the class when its state cannot cross. This is the half worth reading closely, because getting it wrong is worse than the original defect. Returning None for both "carries no state" and "carries state we cannot send" means the controller rebuilds the class either way — and an instance rebuilt without the attributes its own __str__ reads raises AttributeError the moment the controller renders it. A warning that keeps nothing reduces to a two-tuple and is rebuilt exactly; one whose state is lost takes the existing generic-Warning path.

A class-provided __reduce__ counts as lost. Its callable is checked before the tuple length, which matters: such a class commonly reduces to a two-tuple carrying its state inside the args, and checking length first reads that as "no state".

@RonnyPfannschmidt — this is the one item where I am unsure I read you correctly. "honour a class-provided __reduce__ when its callable isn't the class itself" could mean rebuild through it, which is what pickle does. I did not, because it runs an arbitrary callable named by the payload, in the controller's receiver thread. Declining is now recorded as a loss rather than an absence, so the class is dropped instead of shipping a broken instance — but if you meant the stronger reading, say so and I will implement it.

The second half: resolving the class must not end the run

Not in the review, and separable — happy to split it out if you would rather.

mod = importlib.import_module(data["message_module"])   # receiver thread, unguarded
cls = getattr(mod, data["message_class_name"])          # receiver thread, unguarded

Both run in the controller's receiver thread, and anything they raise ends the session: the node goes down mid-test and the run fails in the scheduler with KeyError: <WorkerController gwN> — a different subsystem, naming nothing that leads back to the import. The category resolution below had no guard at all and no test, and category is what pytest filters and reports on.

This is #404. That issue is open, carries "I believe we need some kind of fix" from 2019, and its 2022 report names the getattr line and shows it arriving as an INTERNALERROR. test_warning_serialization_tweaked_module was added alongside that report and asserted pytest.raises(ModuleNotFoundError): it characterised the defect rather than fixing it. It now asserts the degradation, and a second test covers the AttributeError shape, which is the one that report actually hit.

The fallback text carries the reason (class not resolved: ModuleNotFoundError: ...). Widening a guard around an import is how a genuinely broken package becomes a warning nobody can explain.

Field evidence. In uibcdf/molsysviewer#76, a cold import molsysmt raising inside the receiver thread discarded roughly 950 tests per occurrence, on about half of all -n 12 runs. It took three attempts to attribute, because the error names the scheduler and the cause is an import two steps away. A warning failing to deserialize should not be able to do that.

Tests

Assert the resulting type and the exact text, per the review. Round-trip fidelity for the field-first and state-only shapes; fallback to a plain Warning for the custom-__reduce__ and untransferable-state shapes; exact rebuild for a warning with no state, so the fallback does not widen to warnings with nothing to lose; and one test each for an unimportable module, a class missing from its module, and an unresolvable category.

Each guard is mutation-verified: removing the message-class guard fails 3 tests, the category guard 2, the lost-state check 2.

Full suite green. ruff check, ruff format and mypy clean on the changed files.

[RonnyPfannschmidt]

Ill give a more detailed reply when I get back to the computer

But the premise here is wrong as is

[RonnyPfannschmidt]

up front: the analysis below and the cell were put together by claude at my direction, and the output block is what it printed running that cell in its own sandbox on cpython 3.12.3. i have not rerun it locally, so treat the numbers as something to check rather than something established. the reasoning and the conclusions are mine. whats put here is iteration 7

---

this misunderstands how warnings and exceptions get reconstructed.

cls(*args) is not something xdist made up. BaseException.__reduce__ returns (cls, self.args), plus the instance dict as a third element when non-empty, and pickle calls the class with those args. your first case therefore doubles under plain pickle and copy too, no xdist involved. and warnings.warn(msg, category) builds the instance as category(msg), so neither of your classes can be used as a category at all - one re-renders the message, the other swallows it.

the check in this pr is also at the wrong end. what we get wrong is the state: __reduce__ carries a third element, BaseException implements __setstate__ (setattr per key, the branch pickle takes for exceptions), and we transfer neither. rebuilding without re-running __init__ gets both of your cases back exact, class included.

paste this in a cell:

import copy
import pickle
import warnings


class BrokenResourceWarning(UserWarning):
    """field first, message rendered in __init__"""
    def __init__(self, resource):
        self.resource = resource
        super().__init__(f"{resource!r} is not available")


class BrokenCodeWarning(UserWarning):
    """no args at all, text derived from state"""
    def __init__(self, code=None):
        self.code = code
        super().__init__()

    def __str__(self):
        return f"code {self.code or 'unknown'} tripped"


class FixedResourceWarning(UserWarning):
    """message first, structured data kept alongside"""
    def __init__(self, message, resource=None):
        super().__init__(message)
        self.resource = resource

    @classmethod
    def for_resource(cls, resource):
        return cls(f"{resource!r} is not available", resource=resource)


def xdist_today(w):
    """class + args, what unserialize_warning_message does"""
    return type(w)(*w.args)


def via_reduce(w):
    """args plus reduce state, rebuilt without re-running __init__"""
    red = w.__reduce__()
    cls, args = red[0], red[1]
    state = red[2] if len(red) > 2 else None
    new = cls.__new__(cls)
    BaseException.__init__(new, *args)
    if state is not None:
        new.__setstate__(state)
    return new


def report(w):
    print(f"{type(w).__name__}")
    print(f"    {'original':12} {str(w)!r}")
    for name, fn in [
        ("pickle", lambda x: pickle.loads(pickle.dumps(x))),
        ("copy", copy.copy),
        ("xdist today", xdist_today),
        ("via reduce", via_reduce),
    ]:
        try:
            out = repr(str(fn(w)))
        except Exception as exc:
            out = f"{type(exc).__name__}: {exc}"
        print(f"    {name:12} {out}")
    print()


report(BrokenResourceWarning("gpu"))
report(BrokenCodeWarning(42))
report(FixedResourceWarning.for_resource("gpu"))

for cls in (BrokenResourceWarning, BrokenCodeWarning, FixedResourceWarning):
    with warnings.catch_warnings(record=True) as rec:
        warnings.simplefilter("always")
        try:
            warnings.warn("boom", cls)
            got = repr(str(rec[0].message))
        except Exception as exc:
            got = f"{type(exc).__name__}: {exc}"
    print(f"warn('boom', {cls.__name__}) -> {got}")

reported output, cpython 3.12.3, sandbox run as noted above:

BrokenResourceWarning
    original     "'gpu' is not available"
    pickle       '"\'gpu\' is not available" is not available'
    copy         '"\'gpu\' is not available" is not available'
    xdist today  '"\'gpu\' is not available" is not available'
    via reduce   "'gpu' is not available"

BrokenCodeWarning
    original     'code 42 tripped'
    pickle       'code 42 tripped'
    copy         'code 42 tripped'
    xdist today  'code unknown tripped'
    via reduce   'code 42 tripped'

FixedResourceWarning
    original     "'gpu' is not available"
    pickle       "'gpu' is not available"
    copy         "'gpu' is not available"
    xdist today  "'gpu' is not available"
    via reduce   "'gpu' is not available"

warn('boom', BrokenResourceWarning) -> "'boom' is not available"
warn('boom', BrokenCodeWarning) -> 'code boom tripped'
warn('boom', FixedResourceWarning) -> 'boom'

FixedResourceWarning is the shape that holds up everywhere: message first, structured datum as a keyword, a classmethod for the convenient call. it survives pickle, copy, the category api and our current serializer unchanged, and needs no xdist patch at all.

what i'd review on our side:

• serialize args and the reduce state, each gated on what execnet.dumps can carry
• rebuild via __new__ + __setstate__ instead of cls(*args), so a re-rendering __init__ never runs
• honour a class-provided __reduce__ when its callable isn't the class itself
• slots state arrives as a (dict, slots) 2-tuple - handle it or fall back
• fall back to a plain Warning only when the state won't transfer

comparing str() and discarding the result detects that our transfer is lossy without making it less lossy, and pays for it by dropping the message class and prefixing the text with module.Class: .

tests should assert the resulting type and the exact text, not a substring and an occurrence count.

[dprada]

Still reproducible, now differently explained: on master, -n0 and -n1 disagree about what the same warning says. The first version of this PR misread why, and its check detected the loss instead of repairing it — @RonnyPfannschmidt's review is correct on every point.

Thank you for it. The write-up did more than reject an approach: it explained where the mechanism actually lives, which shape of warning class holds up everywhere, and what you would want to see on your side. I ran your cell on CPython 3.13.14 and it reproduces exactly, BrokenCodeWarning included — the case that shows this is not only about badly shaped classes.

I have kept this open rather than closing it because the behaviour it was opened for is still observable, and I believe the five items you listed close it properly. If you conclude otherwise, closing this is entirely reasonable and the analysis you wrote was worth more than the patch either way.

The defect

serialize_warning_message sends only args, and unserialize_warning_message rebuilds as cls(*message_args). BaseException.__reduce__ carries the instance dictionary as a third element and BaseException.__setstate__ applies it; we transfer neither. A warning keeping anything beside its args therefore arrives without it, and one deriving its message from that state arrives silently wrong.

-n0 serializes nothing, so it is the reference for what the warning actually says:

class CodeWarning(UserWarning):
    def __init__(self, code=None):
        self.code = code
        super().__init__()
    def __str__(self):
        return "code {} tripped".format(self.code or "unknown")

def test_func():
    warnings.warn(CodeWarning(42))

-n0   code 42 tripped
-n1   code unknown tripped

That class round-trips correctly under pickle and under copy. It only breaks crossing a worker boundary, and the result reads like a legitimate message rather than an error.

The change

Following your five items:

• Serialize the reduce state alongside args, each gated on what execnet.dumps can carry, so a payload that cannot cross is reported absent rather than half-sent.
• Rebuild with cls.__new__ + BaseException.__init__ + __setstate__, so a re-rendering __init__ never runs.
• Handle the (dict, slots) two-tuple that __slots__ classes reduce to.
• Leave a class-provided __reduce__ alone when its callable is not the class itself, falling back to the generic Warning. This is my reading of your fourth point and may not be what you meant: execnet cannot carry that callable, so I could not find a way to honour such a protocol rather than override it. If you had something else in mind I would rather follow it than guess.
• Fall back to the generic Warning only when the state will not transfer.

Tests

test_state_beyond_args_survives is the acceptance test, parametrized over -n0/-n1; it fails on master at -n1 while passing at -n0. Two unit tests cover the shapes from your review — field-first and state-only — asserting the resulting type, the exact text and the restored attribute, rather than a substring and an occurrence count. All three fail without the change.

[dprada]

@RonnyPfannschmidt — ready for another look, and I owe you an apology for part of the delay.

The code has implemented your direction since 2026-08-17, but I never rewrote the PR description, so it still argued the first approach — the one you said was at the wrong end. Anyone returning to this PR was reading a design that is no longer in it. That is fixed now; the body above describes what is actually here.

Since your review

Your five items, and where each landed:

item	state
serialize args and the reduce state, each gated on execnet.dumps	done
rebuild via __new__ + __setstate__	done
slots state as a (dict, slots) two-tuple	done
fall back to a plain Warning only when the state won't transfer	was incomplete — now done
honour a class-provided __reduce__ when its callable isn't the class	was missed — see below

Two of them were not actually finished when I said the rework was complete, and together they left something worse than the original defect:

CustomReduce      worker: "code 7 tripped"  ->  controller: AttributeError: no attribute "code"
UnDumpableState   worker: "code 9 tripped"  ->  controller: AttributeError: no attribute "code"

_serializable_reduce_state returned None both for a warning that carries no state and for one whose state cannot cross, and the controller rebuilt the class either way. Rebuilding without the attributes a class's own __str__ reads means the controller raises when it renders the warning — a crash introduced while fixing a rendering defect. It returns (state, lost) now, and a lost state takes the fallback.

The __reduce__ item was missed for a reason worth naming: the tuple-length check ran before the callable check, and such a class commonly reduces to a two-tuple carrying its state inside the args. That read as "no state, nothing lost". The callable is checked first now.

One question, on that item

I am not sure I read "honour" correctly. It could mean rebuild through it, which is what pickle does. I did not, because it calls an arbitrary callable named by the payload, in the controller's receiver thread — the same place the second half of this PR is about protecting.

What it does instead is treat it as a loss rather than an absence, so the class is dropped and the text reported once, instead of an instance arriving broken. If you meant the stronger reading, say so and I will implement it.

The second half, and #404

Separable — happy to split it into its own PR if you would rather review them apart.

Resolving the warning's class is importlib.import_module plus a getattr, both outside every guard, both in the receiver thread. This is #404: open since 2019, with your own "I believe we need some kind of fix" on it, and its 2022 report names the getattr line and shows it arriving as an INTERNALERROR. test_warning_serialization_tweaked_module was added alongside that report and asserted pytest.raises(ModuleNotFoundError) — it characterised the defect rather than fixing it. It now asserts the degradation, and a new test covers the AttributeError shape, which is the one that report actually hit.

The category resolution below it had no guard at all and no test, and category is what pytest filters and reports on.

Verification

Rebased onto the current branch head after the master merge and re-run there: 235 passed, 2 skipped, 10 xfailed. ruff check, ruff format and mypy clean on the changed files.

Each guard is mutation-verified — removing the message-class guard fails 3 tests, the category guard 2, the lost-state check 2 — because a guard nobody has watched fail is not a guard.