small fix to client's fetch error handling

[chadxz]

so what was the problem? it's non-obvious to me :/

[syjulian]

Before, when I would get a 500 error, this if-check wasn't being hit on public/app/index.js

if (response.statusCode < 200 || response.statusCode >= 300) {      
  throw new Error(response.statusText);     
}

It turns out response doesn't have a statusCode property but just status

So changed that to check for status instead.

I also noticed that the object here that is being sent by the express server from lib/routes/restaurants.js on a 500 error

if (!googleApiKey) {
    res.status(500).send({
      error: 'internal server error',
      statusCode: 500,
      description: 'server not configured for restaurant lookup'
    });

isn't on response when doing the callback for fetch() in public/app/index.js. I looked at the fetch polyfill documentation, and it showed that I would need to call response.json() to return a promise that resolves to the object.

I extracted out the checking of the status to getStatus() and the returning of the response.json() to getJson(). I tried to make it look like the way fetch() was used in its documentation.

This was how I understood things. I hope it made sense.

[chadxz]

Hey Julian, thanks for the details :+1: Regarding point 1 about statusCode vs. status, that makes sense! We'll definitely need to fix that. Regarding point 2 though, I don't think any of the remaining changes you made actually address the problem you described. The error still does not have the response body as part of the error.

[syjulian]

Yeah, regarding the second point, the change I did didn't really use the response body. It's using the status property that comes with XHR. I was just assuming it might be the conventional way to do it.

But, if you check the response object during the callback when json.response() is called, it will contain the statusCode property along with the others in the response body.

If you want, I can change it to use the response object to make the status code checks and the error description.

[chadxz]

So what I'd like to see to merge in this change is to make the getRestaurants actually resolve to the restaurants array like it was before. I feel like it makes the most semantic sense that way. I'm fine with keeping the helper functions broken out if you want.

[syjulian]

Ok, I took out the helper functions and added their logic back to the getRestaurants. Also, I changed it so that it uses the resolved response object for checking the status code with response.statusCode and showing the error info with response.description.

[chadxz]

Thanks Julian!