Supporting non-json responses

[dhritzkiv]

I have use cases where the data that I'm requesting is not application/json, but is text/html or application/xml. I was planning on parsing this incoming data in ampersand-model's parse function, and setting the model's properties there.

However, the current logic of ampersand-sync prevents that from happening. See the code from lines 117 of ampersand-sync.js:

if (body && typeof body === 'string') {
    try {
        body = JSON.parse(body);
    } catch (err) {
        if (options.error) return options.error(resp, 'error', err.message);  
    }
}
if (options.success) return options.success(body, 'success', resp);

Here, if the body is a string (true of JSON, HTML, XML…), the code will try and parse it as JSON, which will fail for non-json, and result in options.error being called, thus never making it to the model's parse function.

Is there a solution which doesn't involve calling options.error when the data can't be parsed as JSON? It could be as simple as not doing anything in the catch block.

[wraithgar]

It seems we could be paying better attention to the json option at that point in the code?

[sansthesis]

I think that this is a regression introduced in 1597cd1b1525342ab0a80a4f3b0932ceae5f0ec6. I was relying on this behavior before, with the same xml use case, and it broke when I updated all of my npm dependencies.

[dhritzkiv]

I can submit a pull request that checks, near L117, if headers["accept"] is still set to application/json (default)*, or overridden to something else (application/xml, for example).

*I'll wait 'til #24/#45/#46 get merged, so there's no duplication of efforts/lost work.

[naugtur]

45 #46 merged. Let's address this before 4.0.0 release.

edit: #24 too.