Repository navigation
[Bug] Dataclass fields with init=False break payload round tripping #1934
Description
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 in1.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
AmitKulkarni23 commentedon Oct 6, 2026
First time contributor here. I would like to take this up and work on this today
AmitKulkarni23 commentedon Oct 7, 2026
Created a PR to address this - #1942
tconley1428 commentedon Oct 7, 2026
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.
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:
pydantic_data_converterinstead, 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)tconley1428 commentedon Oct 7, 2026
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.
tomasfarias commentedon Oct 8, 2026
Hey @tconley1428, thanks for taking a look. To answer your questions:
No, completely unrelated to me, I don't recognize that contributor.
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.
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 😂).tconley1428 commentedon Oct 8, 2026
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.
AmitKulkarni23 commentedon Oct 8, 2026
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
AmitKulkarni23 commentedon Oct 9, 2026
Thanks @tomasfarias and @tconley1428. I've reworked this.
My first version skipped
init=Falsefields 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=Falseones 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.
tomasfarias commentedon Oct 9, 2026
Thanks @AmitKulkarni23.
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.