[NT-463] Improve error handling for graph requests

[ifbarrera]

📲 What

This PR makes some changes to how we make Graph requests, and how we parse responses from Graph requests.

🤔 Why

To improve error parsing/handling for graph requests and make our requests more robust to changes in the JSON structure coming back from the API. Also, to make graph requests more consistent with how our v1 requests are made.

🛠 How

Previously, we made all graph requests through a single performRequest function that started a data task within a producer and parsed errors and data in the completion block of that task. This approach was a little inconsistent with our approach for v1 requests, which splits up error handling and model decoding by running all error handling through rac_* helper functions defined in an extension of URLSession. These helpers also make use of Reactive Swift's data(with request:) function which is specially designed to return a SignalProducer which "performs work associated with an NSURLSession". It seemed like a good idea for Graph requests to follow a similar pattern of doing error handling in a similar rac_graphDataResponse function, and then running the resulting Data through a decoding function (decodeGraphModel).

Now, v1 requests are made like this (no change here):

 return Service.session.rac_JSONResponse(preparedRequest(forURL: paginationUrl))
      .flatMap(self.decodeModel)

... and Graph requests are made like this:

return Service.session.rac_graphDataResponse(request)
        .flatMap(self.decodeGraphModel)

♿️ Accessibility

N/A

🏎 Performance

N/A

✅ Acceptance criteria

Setup: Using Charles Proxy, set up a breakpoint such that all requests hitting the Graph endpoint are intercepted.

• Test that aborted requests correctly show a request error.
• Test that removing the "data" field from a request with a validation error (ex. entering the wrong password when trying to change your email) does not affect parsing of the returned error. The message banner should still show the validation error returned from the API.
• Test that errors returned from the API (ex. entering the wrong password when changing your email) are correctly parsed and displayed.
• Test that modifying the response body to an invalid object correctly produces a decoding error

[ifbarrera]

Here we decouple data and errors by removing the errors from GraphResponse and giving them their own GraphResponseErrors struct. This allows us to decode errors separately from data.

[ifbarrera]

The performRequest function has been removed in favor of a decodeGraphModel function that decodes Data into objects, and an rac_graphDataResponse function which is responsible for actually making the request and parsing any resulting errors.

[justinswart]

Thanks for digging into this, I think this is certainly better than what we have currently!

Now that I've had a bit of time to think through it a bit more I have some suggestions since our pairing on it, just to simplify and improve readability I think.

[justinswart]

This all checks out! Only thing to mention is that during testing, it correctly produced a decoding error by way of entering that part of the code, but the message is still just "Something went wrong". Was that your intention?

If so -

[ifbarrera]

This all checks out! Only thing to mention is that during testing, it correctly produced a decoding error by way of entering that part of the code, but the message is still just "Something went wrong". Was that your intention?

If so -

By "decoding error" are you referring to the error that results if the structure of the data doesn't match our expectation? If so, then yes this is the expected behavior.

[justinswart]

By "decoding error" are you referring to the error that results if the structure of the data doesn't match our expectation? If so, then yes this is the expected behavior.

Yes. The console of course still said that it was a decoding error.