natevw / node-chargify

Simple wrapper around Chargify's REST API for node.js
17 stars 12 forks source link

When Chargify sends a specific error, we may return a JSON parse error instead #7

Open natevw opened 3 years ago

natevw commented 3 years ago

When trying to run the tests after updating dependencies, I discovered that Chargify's API is kind of broken. For example, when one doesn't have valid API token, Chargify return a response like:

status: '401 Unauthorized',
'content-type': 'application/json; charset=utf-8',
body: 'HTTP Basic: Access denied.\n',

But regardless because Chargify claims this is "application/json" we try to parse it as JSON and so the resulting error is just something like:

 Uncaught SyntaxError: Unexpected token H in JSON at position 0

instead of something more related to the 401 which would be more helpful.

I wonder if older versions of the request module used to flag non-2xx responses like this as an err to our callback? Or if this is a temporary bug on Chargify's side?

natevw commented 3 years ago

(I'm not eager to spend my own time working around Chargify's broken responses but filing this as a known issue at least and would be willing to review a pull request.)

natevw commented 3 years ago

Oh, maybe this this is a longstanding thing, since in my notes about migrating to Fermata instead I mentioned one of the benefits being:

improved callback arguments (an error is now provided for bad status codes)