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

[dprada]

On the controller, unserialize_warning_message recreates a warning as cls(*message_args), where message_args is the original instance's args. That assumes the first constructor argument is the message. A Warning subclass is free not to do that — it may name a field of its own and build the message out of it, which is a common shape in libraries that render diagnostics from structured data.

For such a class the rebuilt instance is a different warning: the rendered text goes back into whatever field the first argument names, and __str__ renders around it a second time.

class ResourceWarning(UserWarning):
    def __init__(self, resource):
        self.resource = resource
        super().__init__(f"{resource!r} is not available")

worker:     gpu is not available
controller: "gpu is not available" is not available

A subclass whose parameters all have defaults is quieter and worse: it rebuilds without error and reports the default message, so the text is wrong without looking wrong. Only subclasses that reject the call are handled today, by the existing except TypeError fallback.

Change

Keep the rebuilt instance only when it still says what the original said; otherwise take the existing "could not recreate the original warning instance" path, which reports the original text once. category is unaffected — it is rebuilt from its own fields.

The check is one string comparison against data already carried in the payload, and it does not change behaviour for any warning that rebuilds faithfully.

How it turned up

A test suite reported warning text nested inside itself under -n 12 while the same suite run serially did not. Across the 15 warning classes in that project the reconstruction split three ways: 5 re-rendered, 7 raised TypeError into the generic fallback, and 3 happened to round-trip.

Tests

test_unserialize_warning_msg_that_cannot_round_trip covers it, and fails without the change. Existing testing/test_workermanage.py passes unchanged.

I will add the changelog fragment once this PR has a number, unless you would rather it be filed against a separate issue.

[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.