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

request.perform fails despite Model being initialized with the desired json #69

Closed
1 task done
lk251 opened this issue Apr 6, 2019 · 6 comments
Closed
1 task done

Comments

@lk251
Copy link

lk251 commented Apr 6, 2019

What did you do?

let request: APIRequest<Home, JSONError> = tron.swiftyJSON.request("/twitter/home")
 request.perform(withSuccess: { (home) in
            print("Fetched json objects")
            print(home.users.count)

        }) { ("Failed to fetch json, ", err) in

            print(err)
        }

What did you expect to happen?

I expected the request to be successful and home.users.count to be printed. Especially because upon initialization of the home object, it successfully prints the desired json.

If it would fail, which it shouldn't, I expected the fail closure to give me information on the error (I'd think it would print eg. the error and response variables that the ErrorSerializable-conforming object err should have.

What happened instead?

I got this error:
Failed to fetch json, Projectname.HomeDatasourceController.JSONError

TRON Environment

TRON version: 5.0.0-beta.1'
Xcode version: 10.2
Swift version: 5
Platform(s) running TRON:
macOS version running Xcode: 10.14.3

Demo Project or sample code

ASAP

@DenTelezhkin
Copy link
Member

Hi!
For more information you could subclass APIError with your JSONError, or you can implement JSONError in the way that accepts all HTTP stuff ErrorSerializable accepts.

@lk251
Copy link
Author

lk251 commented Apr 6, 2019

Hi @DenTelezhkin , thanks. Subclassing with ErrorSerializable, like so,

class JSONError: ErrorSerializable {
        required init?(serializedObject: Any?, request: URLRequest?, response: HTTPURLResponse?, data: Data?, error: Error?) {
            print("MY JSON ERROR")

        }
    }

Produces errors like:

Value of type 'HomeDatasourceController.JSONError' has no member 'response'
Value of type 'HomeDatasourceController.JSONError' has no member 'error'

When trying to print err.error or err.response.

Additionally, shouldn't the request succeed, given that the Home object (a JSONDecodable) is initialized with the desired json? (This was checked by printing out the json upon initialization)

Thank you again in advance.

PS. In case this information serves, printing out the JSONError vars upon initialization:

class JSONError: ErrorSerializable {
        required init?(serializedObject: Any?, request: URLRequest?, response: HTTPURLResponse?, data: Data?, error: Error?) {
            print("MY JSON ERROR")
            print("Error is ", error)
            print("Response is ", response)
            print("Serialized object is ", serializedObject)
        }
    }

Produces

MY JSON ERROR
Error is  nil
Response is  Optional(<NSHTTPURLResponse: 0x6000023e95e0> { URL: https://api.letsbuildthatapp.com/twitter/home } { Status Code: 200, Headers {
    Connection =     (
        "keep-alive"
    );
    "Content-Type" =     (
        "application/json"
    );
    Date =     (
        "Sat, 06 Apr 2019 12:26:18 GMT"
    );
    Server =     (
        Cowboy
    );
    "Transfer-Encoding" =     (
        Identity
    );
    Via =     (
        "1.1 vegur"
    );
} })
Serialized object is  Optional(Myproject.HomeDatasourceController.Home)
Failed to fetch json Myproject.HomeDatasourceController.JSONError

@lk251
Copy link
Author

lk251 commented Apr 6, 2019

Trying your other suggestion, subclassing APIError, did work! Thank you.

(Still interested in what's wrong with the Errorserializable approach.)

P.S. Thanks for your awesome work!

@DenTelezhkin
Copy link
Member

DenTelezhkin commented Apr 6, 2019

I feel that probably ErrorSerializable intention was not telegraphed well in the documentation, that's something I will improve. Current design assumes that if response validation was successful, you should return nil in implementation of optional initializer for ErrorSerializable, and if you don't - object is created and TRON considers this object as a validation error that happened while processing the response.

It's possible I may revert this change and go back to throwing initializer or do something else to improve current behavior. I would like to keep this issue opened, if you don't mind, I want to think about it more.

@DenTelezhkin DenTelezhkin reopened this Apr 6, 2019
@DenTelezhkin DenTelezhkin self-assigned this Apr 6, 2019
@DenTelezhkin
Copy link
Member

I changed ErrorSerializable to have non-optional initializer and ensure to only be called when error does actually happen instead of calling it all the time, this should be more straightforward and easier to use.

I made a new beta release with this change and several others.

Thanks for your feedback, it really showed me a flaw in my API design. 👍

@lk251
Copy link
Author

lk251 commented Apr 6, 2019

@DenTelezhkin My pleasure! Thanks a lot for this awesome library and for the response.

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