Implement additional error handling in util.import_object function #430

Merged
merged 3 commits into from Apr 14, 2013

Projects

None yet

2 participants

@kachayev
Contributor
kachayev commented Jan 6, 2012

Actually tornado.util.import_object can raise several different exceptions (ValueError, IndexError, AttributeError, ImportError) in different cases. This is not so convenient to handle all of them. Of course, there are two ways to improve such behavior:

  1. Standardize exceptions and raise for ex. ImportError or ValueError.
  2. Return None in case of failed operation.

According to usage of this function in framework:

https://github.com/facebook/tornado/blob/master/tornado/httpclient.py#L196

I implemented second approach. DocTest for function is updated so you can find all cases there.

@bdarnell
Member

Returning None instead of raising an exception is a semantic change that needs to be matched by a check on the return value at every call site. I think that in general it's better for import_object to raise an exception than return None. It would be nice if it raised a consistent exception, but not necessary (and your change doesn't guarantee None on a failed import; it could still raise SyntaxError or any other error produced by top-level code in the imported module).

@kachayev
Contributor

So, as far as first approach is preferable, I'll update this branch ASAP (actually I have both implementations).

P.S. Regarding to SyntaxError, I don't see problems here cause SyntaxError can be raised anywhere and we can't protect against it (and we even should not do this).

@kachayev
Contributor

Done. Now we will get standard ImportError for "missing" modules.

@bdarnell bdarnell merged commit 54f56d4 into tornadoweb:master Apr 14, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment