rnag / rnag/dataclass-wizard

CatchAll fields break on reuse across structural contexts (self-referential dataclasses are a special case) — TypeError: issubclass() arg 1 must be a class

Open
#252 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
250
Forks
36
PR merge metrics
No merged PRs in 30d

Description

Update: The root cause has been identified and a fix is up in #253. It turns
out the self-referential case originally reported here is actually a special
case of a broader bug: any dataclass with a CatchAll field can only be
successfully loaded/dumped in one distinct structural context per process,
not just self-referential ones. Leaving the original repro below for
reference, but updating the title/description to reflect the full scope.

The bug (general form)

A dataclass with a CatchAll field (used to capture unrecognized JSON keys
into a dict) can only be successfully loaded/dumped by dataclass_wizard 1.0.0
in one distinct structural context per process. "Context" means: used as
the direct top-level target of .from_dict()/.to_dict(), vs. used as a
field of enclosing class X, vs. used as a field of enclosing class Y — each
is a different context. Whichever context is hit first works. Any second,
differently-shaped use of that same class then fails, permanently, for the
rest of the process, with:

TypeError: issubclass() arg 1 must be a class

raised from dataclass_wizard/_loaders.py::load_dispatcher_for_annotation
at if issubclass(origin, t): (dump side: _dumpers.py, analogous
function/line).

This is not recursion-specific — plain, non-self-referential classes hit
it too: e.g. calling .from_dict() directly on some class Leaf (with a
CatchAll field), then later parsing a different class that has Leaf as a
nested field, breaks the second call.

Self-referential ("recursive") classes — the original report below — are a
special case that fails on their very first use rather than their second:
resolving a self-referential class as a top-level .from_dict() target
requires generating codegen for that class twice within a single call
(once for the top-level entry, once for the self-referential field via the
forward reference) — two conflicting structural contexts compressed into one
call, so it fails immediately.

Minimal repro (general case, no recursion involved)

from dataclasses import dataclass, field
from dataclass_wizard import JSONWizard
from dataclass_wizard.models import CatchAll

@dataclass
class Leaf(JSONWizard):
    name: str
    extra: CatchAll = field(default_factory=dict)

@dataclass
class Wrapper(JSONWizard):
    value: Leaf

Leaf.from_dict({"name": "a", "unrecognized": "x"})                # OK
Wrapper.from_dict({"value": {"name": "b", "unrecognized": "y"}})  # TypeError: issubclass() arg 1 must be a class

Root cause

In dataclass_wizard/_loaders.py, inside load_func_for_dataclass:

field_to_aliases = resolve_dataclass_field_to_alias_for_load(cls)
...
catch_all_field: str | None = field_to_aliases.pop(CATCH_ALL, None)
has_catch_all = catch_all_field is not None

resolve_dataclass_field_to_alias_for_load (in _class_helper.py) returns a
direct reference to a dict cached once per class in a module-level
WeakKeyDictionary (DATACLASS_FIELD_TO_ALIAS_FOR_LOAD), not a copy. The
.pop(CATCH_ALL, None) call destructively mutates that shared cached dict.

The first time load_func_for_dataclass(cls) runs for a given class,
has_catch_all is correctly True, and the CatchAll field is
stripped/special-cased as intended — but the shared per-class alias cache is
now permanently missing its CATCH_ALL entry. If load_func_for_dataclass
runs again for the same class in a different context (nested-field
codegen doesn't reuse the cached compiled function — it re-invokes
load_func_for_dataclass directly), has_catch_all is now incorrectly
False. The CatchAll-typed field is no longer stripped, so its declared
type annotation — CatchAll = NewType('CatchAll', Mapping), a
typing.NewType, which is callable but not a class — gets resolved
through the normal dispatch path instead, and issubclass(origin, t) raises
exactly the observed error.

The exact same pattern exists on the dump side (_dumpers.py,
resolve_dataclass_field_to_alias_for_dump).

Fix

See #253: swap .pop(CATCH_ALL, None) for .get(CATCH_ALL) in both
_loaders.py and _dumpers.py. CATCH_ALL is a sentinel string that never
collides with a real field name, so nothing relies on the entry being
removed from the dict — only on reading whether it's present. This makes the
cache read side-effect-free, fixing reuse across any number of structural
contexts, including the self-referential case originally reported here.


Original report (self-referential case only)

A self-referential ("recursive") dataclass -- e.g. a tree-node type with a
children: list[Self] field -- that ALSO has a CatchAll field raises

TypeError: issubclass() arg 1 must be a class

when .from_dict() / .from_list() / .to_dict() is called directly on
it, i.e. when it is the top-level type being loaded/dumped. The traceback
bottoms out in dataclass_wizard's codegen (1.0.0):

dataclass_wizard/_loaders.py:1014, in load_dispatcher_for_annotation
    if issubclass(origin, t):
TypeError: issubclass() arg 1 must be a class

The same class works FINE if:

  • it has no CatchAll field (recursion alone is not enough to trigger it), or
  • it has a CatchAll field but is never used as the top-level entry point --
    e.g. it's reached only as a nested field of some other, non-recursive,
    enclosing dataclass.

Once a dataclass has failed in this way, it is poisoned for the remainder of the process lifetime.

Best illustrated with a repro case, tested on Python 3.14:

#!/usr/bin/env python3
"""
Minimal, standalone demonstration of a dataclass_wizard 1.0.0 bug.

Depends on nothing but `dataclass_wizard` itself (and its implicit dependencies)
plus the Python standard library. No import from this project.

    pip install dataclass-wizard==1.0.0

Note: every self-referential class below is defined at MODULE level, not
nested inside a function. dataclass_wizard resolves a `list[Self]`-style
forward reference (the string "Node", say) by evaluating it against the
class's `__module__` globals -- if the class were defined inside a function
instead, its own name wouldn't exist in those globals yet, and you'd get an
unrelated `NameError: name 'Node' is not defined` instead of the bug this
script is about. This is just a quirk of how the demo has to be written, not
part of the bug itself.

THE BUG
=======
A self-referential ("recursive") dataclass -- e.g. a tree-node type with a
`children: list[Self]` field -- that ALSO has a `CatchAll` field (used to
capture unrecognized JSON keys) raises

    TypeError: issubclass() arg 1 must be a class

when `.from_dict()` / `.from_list()` / `.to_dict()` is called *directly* on
it, i.e. when it is the *top-level* type being loaded/dumped. The traceback
bottoms out in dataclass_wizard's codegen (1.0.0):

    dataclass_wizard/_loaders.py:1014, in load_dispatcher_for_annotation
        if issubclass(origin, t):
    TypeError: issubclass() arg 1 must be a class

The same class works FINE if:
  * it has no `CatchAll` field (recursion alone is not enough to trigger it), or
  * it has a `CatchAll` field but is never used as the top-level entry point --
    e.g. it's reached only as a *nested* field of some other, non-recursive,
    enclosing dataclass -- PROVIDED it is never *first* used as a direct
    top-level target (see the "POISONING" section below -- this is the
    trickiest part of this bug's behavior).

Sections 1-2 demonstrate that self-reference and CatchAll are each harmless
alone. Section 3 demonstrates the buggy combination failing directly. Section
4 demonstrates the same combination succeeding when reached only as a nested
field (using a *fresh, not-yet-used* class -- important, see below). Section
5 demonstrates the wrapper-class workaround, again applied proactively to a
fresh class. Section 6 demonstrates the "poisoning" caveat: once a class has
been used (and failed) as a direct top-level target, it is permanently broken
for the rest of the process -- even the nested and wrapper approaches, which
otherwise work fine, will ALSO start failing if tried afterwards on that same
already-poisoned class. This was surprising enough that it's worth calling
out explicitly rather than leaving implicit in the ordering below.

WHY (best understanding)
=========================
dataclass_wizard generates a specialized loader/dumper function per dataclass
the first time it's needed, walking the type's fields and recursively
resolving each field's annotation to a per-field (de)serialization strategy,
then CACHES the generated function on the class. For a field whose declared
type is the class currently being defined (a forward reference to itself),
that type must be resolved lazily.

When the class is reached as a *nested* field of some other class (and this
is the first time its codegen runs at all), the codegen for the *inner* class
resolves it as an already-fully-built type object -- no self-reference is "in
flight" at that point. But when the recursive class is itself the *direct*
top-level target, and it *also* has a `CatchAll` field, something about how
`CatchAll` fields are special-cased in the codegen's dispatch logic (they need
to collect "leftover" keys not claimed by any other field) apparently
interacts with the lazily-resolved self-reference such that a non-class
object ends up passed as the first argument to `issubclass()`.

Critically, this appears to happen during the ONE-TIME codegen step, which
is then cached -- so once codegen has run and failed once for a given class
(regardless of how it was triggered), later attempts reuse/re-trigger the
same broken state rather than getting a clean second chance. That's the
"poisoning" effect in section 6: it isn't specific to calling the class
directly again -- any subsequent use of that same class object, including
nested or wrapped, is affected too.
(This is an empirical characterization, not a reading of dataclass_wizard's
internals -- treat "why" as a hypothesis, not a verified root cause.)

THE WORKAROUND
===============
Wrap the recursive type in a throwaway, non-recursive dataclass with a single
field of the recursive type (or `list[recursive_type]`), and call
`.from_dict()`/`.to_dict()` on the *wrapper* instead of on the recursive class
directly, then unwrap. This works because:
  * by the time the wrapper's codegen runs, the recursive class already exists
    as a fully-resolved real type object (not a lazily-resolved forward
    reference) -- exactly the same situation as the "nested field of an
    enclosing class" case that already works fine, and
  * the wrapper class itself is not self-referential, so it's safe to use as
    a top-level target.

The workaround MUST be applied proactively, before the recursive class is
ever used as a direct top-level target -- per section 6, once a class has
failed that way even once, it's too late for the wrapper to help. In
production, this means the recursive class should simply never be called
directly at all; always route every load/dump through the wrapper (or,
better, make that automatic -- see below).

See `codingame_client/common/dataclass_wizard_x.py` in this project for a
production version that applies this automatically (detecting self-referential
classes up front and always routing them through a cached, dynamically-built
wrapper, so the buggy direct path is never hit even once), so callers never
have to think about it or worry about poisoning.
"""

from __future__ import annotations

from dataclasses import dataclass, field, make_dataclass

from dataclass_wizard import JSONWizard
from dataclass_wizard.models import CatchAll

SAMPLE_DATA = {
    "name": "root",
    "unrecognized_field": "should end up in extra_data",
    "children": [
        {"name": "child", "children": []},
    ],
}


# --- Section 1: self-reference alone, no CatchAll -- works fine ---

@dataclass
class NodeNoCatchAll(JSONWizard):
    name: str
    children: list[NodeNoCatchAll] = field(default_factory=list)


# --- Section 2: CatchAll alone, no self-reference -- works fine ---

@dataclass
class LeafWithCatchAll(JSONWizard):
    name: str
    extra_data: CatchAll = field(default_factory=dict)


# --- Section 3: self-reference AND CatchAll, called directly -- FAILS.
#     (This class is deliberately never reused in later sections -- calling
#     it directly here permanently poisons it; see section 6.) ---

@dataclass
class BuggyNodeDirect(JSONWizard):
    name: str
    extra_data: CatchAll = field(default_factory=dict)
    children: list[BuggyNodeDirect] = field(default_factory=list)


# --- Section 4: same buggy shape, but a FRESH class only ever reached as a
#     nested field -- works fine, as long as it's fresh. ---

@dataclass
class BuggyNodeNested(JSONWizard):
    name: str
    extra_data: CatchAll = field(default_factory=dict)
    children: list[BuggyNodeNested] = field(default_factory=list)


@dataclass
class EnclosingWrapper(JSONWizard):
    value: BuggyNodeNested


# --- Section 5: same buggy shape, another FRESH class, using the wrapper
#     workaround applied proactively (never called directly first). ---

@dataclass
class BuggyNodeWorkaround(JSONWizard):
    name: str
    extra_data: CatchAll = field(default_factory=dict)
    children: list[BuggyNodeWorkaround] = field(default_factory=list)


def run_self_reference_alone() -> None:
    node = NodeNoCatchAll.from_dict({"name": "root", "children": [{"name": "child", "children": []}]})
    print(f"    OK: {node}")


def run_catch_all_alone() -> None:
    leaf = LeafWithCatchAll.from_dict({"name": "root", "unrecognized_field": "captured here"})
    print(f"    OK: {leaf}")


def run_buggy_case() -> None:
    try:
        node = BuggyNodeDirect.from_dict(SAMPLE_DATA)
        print(f"    UNEXPECTED SUCCESS (bug may be fixed in your dataclass_wizard version): {node}")
    except TypeError as e:
        print(f"    FAILED as expected: {type(e).__name__}: {e}")


def run_nested_case() -> None:
    wrapper = EnclosingWrapper.from_dict({"value": SAMPLE_DATA})
    print(f"    OK (nested, class used for the first time here): {wrapper.value}")


def run_workaround() -> None:
    """The workaround: build a throwaway single-field wrapper dataclass around
       the recursive class, and call from_dict/to_dict on the WRAPPER instead
       of on BuggyNodeWorkaround directly -- and do this the FIRST time
       BuggyNodeWorkaround is used at all, before anything can poison it."""
    wrapper_cls = make_dataclass("_Wrapper", [("value", BuggyNodeWorkaround)], bases=(JSONWizard,))

    wrapper = wrapper_cls.from_dict({"value": SAMPLE_DATA})  # type: ignore[attr-defined]
    node = wrapper.value  # type: ignore[attr-defined]
    print(f"    OK (via wrapper): {node}")

    dumped = wrapper.to_dict()  # type: ignore[attr-defined]
    print(f"    OK (to_dict via wrapper): {dumped['value']}")


def run_poisoning_demo() -> None:
    """BuggyNodeDirect already failed once, directly, back in section 3. Show
       that it is now permanently broken -- even approaches that work fine on
       a FRESH class of the identical shape (nesting, wrapping) now also
       fail on this specific, already-poisoned class object."""
    print("    4b. Retrying the exact same direct call again -- still fails (not a fluke/retry-fixable):")
    try:
        BuggyNodeDirect.from_dict(SAMPLE_DATA)
        print("        UNEXPECTED SUCCESS")
    except TypeError as e:
        print(f"        FAILED again: {e}")

    print("    4c. Trying to NEST the already-poisoned class -- also fails now:")
    wrapper_cls = make_dataclass("_PoisonedNestWrapper", [("value", BuggyNodeDirect)], bases=(JSONWizard,))
    try:
        wrapper_cls.from_dict({"value": SAMPLE_DATA})  # type: ignore[attr-defined]
        print("        UNEXPECTED SUCCESS")
    except TypeError as e:
        print(f"        FAILED: {e}")
    print("    => Conclusion: the wrapper workaround must be applied BEFORE the recursive")
    print("       class is ever used as a direct top-level target, not as a fallback after a")
    print("       failed attempt. In production, never call the recursive class directly at all.")


if __name__ == "__main__":
    print("1. Self-reference alone (no CatchAll) -- expect OK:")
    run_self_reference_alone()

    print("\n2. CatchAll alone (no self-reference) -- expect OK:")
    run_catch_all_alone()

    print("\n3. Self-reference + CatchAll, called directly as top-level type -- expect FAILURE:")
    run_buggy_case()

    print("\n4. Same buggy shape, a FRESH class reached as a NESTED field -- expect OK:")
    run_nested_case()

    print("\n5. Same buggy shape, another FRESH class, using the wrapper-class workaround -- expect OK:")
    run_workaround()

    print("\n6. Caveat: the class from section 3 is now permanently poisoned -- expect FAILURE both ways:")
    run_poisoning_demo()

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Review dataclass_wizard/_loaders.py and _dumpers.py, especially load_func_for_dataclass and the analogous dump path, then inspect #253. Done is confirmed when CatchAll classes can be reused across top-level and nested structural contexts without TypeError, including self-referential cases.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
15/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.