wvteijlingen / Spine

A Swift library for working with JSON:API APIs. It supports mapping to custom model classes, fetching, advanced querying, linking and persisting.
MIT License
266 stars 109 forks source link

DeserializeOperation does not log all errors #124

Open markst opened 7 years ago

markst commented 7 years ago

I've had errors such resourceIDMissing & resourceTypeMissing which have been hard to debug initially due to no logging of these errors.

Since SerializerError's get promoted to SpineError's it's sometimes hard to debug what the issue is.

markst commented 7 years ago

What if serializerError passed it's error as argument like network & server errors do?

/// An error occured during (de)serializing.
case serializerError(serializerError: SerializerError)
wvteijlingen commented 7 years ago

I agree that this is a problem with the current Error handling. I'm still not sure about whether promoting everything to a SpineError is the way to go. On one hand it's easy to catch all Spine errors, because they are al SpineError types. On the other hand it feels like unnecessary wrapping and leaky abstracting.

I believe the reason I chose this was because Swift needed a concrete type for the Futures returned by Spine. So I think the only solution for now is indeed to wrap serializerError like you suggested.

markst commented 7 years ago

Happy to PR for this one. Not sure whether to add a log within the DeserializeOperation

} catch let error as SerializerError {
    Spine.logInfo(.serializing, error.localizedDescription)
    result = Failable(error)
    return
} catch {

or the owner:

switch serializeOperation.result! {
case .failure(let error):
    Spine.logInfo(.serializing, error.localizedDescription)
    throw error
case .success(let data):
    return data
}
wvteijlingen commented 7 years ago

I think the best place is to log them using logError in the guards in the DeserializeOperation.

guard representation.dictionary != nil else {
    throw SerializerError.invalidResourceStructure
}

guard let type: ResourceType = representation["type"].string else {
    throw SerializerError.resourceTypeMissing
}

guard let id = representation["id"].string else {
    throw SerializerError.resourceIDMissing
}