Skip to content

[Bug] Dataclass fields with init=False break payload round tripping #1934

Closed

Description

@tomasfarias

What are you really trying to do?

I am trying to create a Schedule with a ScheduleActionStartWorkflow action. The Workflow associated to the action takes a dataclasses.dataclass as input. This input dataclass has a field with init=False, as an example:

@dataclasses.dataclass
class MyModel:
    name: str = dataclasses.field(default="my-model", init=False)

The broader context is that there are actually many of these input classes, some nested in others, and we are giving them each a unique name. Ideally this would be a class attribute with ClassVar, but ClassVars are essentially ignored by the dataclass mechanism, for example they are not yielded by the fields() iterator, and we need the attributes for serialization/deserialization purposes.

Describe the bug

Continuing with the example above: The value_to_type function of the default payload converter does not handle init=False fields, and happily passes along "my-model" when decoding MyModel. This happens in this block that handles dataclasses:

if dataclasses.is_dataclass(hint):
.

This causes a TypeError to be raised as MyModel is called with an argument it does not take in its __init__ (see section below for a minimal example to reproduce this).

When constructing fields for a dataclass, I think value_to_type should skip init=False fields (let default and __post_init__ handle them), and (optionally) assign the value after construction (although this would require checking that the classes are not frozen=True, or deliberately bypassing frozen=True by using __setattr__, which I would say goes too far).

One thing to keep in mind is that init=False fields should still be passed to value_to_type to ensure they are used to validate the type to use in the event a union is used, just not passed to the constructor on the final line of the dataclasses block.

Minimal Reproduction

import dataclasses 
from temporalio.converter import JSONPlainPayloadConverter

@dataclasses.dataclass(frozen=True)
class MyModel:
    name: str = dataclasses.field(default="my-model", init=False)

converter = JSONPlainPayloadConverter()
converter.from_payload(converter.to_payload(Model()), Model)

Raises:

Traceback (most recent call last):
  File "<stdin>", line 9, in <module>
  File "/.../python3.14/site-packages/temporalio/converter/_payload_converter.py", line 800, in from_payload
    obj = value_to_type(type_hint, obj, self._custom_type_converters)
  File "/.../python3.14/site-packages/temporalio/converter/_payload_converter.py", line 1012, in value_to_type
    return hint(**field_values)
TypeError: Model.__init__() got an unexpected keyword argument 'name'

Environment/Versions

  • OS and processor: Linux.
  • Temporal Version: Locally I am running 1.33.0. Although I think it is also present in 1.34.0.
  • Are you using Docker or Kubernetes or building Temporal from source? Source.

Additional context

For disclosure: An AI tool running a normal review process spotted this issue. It was not specifically instructed to find issues with the temporal SDK internals as the code changes I am working on are unrelated to SDK internals (just creating a schedule like I mentioned above).

After reading through the review results, I decided this should be reported as a bug, I constructed the minimal example after narrowing down the issue, and I wrote the report you are reading here myself (all without any AI assistance, so I own any errors).

I tried to look for similar uses using queries like:

  • "init=False"
  • "value_to_type" dataclass

But didn't get any hits, if this is a duplicate I apologize.

In the meantime, I am able to work around the issue with a custom decoder, but I do think that the handling of init=False dataclass fields is incorrect: at the very least these should be skipped as I mentioned above.

Thanks for taking a look at the report! I would be happy to provide a patch for this if you agree with my proposed solution steps or, if you don't, happy to also work on whatever proposed solution you have in mind (as long as it helps with my use case!).

Activity

  1. AmitKulkarni23 commented on Oct 6, 2026

    @AmitKulkarni23
    Contributor

    First time contributor here. I would like to take this up and work on this today

  2. AmitKulkarni23 commented on Oct 7, 2026

    @AmitKulkarni23
    Contributor

    Created a PR to address this - #1942

  3. tconley1428 commented on Oct 7, 2026

    @tconley1428
    Contributor

    Overall seems like a reasonable thing to fix. Since it fails currently, rather than serializing/deserializing incorrectly, it doesn't seem like it would have backwards compatibility concerns as sometimes happens with incorrect decoding here.

    and (optionally) assign the value after construction (although this would require checking that the classes are not frozen=True, or deliberately bypassing frozen=True by using setattr, which I would say goes too far).

    I'm conflicted on that. It seems like both failure modes would be somewhat unusual, but I think at the end of the day ensuring the resulting object is the same as the one which was serialized is probably paramount, and we should set the attribute even on a frozen dataclass. We can't know for sure the user didn't do that to their dataclass as well.

    Two other questions though:

    1. We also got Restore init=False dataclass fields when converting from JSON #1920, was this affiliated with you (we just closed because there was no CLA signature), or a crazy coincidence?
    2. Often, we would recommend using the pydantic_data_converter instead, as it is more robust in a number of ways than this hand rolled default. Do you have a reason that isn't an option? (A no here won't mean we won't accept a fix, just want context as this is far from the only limitation with this data converter)
  4. tconley1428 commented on Oct 7, 2026

    @tconley1428
    Contributor

    and we should set the attribute even on a frozen dataclass. We can't know for sure the user didn't do that to their dataclass as well.

    Counterpoint to my own argument, some quick AI research suggests pydantic does not do this, modifications to a frozen dataclass would be dropped, so that may be a good model to follow for consistency.

  5. tomasfarias commented on Oct 8, 2026

    @tomasfarias
    Author

    Hey @tconley1428, thanks for taking a look. To answer your questions:

    1. We also got Restore init=False dataclass fields when converting from JSON #1920, was this affiliated with you (we just closed because there was no CLA signature), or a crazy coincidence?

    No, completely unrelated to me, I don't recognize that contributor.

    1. Often, we would recommend using the pydantic_data_converter instead, as it is more robust in a number of ways than this hand rolled default. Do you have a reason that isn't an option? (A no here won't mean we won't accept a fix, just want context as this is far from the only limitation with this data converter)

    We've been using dataclasses forever and have dataclass inputs in a lot of workflows/activities. They work completely fine, outside of this limitation. So, not a technical reason, but it's just not a priority at the moment with all the other stuff we have going on. I am open to that change at some point in the future though.

    Counterpoint to my own argument, some quick AI research suggests pydantic does not do this, modifications to a frozen dataclass would be dropped, so that may be a good model to follow for consistency.

    Before learning about how pydantic does this, I would have said both options are reasonable with frozen=True. But now that you share this info, then I agree with you: Consistency with pydantic would be very desirable to avoid surprises for anybody migrating from one input form to another (read: Me from the future 😂).

  6. tconley1428 commented on Oct 8, 2026

    @tconley1428
    Contributor

    Sounds good. If you'd like to work on it feel free, or we can see if Amit wants to address the issues with their PR. Otherwise, I will do it but it will take a bit longer potentially.

  7. AmitKulkarni23 commented on Oct 8, 2026

    @AmitKulkarni23
    Contributor

    Hey @tconley1428 and @tomasfarias - I appreciate you guys taking a look at the PR. I will go through the discussion, understand your points and incorporate review comments in my next commit. Estimated Completion Date - Oct 09th

  8. AmitKulkarni23 commented on Oct 9, 2026

    @AmitKulkarni23
    Contributor

    Thanks @tomasfarias and @tconley1428. I've reworked this.

    • My first version skipped init=False fields entirely. That stopped the TypeError, but serialized values were lost on decode, and the fields were never type-checked, so a union could pick the wrong class.

    • It now follows Restore init=False dataclass fields when converting from JSON #1920: every field is converted, the init=False ones are set after construction (including on frozen dataclasses), and there are tests for round trip, frozen, and union matching.

    • On pydantic: 2.13.4 (TypeAdapter and Temporal's pydantic_data_converter) drops init=False values on all dataclasses, not just frozen ones, and gets the union case wrong too. This PR is stricter, and I'm happy to switch to matching pydantic if you'd prefer.

  9. tomasfarias commented on Oct 9, 2026

    @tomasfarias
    Author

    Thanks @AmitKulkarni23.

    It now follows #1920: every field is converted, the init=False ones are set after construction (including on frozen dataclasses), and there are tests for round trip, frozen, and union matching.

    On pydantic: 2.13.4 (TypeAdapter and Temporal's pydantic_data_converter) drops init=False values on all dataclasses, not just frozen ones, and gets the union case wrong too. This PR is stricter, and I'm happy to switch to matching pydantic if you'd prefer.

    I think there is a stronger argument to be made to dropping them from frozen classes, but honestly I don't see why pydantic wouldn't at least try to preserve state with non-frozen classes (not super familiar with pydantic myself).

    It's frustrating that the decoders don't agree here! Ultimately, maintainers should make a call about what to prioritize: maintaining compatibility or preserving state. For my particular use case this time, this doesn't matter: we are not updating the state after creating the inputs.

    I'll take a look at the PR and make some comments in case I see anything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions