DeviceType serialization and deserialization is a bit confused.

[mhammond]

On the back of removing the dupes of DeviceType (#4397), , it turns out there's still significant confusion around DeviceType - specifically:

• We now say that DeviceType::Unknown exists purely to avoid Option<DeviceType>, but Option<DeviceType> exists in a number of places (including the struct immediately above that comment 🤦 ).
• We have #[serde(skip_serializing)] on the Unknown variant - so we Err() if we attempt to serialize that value. However, we happily serialize None and explicitly write "type": null in the serialized JSON - which is not what we want (we'd prefer to omit it entirely IMO) - but regardless, having None and DeviceType::Unknown with different semantics is clearly wrong.

All of which is confusing and a footgun. The options seems to be:

• Remove the Unknown variant and embrace Option<>
• Remove the Option's

I vote the latter, simply because it being an Option is the minority of cases (ie, embracing Option would touch far more structs)

Which still leaves the question of: how do we serialize and deserialize? I vote:

• We continue to fail when attempting to serialize Unknown (ie, never serialize null). This seems fine, because in practice we never need to serialize records other than our own, and it seems reasonable to insist that we know what type of device we have.
• We will continue to deserialize null and unknown values - which implies due to the above that we will fail to round-trip such values via json, but that seems OK.
• If it turned out there was a need to serialize these values, we would opt to omit the field completely (ie, as above, we should never write null).
• The above is only for JSON serialization - we might still want/need to write such values to a database (eg, for tabs, where we want to record the tabs for a device, but it fails to provide the type in its client record.) What we do there will be decided on a case-by-case, but valid options might be to write DeviceType::Mobile, or even to write the string Unknown)
• We needs tests for these cases, which we currently do not.

WDYT?

┆Issue is synchronized with this Jira Task ┆epic: FxA Ecosystem (backlog)