Added check for invalid responses which are not empty. #308
Conversation
There was a problem hiding this comment.
this looks good to me. one improvement you might consider would be getting the error and logging: http://php.net/manual/en/function.json-last-error.php
|
fyi, i sorta accidentally turned on StyleCI when i was futzing with packagist last week. i just disabled it again. |
|
@acinader that would be helpful to have a diagnostic message of some sort. There's also Yea probably best to turn that back off, looks the analysis page isn't even reachable :/. |
|
how about a test :)? If its too much for you to get to, just merge it. |
|
I can add a test, but the only way I can see firing this new code is by running it against the old api.parse.com. That would work as a case, but if it's ever corrected we won't have anything to test against :/. Unless we have a special dummy server setup that explicitly returns text with the wrong content type. I did write a test locally to check this but decided against adding it for the above reason. However in the interest of covering it I can go ahead and add it in? We'll just have to keep an eye on it in case they correct the old api in the future. |
|
nah, if you tested locally. if you could mock it, but if that isn't easy, i don't think introducing an external dependency is worth it. |
|
I'll keep a thought on it. There's probably a way to mock up something more reliable... I'll see if I can't come up with something. |
This double checks whether a response from the server was decoded successfully. It additionally checks if there was any response to begin with, to avoid false positives on such operations as ParseFile deletion where no response is returned.
This is in regards to #307 , which has pointed out that https://api.parse.com/ now returns a response indicating the service has shut down. This is returned as plain text, however the Content-type indicates application/octet-stream. As a result the sdk tries to process the request to no avail.
Although noone should be using the old api, this makes a point that the sdk could try to run after an invalid response with the 'right' content type. This change prevents that from occurring.
This does not add tests, as this is an unlikely error in how the sdk handles responses. That and the resulting tests would have to target the old api to replicate this, and that may very well be corrected at some point in the future.