search

restify / clients

HttpClient, StringClient, and JsonClient extracted from restify
MIT License
57 stars 34 forks source link

breaking: support parsing of valid falsy json #183

Open DonutEspresso opened 6 years ago

DonutEspresso commented 6 years ago

As part of revving dependencies, I discovered a regression in JSONClient. I'm not sure when it happened, it looks like the tests were not even checking for the right thing.

Essentially, if the server sends 0, null, or false, that is valid JSON and the client should respect it. This is similar to restify/node-restify#1609.

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.01%) to 87.927% when pulling 3470786128ad72c7b63081e3fa9f1783b20f74b2 on rev-restify into b86b6d7712cac97afdea8a7c6743dc99bf6d6483 on master.

misterdjules commented 6 years ago

Is that really a regression, it seems that the erroneous behavior may have been present for a while. For instance, the commit at https://github.com/restify/clients/commit/fb52f481adf8e7be9cd7c8249cb99e2547c88bf4#diff-1f5c7c63b7ce8433f5b0480296a5f265R57 uses:

obj = obj || {};

to set the response object. Wouldn't that have the same problem? If so, that commit was merged ~2015, wouldn't that make this PR semver major?

DonutEspresso commented 6 years ago

@misterdjules You're right - I confused this with restify's ability to send these values as strings (but not for restify-client's ability to parse those strings as valid JSON).

I guess this is indeed semver major.