SE-0396: Conform Never to Codable

this is also true in algorithmic trading. if you crash in the middle of a trade (i.e. before you have hedged) recovery can be very painful. i once got into an argument with a friend who is a C++ algotrader, over whether crash on precondition failure is a good idea. (he won.)

1 Like

not always seriously wrong, you might just be doing schema validation. if you decode Never, that means you are decoding a newer/unsupported version of the schema, and you need to update the client to decode it.

exactly

3 Likes

From the perspective of such a client, what's the difference between "data encoded by a newer client with a different schema" and "data that is corrupt with respect to the current schema"? Communicating that via "I decoded a Never" seems kinda dubious anyway, and I'm not sure typeMismatch would accurately describe this situation either.

Invalid JSON is certainly one way that data can be corrupt, but it's not the only way. I don't think its possible to say that {"hi": 1} is 'totally fine' in a vacuum, but if its being used to decode a type which will attempt to decode Never for that data due to some broken invariant, it doesn't seem reasonable to say that the data is fine.

1 Like

one of them can be an expected failure when clients are gradually being migrated to a newer version. but there are many kinds of data corruption that should never be expected, they indicate that something is wrong with the server/cluster or something malicious/malformed made it past server-side schema validation.

2 Likes

What I'm getting at is that to the outdated client, it all looks the same. It's all data which doesn't conform to the expected schema unless you build specific logic to catch the "encoded by a newer client" situation, in which case it seems to me that typeMismatch and dataCorrupted are both equally inappropriate errors—you probably want some bespoke wrongVersion error that you've designed for the specific purpose rather than just having some general error fall out of the decoding process.

4 Likes

+1 on the general concept, but I agree with other commenters here that attempting to decode a Never should throw a typeMismatch instead of dataCorrupted.

typeMismatch is a more accurate error than dataCorrupted — it's described as "an indication that a value of the given type could not be decoded because it did not match the type of what was found in the encoded payload", which is exactly the error that occurred in the situation. Trying to decode a Never value could indicate that the data is corrupt in some contexts, but interpreting that should be left to the caller. Additionally, in a JSON decoding context, dataCorrupted specifically means that the data is not valid JSON, not just that the data is considered "invalid" (see my response to Jumhyn below).

typeMismatch is also a more precise error than dataCorrupted — If you see a typeMismatch(Never.self, ...), you can be pretty sure it came from trying to decode a Never value. If you see a dataCorrupted error, then there's no hint that it came from trying to decode a Never.


The documentation for JSONDecoder.decode states:

If the data isn’t valid JSON, this method throws the DecodingError.dataCorrupted(_:) error. If a value within the JSON fails to decode, this method throws the corresponding error.

Decoding a Never doesn't mean the data isn't valid JSON, so dataCorrupted wouldn't be an appropriate error to throw in that context.

6 Likes

JSONDecoder, defined in Foundation, uses DecodingError.dataCorrupted, defined in the standard library, to communicate that the decoder encountered invalid JSON, but this doesn’t mean that the only way for data to be corrupted is for it to be invalid JSON.

Anyway, at this point I’m probably arguing beyond how strongly I feel about the choice of specific error here. :slight_smile:

With a decent debug description I think both options are acceptably expressive.

3 Likes

The way I interpret DecodingError.typeMismatch is "in the data, I expected to find the type I declared in my model, and instead I found ___", so the point of reference is the model, not the data.

For example, with this model

struct Foo<Bar>: Decodable where Bar: Decodable {
  var bar: Bar?
  var baz: String
}

struct PossibleBar: Decodable {
  var value: Int
}

if I try to decode Foo<PossibleBar> from the JSON data

{
  "bar": {
    "value": 42
  },
  "baz": { "ok": 1 }
}

I get DecodingError.typeMismatch with description Expected to decode String but found a dictionary instead., so it's clear that what I "expect" is what I declared in the model.

But if my model is Foo<Never>, I don't "expect" to decode a Never at the bar key... the point of Never is exactly that it cannot be instantiated. So, if I try to decode this JSON data

{
  "bar": true,
  "baz": "yello"
}

Getting a DecodingError.typeMismatch with description Expected to decode Never but found a Bool instead. makes no sense, I cannot expect to decode Never, and I use Never precisely to express the fact that I don't expect to decode something at that key.

The only valid JSON data would be

{
  "bar": null,
  "baz": "yello"
}

or

{
  "baz": "yello"
}

so I feel that "data corrupted" better represents the underlying issue (if we don't add a completely new error case).

2 Likes

Solution: Add a new error type:

extension Never {
    enum DecodingError: Error { ... }
}

I don't think that's good idea: it's simpler to always expect a DecodingError from JSONDecoder, in case we do some extra logic based on the error type (in the case of my team, we do). I'd prefer a new case on DecodingError, but overall I think thet simply using dataCorrupted is the best solution.

Is this even possible? In general, adding a new case to an existing enum is source-breaking.

This is muddling the agent here: the "I" who is doing the decoding isn't the "I" who is choosing Never for the generic parameter. If we reject the argument that it isn't definitely a logic error to try to decode Never (e.g., in the generic context; see @Joe_Groff's post), then attempting to decode would be a fatal error. However, if we accept that argument (and thus attempting to decode should throw), then we are saying it does make sense that "I" (the decoder) expects to try to decode Never, and the most specific description of the problem is that the data encountered is an instance of a different type.

1 Like

I don't think I agree (or maybe I don't understand your point). I definitely think that attempting to decode Never should throw an error, but the error to me is caused by the fact that, with Never, I (the programmer) am saying "I don't expect to decode this at all", not "I do expect to decode this specific type, and instead I'm getting a different type": the fact that the data that I received will make the decoder try to decode it seems to me bad data, rather than data that makes sense but I'm not modeling it correctly, because of the semantics that I want to express with Never. In other words, if I write Never I'm not saying "I expect to decode Never": I'm instead effectively saying "no type is fine here".

The error produced when "no type is fine here" might as well be typeMismatch, but the latter, at least to me, seems a little more "recoverable", like "you wanted type X and instead got type Y", which is not what I would express with Never.

2 Likes

IIUC, the current debate is basically:

"If we take a chunk of Data and hand it to the Never type so that it can make an instance of itself, what error should we throw?"

I know that making Never conform to Codable requires us to answer this question.

(For the record I think I'm convinced by Xiaodi's arguments for typeMismatch)

Does anyone else agree with me that it may prove relevant to that debate to simultaneously discuss ways in which we can avoid calling Never.init(from:) in the first place?

Specifically, again, I'm proposing that it really should be the responsibility of Result<Int, Never> to handle throwing a truly correct error when decoding the data: {"failure":{"errorCode":1}}

If you, the writer of code that’s doing the decoding, expect never to attempt decoding Never, then it’s a logic error if your code ends up attempting to do so. If you, the writer of the post, think that’s necessarily what goes on every time an attempt to decode Never takes place, then it’s not a recoverable error and should not throw but fatal error.

Put another way, if you believe in the second half of your statement above, then Swift’s error handling philosophy would disagree with the first half of your statement, and vice versa.

To be clear, I am convinced by Joe’s argument that it’s not necessarily a logic error to attempt to decode Never, for which reason I agree with throwing an error. But it it’s not a logic error, then the reason that best describes the recoverable problem in my view is that the data being decoded has a value there, which is necessarily of some type other than Never, where no value can be instantiated.

5 Likes

I think implicit in "I don't expect to decode this at all" is "from valid data," and the nature of Codable is that it is deliberately robust against potentially invalid data. I understand the view that decoding Never is in some sense a type error since Never is the canonical type which admits no values, but it still strikes me as somewhat different in kind than other sorts of type errors.

I think the idea of recoverability that @ExFalsoQuodlibet brings up gets at the mental disconnect I'm feeling. With a typeMismatch error decoding the key "foo" from some piece of data { "foo": ... } it feels like there's an implicit expectation that you could resolve the error by adjusting the type of the data in ..., but when it comes to Never the error occurs before you even get to inspecting ...—the data was invalid as soon as you found the key at all.

1 Like

Thanks for this discussion so far!

I've taken a look at the ways that errors are used in JSONDecoder, and the dataCorrupted error seems to be used both in cases of invalid syntax (e.g. malformed JSON) or invalid data (e.g. an invalid date or URL string). Given that, I agree with those arguing that the typeMismatch error is a better fit. In addition, given the existing documented errors, typeMismatch is a more useful error for a developer who encounters this kind of error than the alternatives (either dataCorrupted or a new error created for this purpose).

I've updated the implementation and the proposal with this change.

17 Likes

Can you clarify why you think it “should” be that way? Why would it be insufficient to delegate to Never.init(from:)? If there isn’t a single correct way to implement an initializer attempting to decode Never, then isn’t that an argument that Never doesn’t truly meet the semantic requirements of conformance and we ought to reject this altogether?

Well yes, immediately after my last post I started writing a new post suggesting that we reject this pitch (on the grounds that it is philosophically unsound to conform Never to a protocol with initializer requirements) and approach enabling the desired use case (e.g. Result<Int, Never> being codable) in a different way, but then I realized some conceptual complications so I deleted it to keep thinking about it.

This is exactly why I'm saying that we "should" consider thinking one level up from the data that we're attempting to decode as Never, because up there (e.g. at the level of Result<Int, Never> instead of at the level of Never) there is a more objectively correct error to throw (namely, whatever error a normal enum throws when trying to decode itself from: {"notOneOfItsCaseNames":{"someData":7}}).

The alternative that I was going to propose can be hand-wavily described like this:

"Rather than Never conforming to Codable such that all of the cases of Result<Int, Never> are Codable such that Result<Int, Never> is Codable, we instead give Never the special power to "stamp out" individual cases from enums, allowing whatever's left to conform to Codable if it is able, thereby avoiding this impossible question of which error to throw when decoding Never directly."

However, as I began to think about the completely concrete proposition I was making I began to have doubts about its coherence. Since you asked though, I've written here what I have in mind so far so that anyone who wants to can run with it.

1 Like

That's a great point. Conceptually, with one expects data of type A but finds data of type B, one can reconcile the problem either way, and it's certainly true that going in one of those directions is foreclosed in the case of Never.

Notionally, though, that represents a degenerate case rather than a difference in kind. Pragmatic considerations may override, but I find it hard to imagine, in the generic context where attempting to decode T isn't necessarily a logic error where T == Never, how such code could "adjust the data" to be valid for any type T upon encountering DecodingError.typeMismatch but for the Never case.

2 Likes