fix(shadows): accept bare-string unit variants in externally-tagged parse_delta#129
Merged
KennethKnudsen97 merged 2 commits intoMay 26, 2026
Merged
Conversation
…arse_delta
The derive emitted `parse_delta` for an externally-tagged enum with mixed
unit + newtype variants only handled the object form (`{"dhcp": ...}`,
`{"static": {...}}`). Serde, by contrast, serializes unit variants as a
bare string (`"dhcp"`), and AWS echoes that shape back via shadow
desired/delta — causing devices to loop on `InvalidPayload`.
Add a bare-string fast-path before the FieldScanner call: trim ASCII
whitespace, and if the value starts with `"`, match it against each
unit variant's quoted name; fall through to the existing object-form
scanner otherwise. Brings the macro's behaviour into line with what
`serde::Deserialize` already accepts.
MathiasKoch
reviewed
May 26, 2026
MathiasKoch
approved these changes
May 26, 2026
MathiasKoch
left a comment
Member
There was a problem hiding this comment.
One minor thing.
Also, could we remove some of the unnecessary comments, and keep the comments to a strict "why we are doing this" on code that is not directly clear or is complex, as opposed to the current "what we are doing" style?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
For an externally-tagged enum with mixed unit + newtype variants (e.g.
IpSettings { Dhcp, Static(...) }), the derivedparse_deltaonly handled the object form ({"dhcp": ...},{"static": {...}}). Serde itself serializes unit variants as a bare string ("dhcp"), and AWS echoes that shape back via shadow desired/delta — so devices loop onInvalidPayloadwhenever the cloud has the unit variant set.This adds a bare-string fast-path before the
FieldScannercall: trim ASCII whitespace, and if the value starts with", match it against each unit variant's quoted name; fall through to the existing object-form scanner otherwise. Brings the macro's behaviour into line with whatserde::Deserializealready accepts.Found via factbird-mini #667.
Test plan
cargo test --lib --features std,shadows_kv_persist— 177 pass (incl. 4 new insrc/shadows/shadow/tests.rsunder6.4 parse_delta accepts both bare-string and object forms)[patch]— the previously failingparse_known_network_with_bare_string_ip_settingsregression now passes