Possible issues after switching to [email protected]

[naugtur]

• c-r-mixin.fetch is passing options down to sync, which in turn passes them to xhr. we need to consider consequences of field names matching. Maybe it should create a new object and only pass relevant fields?
• signature of the response object passed in to success and error function changed. Is it a problem?

[dhritzkiv]

Seems that the switch to sync@v4 has caused issues in this module where the err argument in getOrFetch and error callback in fetchById have been producing empty Error objects. I've been seeing this bug periodically popping up in my logs over the last few months (as 404s and timeouts –conditions which cause this– are fairly rare)

I believe it all In fetchById the error callback lists the arguments as (collection, resp), which is right. However, resp is not a standard response/request object; it has these fields:

• body,
• headers,
• method,
• rawRequest,
• statusCode,
• and url

The error handler then tries to access resp.statusText, and resp.status, neither of which are defined. Instead, they are defined in resp.rawRequest. As a result of statusText and status being undefined, a not-very helpful Error is produced.

Could be as simple as change this code (starting at L103):

if (cb) {
   var error = new Error(resp.rawRequest.statusText);
   error.status = resp.rawRequest.status;
   cb(error);
}

Not sure if this affects fetch (specifically via the private wrapError function).

I'm surprised this wasn't caught in the test suite.

[hailwood]

I've run into this as well however I would be more inclined to simply always pass resp into the cb, possibly adding a success key based on the statusCode rather than an error since it's never thrown it's not like you can wrap the fetch request in a try/catch block, nor do the line/file numbers line up to anything useful so we gain no advantage for using Javascript errors, and worse dumping this out in the console yields nothing useful.