Misleading Exception handling in GraphDatabase.__init__() #65

Closed
doismellburning opened this Issue Jun 1, 2012 · 5 comments

Projects

None yet

2 participants

Contributor

At https://github.com/versae/neo4j-rest-client/blob/master/neo4jrestclient/client.py#L80 is the following code:

    try:
        response, content = Request(**self._auth).get(url)
    except Exception:
        raise NotFoundError(result="Unable get root")

When I changed that raise to raise NotFoundError(result="Unable get root (caused by: %s)" % e) I found that in fact, it was caused by a 401, which the NotFoundError obfuscates:

NotFoundError: Error [404]: Not Found. Nothing matches the given URI.
Unable get root (caused by: Error [401]: Unauthorized. No permission -- see authorization schemes.
Authorization Required)`

Now subsequent code handles response.status not being 200, so I'm not sure of what the except was expected to handle. I can see a number of ways of solving this; it is after all not much more than a cosmetic issue, but I'm not sure I see the best way to do so.

Owner
versae commented Jun 1, 2012

I see, maybe it's a better option there to use StatusException instead of NotFoundError. Please, try StatusException and tell me if it makes more sense:

    try:
        response, content = Request(**self._auth).get(url)
    except Exception:
        raise StatusException(result="Unable get root")
@versae versae was assigned Jun 1, 2012
Contributor

It already is a StatusException that's being raised though...

Owner
versae commented Jun 1, 2012

Ouch! You are right. Better removing the try except block and leaving just the

    response, content = Request(**self._auth).get(url)
Contributor

#66 :)

Owner
versae commented Jun 1, 2012

Thanks!

@versae versae closed this Jun 1, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment