Proposal: Exposing the API Error text in exceptions

[bdowling]

Throughout the API we use the requests.raise_for_status, however this often hides the API usage hints that Robinhood returns in the content of the error message which makes it harder to troubleshoot. (I'm certain more than a few of us have been confounded by this at times)

I experimented with a few options. One of them being the exception chaining, but that just seemed clumsy, resulted in useless extra traceback and is also only py3 compatible.

I finally ended up with this:

    def raiseForStatus(self,res):
        """Helper for requests.raise_for_status that returns the content as well

            The Robinhood API often returns useful hints about what params are
            out of bounds.  This wrapper helps expose those to the client
            """
        try:
            res.raise_for_status()
        except requests.exceptions.HTTPError as error:
            # HTTPError expected to use args, if it isn't this needs fixing anyway
            if error.args is not None:
                args = error.args
                if error.response.text:
                    apitext = "API Returned: " + error.response.text
                    error.args = args, apitext
            raise

Which results in error messages such as (last line being the addition):

>>> rh.get_historical_quotes('AAPL', interval='day', span='week')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/~/src/Robinhood/Robinhood/Robinhood.py", line 474, in get_historical_quotes
    self.raiseForStatus(res)
  File "/~/src/Robinhood/Robinhood/Robinhood.py", line 121, in raiseForStatus
    res.raise_for_status()
  File "/~/src/Robinhood/py3/lib/python3.4/site-packages/requests/models.py", line 935, in raise_for_status
    raise HTTPError(http_error_msg, response=self)
requests.exceptions.HTTPError: (('400 Client Error: Bad Request for url: https://api.robinhood.com/quotes/historicals/?symbols=AAPL&bounds=regular&interval=day&span=week',), 
'API Returned: {"span":["\\\\\\"week\\\\\\" is not a valid choice."]}')

Essentially all calls to res.raise_for_status() would be replaced with:

    self.raiseForStatus(res)

Always more than one way to skin it, but I figured this was a reasonable approach. Let me know your thoughts.

[lockefox]

would robust logging do better for this kind of debugging? By creating a named logger for the library, we could add additional debug information along with the trace.

try:
    res = request.get(ADDRESS, params=params)
    res.raise_for_status()
except Exception:
    logger.error('failed GET: %s', ADDRESS, exc_info=True)
    logger.error(res.json())
    raise 

And if users need better info, they could use logging.getLogger('Robinhood') to subscribe to issues.

https://docs.python.org/3.6/howto/logging.html#advanced-logging-tutorial

[Jamonek]

@bdowling I agree we need to expose the error message. I like your solution and the logging suggestion @lockefox has provided

[jasonleehodges]

Looks like this was fixed in PR #62.