Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better exception message when throwing MalformedResponse #17

Closed
vitorhugods opened this issue Nov 17, 2017 · 11 comments
Closed

Better exception message when throwing MalformedResponse #17

vitorhugods opened this issue Nov 17, 2017 · 11 comments
Assignees
Milestone

Comments

@vitorhugods
Copy link

It is very hard to find where is the malformed part of a response when dealing with a big JSON.
Would be nice if a more specific message was shown, like "Could not find "key" in the JSON".

And also, I couldn't find a way to get the raw text response and print it to help me debug.

Thanks for the library. I really wish I could be of more help, but I'm learning Swift.

@sean7512
Copy link
Owner

The malformed error means different things depending on which Deserializer you are using (the old JSON or the Swift4 Codable way for JSON).

Can you post some sample code?

@sean7512
Copy link
Owner

As for getting the raw Data object:

do {
    let response = try result.value()
    // ...
} catch NetworkingError.malformedResponse(let data) {
    print("Error performing GET, malformed data response: \(data!)")
}

The data variable will be of type Data? which represents the raw bytes received. In a future RestEssentials release, it will not be an optional, but just a Data object. For now, you can safely force unwrap it (only for malformedResponse errors). It is not safe to force unwrap the data object for ANY other type of error.

@vitorhugods
Copy link
Author

vitorhugods commented Nov 17, 2017

First of all, thanks for such a quick answer!

After some source code digging, I found out that the Exception has the data, but I didn't know how to get it.
Now at least I can actually see the received JSON and send reports of failure to the API I'm using.

Thanks a lot!

About the Deserializer, I'm using only Swift 4's Codable for now. Some example:

    //Usage in my ViewController
    ServerClient.fetchStartupData(){ startupdata, httpResponse in
            if let data = startupdata{
                //Handle received data
            }else{
                print("Nope, Connection Error")
            }
        }

    //Expected response
    public struct StartupDataResult: Codable {
        public let name: String
        public let photo: String
        public let active_orders: [ActiveOrder]
        public let notifications: [Notification]?
    }

    //Global function that can be accessed in my whole project
    public static func fetchStartupData(callback: @escaping (StartupDataResult?, HTTPURLResponse?) -> ()) {
        let name = "buscaDadosDeStartup"
        let dados = Server.getBasicInfo() //Codable with API Token and more basic info, nothing much
        Server.client(callName: name).post(dados, responseType: StartupDataResult.self) { result, httpResponse in
            handleDefaultConnection(callback, result, httpResponse)
        }
    }
    
    //Some connections only require "Data or No Data", so I created this handler
    static func handleDefaultConnection<T>(_ callback: (T?, HTTPURLResponse?) -> (), _ result: Result<T>, _ response: HTTPURLResponse?) {
        do {
            print("Response = \(response)")
            let response = try result.value()
            callback(response, response)
        } catch {
            print("Error receiving fetchStartupData = \(error)") //I will modify this go get the raw data now
            callback(nil, response)
        }
    }

Probably there is a better way to do what I'm doing, but I'm already very happy for now.

Is there any way to get an exception message that indicates what key is missing in the JSON response, or something like that, that narrows my search for API misbehavior?
I've searched for JSONDecoder errors and I think those would be great tools to have.

@sean7512
Copy link
Owner

sean7512 commented Nov 18, 2017

It should already be logging the error that comes straight from the Swift API. If you look at the. DecodableDeserializer in Deserializer.swift you can see the error from the native JSONDecoder should be logged out:

print("Error decoding to JSON object \(error.localizedDescription)")

I am not sure how detailed the built-in decoder's errors are, but take a look there. You can breakpoint in there and see if there are different details in the error that isn't being logged out. If you find something else that I should log out, let me know.

@sean7512
Copy link
Owner

@vitorhugods Take a look at the last commit: 43b15f2

I changed the Deserializers so they no longer swallow error, but instead pass them back up. Now when you try to get the result.value() you will get the original error back in the failure case. So you should now be getting the JSONDecoder errors that you linked to. This will be in the next release (4.0.2) and I will release it once you confirm it looks good.

Thanks!

@sean7512 sean7512 self-assigned this Nov 29, 2017
@sean7512 sean7512 added this to the 4.0.2 milestone Nov 29, 2017
@vitorhugods
Copy link
Author

Seems really good to me. Really liked it. =)

Also, I've been thinking here about debugging and reports.
In Android I normally use Firebase/Crashlytics to send reports of connection, failures or edge-case.
It would be very nice to access the raw response data even successful cases as well.
Is this an issue-material?

@sean7512
Copy link
Owner

Ok, I'll start getting 4.0.2 released soon.

As for Firebase/Crashyltics, that would be integrated at the app level, not in the library.

Thanks for the suggestion on the error improvements!

@vitorhugods
Copy link
Author

I'm not saying it should not be in the app level. But so far, the way to get the raw response body in order to log or report, without deserializing is when an serializing error occurs, as seen here. Or is there any other way to get that?

@sean7512
Copy link
Owner

4.0.2 has been released and contains better error handling for Deserializers and it also removes the optional on the Data parameter for a malformedResponse error. I have also updated the Readme to include information on Error Handling.

As of now, there is no way to get the raw data back in a success case. There are some memory constraints with doing this. Imagine using an Image Deserializer on very large raw images, you would be using twice as much memory, which is probably not desirable. Or worse yet, maybe you used a Movie Deserializer.

If you use the built-in JSON type (instead of Codable), we do keep the raw response (it isn't accessible right now, it is private), but if there was a strong argument for it, I could expose it. The issue with exposing it is mutability and how that may break the json parsing. I guess it could be immutable getter only. Either way, feels awkward for only 1 of the deserializers to provide that functionality.

@vitorhugods
Copy link
Author

Understood. I've never though of images and videos.
I've worked in projects that some operations were really sensitive, complexes and cost big money. To the if any error was reported customer-wise, admins should be able to look logs back and see exactly what the app requested, and what it received from the API, so It would be fixed ASAP.
It never came to the point that a costumer reported something, mostly because even when in beta testing, these reports helped to correct mistakes.

@sean7512
Copy link
Owner

sean7512 commented Dec 6, 2017

@vitorhugods going to close now that the 4.0.2 release include much better error handling. Thanks a ton for bringing the issue to my attention.

@sean7512 sean7512 closed this as completed Dec 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants