search

MichaelErmer / eveonlinejs

Node.js/io.js EVE API Client
MIT License
27 stars 4 forks source link

Handle HTTP status code 403 #4

Closed mickdekkers closed 8 years ago

mickdekkers commented 8 years ago

The API responds with HTTP status code 403 upon authentication failure or when a key has expired.

Auth failure: https://api.eveonline.com/account/APIKeyInfo.xml.aspx?keyID=1 Expired: https://api.eveonline.com/account/APIKeyInfo.xml.aspx?keyID=2048

This would previously always cause eveonlinejs to report an Unsupported HTTP response: 403 error.

This commit instead lets the parser handle the error, which provides more useful error messages. See: http://wiki.eve-id.net/APIv2_Eve_ErrorList_XML

NB: this almost certainly qualifies as an incompatible API change as defined by semver, and if accepted, will require a major version increment before publishing.

MichaelErmer commented 8 years ago

Good catch! I will merge this later this week. Should we alter it to return the error message from https://api.eveonline.com/eve/ErrorList.xml.aspx ?

mickdekkers commented 8 years ago

Should we alter it to return the error message from https://api.eveonline.com/eve/ErrorList.xml.aspx ?

It already does. Now that it no longer treats a 403 as an Unsupported HTTP response error, the parser can catch it. The code for this was already in the library, it just did not get a chance to execute because the 403 got caught elsewhere.

MichaelErmer commented 8 years ago

Merged and version bumped to 2.0.0, npm is at 2.0.0 now as well.

mickdekkers commented 8 years ago

Thanks, @MichaelErmer.

I think something went wrong with the merge though. The version somehow went from 0.0.1 to 2.0.0:

Furthermore, your repo's history looks like this: MichaelErmer/eveonlinejs history

These three commits should not have been included in the merge, since they were part of an experimental and ultimately unnecessary feature on our eve-apps fork:

Did you accidentally merge all the changes in our eve-apps/master branch?

MichaelErmer commented 8 years ago

That seems to be the case, I might have time tonight to look at the mess i created. Chances you are on tweetfleet slack or evedev irc?

mickdekkers commented 8 years ago

Cool, I had no idea those existed. I've requested an invite to the Tweetfleet Slack. Also, is this the evedev IRC? http://irc.lc/coldfront/eve-dev I'm on there now and I will be idling there for a while, assuming it doesn't kick me.