Provide codingPath
when decoding fails due to corrupt data
#10
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.
🎯 Motivation
When a data corruption error occurs, it's currently hard to tell what was responsible for the corruption. Beside indeed corrupted data, corruptions can be caused by mismatching
Encodable
andDecodable
implementations, as well as internal errors of this library around the coding and decoding process.Codable
allows to maintaincodingPaths
, which is already partially implemented. However it is not fully utilized yet to indicate where an errors occurs throughout the decoding process.This PR makes sure that all errors due to data corruption are annotated with the coding path of the currently decoded key. As all errors thrown are of the case
DecodingError.dataCorrupted
, it is sufficient to handle just said errors.💪 How
This PR adds a private helper method
wrapError(forKey: _, _)
toKeyedContainer
, which catches the relevant case ofDecodingError
and composes the path, based of the containers coding path, the given key and coding path already set on the error's context. This is done when decoding nested values (viadecode(_, forKey: _)
) and retrieving nested containers.Note some limitations:
KeyedContainer
of BinaryCodable is eagerly reading all keys on initialization. If a container's data is syntactically invalid (e.g. keys or values of invalid length), then this will fail early. This is handled by additional error handling logic in theKeyedContainer
's initializer, but if a data corruption causes a byte length difference within a nested key or value, this cannot be indicated and is likely to cause an error at the next sibling or an ancestor in the tree.KeyedContainer
does not include yet the super key on decoding errors within the coding path.In similar fashion, this PR also utilizes now the private helper method
wrapError(_)
inUnkeyedContainer
. This was previously unused. Further it does update said method's implementation, so that it does attribute the error to a coding path which includes thecurrentIndex
. To achieve that, I've added a helper structAnyCodingKey
to the library, which is essentially a type-erasedCodingKey
allowing to represent arbitrary coding keys values without a concrete type.🧪 Testing
To ensure the correctness of the added error handling, I've added a relatively comprehensive test case
CodingPathTests
. I have chosen a white box testing approach here, coming up with a series of tests cases which should ensure to hit all relevant code paths. I've done this by relying only on the public API by testing through decoding crafted corrupted data.To achieve this, all tests:
This ensures that the tests stay maintainable, by being self-documenting and preventing that changes to the utilized data structure lead to cryptic failures, because the crafted corrupted data mismatches.
Within the tests, I've relied on some helpers structs:
NestedBox<T>
: This is consistently used as the root value of all encoded values to ensure that there is at least one coding keyval
inherited throughout the coding path. This enables the tests to validate that this inheritance is correctly taking place.CorruptBool
: This type decodes a booleanval
. If it istrue
, then it decodes without errors, but it throws a data corruption error when attempting to decode afalse
value. Once again, this is encoding/decoding to a keyed nested container to test that the presence of the coding key within the coding path.Most tests rely on a single byte being removed or 1<>0 bit flips. While some of these tests might even cover useful real-world scenarios with correctly working encoding and decoding implementations, this is not given for all tests. Particularly when bytes were removed, some tests are also making sure to account for this in the encoded byte lengths to ensure that the error is only thrown for a coding path further nested in the tree. Such errors are obviously extremely unlikely to occur in the wild by chance. But this seems useful in development, when encoding and decoding implementations are likely to be not correctly aligned.