API Updates and Promise Support

[evanshortiss]

I've used this wrapper to perform some simple interactions with the Robinhood API and have a few things on a wishlist that would improve developer experience (IMO). These changes would mean a breaking 2.0 change so it's not a small piece of work, but I'd like to hear your thoughts.

Response Structure

Presently, the library uses request format callbacks because under the covers it's wrapping using the request module. That means I need to check res.statusCode and body in my callback.

It would be great if the library instead gave me just what I want - the response body JSON or a structured error like this

api.positions((err, json) => {
  // just provided json and error (if defined), no need to check res.statusCode since 
  // the library verifies it is a 200 for me and just gives me json
})

This would work great with TypeScript since each response type could be defined for code completion and documentation and you'd also be able to declare robinhood types elsewhere as arguments throughout your code, like so:

function doSomethingWithPositions (p: RobinhoodPositions) {
  // I now have intellisense for p and know its structure
}

Promise Support

Having promise support would be great.

return Promise.props([
  positions: api.positions(),
  user: api.user()
])
  .then(results => {
    // results.user
    // results.positions
  })
  .catch(e => { // catch all errors })

Along with the usual Promise benefits, it would allow using async/await for cleaner code in newer node versions:

try {
  const positions = await api.positions()
  const user = await api.user()
} catch (e) {
  // catch all errors
}

Refined Error Checks

Currently all error handling is deferred to the developer, i.e I, as a developer, always need to check status codes but the library could easily do this:

api.positions((err, res, positions) => {
  if (err) {
    // Check error type, handle appropriately
    console.log('error making https call for permissions. could be a connectivity issue')
    console.log(err)
  } else if (res.statusCode !== 200) {
    // Handle status code type
    console.log('error response returned from robinhood')
    console.log(res.statusCode)
    console.log(body)
  } else {
    // Awesome, it worked
    console.log(positions)
  }
})

It would be great instead if the library checked the status code for me:

api.positions((err, positions) => {
  if (err) {
    // something happened, either non 200 status or connectivity issue
  } else {
      console.log(positions)
  }
})

We could extend it so specific error classes are used so developers can identify errors easily:

    if (err instanceof robinhood.RestApiError) {
      // used when the API returns non 200 status or unexpected response format
      // the error could have "res" from request attached if the developer need more info
    } else if (err instanceof robinhood.CommunicationError)  {
      // passed when "err" param in request callback was non null
      // the original error could be attached too for extra info
    } else {
      // generic/unknown error handling
    }

[aurbano]

This is a really good idea!

If you or someone else wants to start working on a proof of concept for it let me know, otherwise I'll give it a go when work gives me some free time 😄

[aurbano]

Let's continue this in #105, I'll implement the changes to the library this week and then we can discuss them and refine.