Problem with variable scoping in Python

Question

This problem is partly due to my lack of completely understanding scoping in python, so I'll need to review that. Either way, here is a seriously trivial piece of code that keeps crashing on my Django test app.

Here's a snippet:

@login_required
def someview(request):
 try:
  usergroup = request.user.groups.all()[0].name
 except:
  HttpResponseRedirect('/accounts/login')

 if 'client' in usergroup:
  stafflist = ProxyUserModel.objects.filter(groups__name='staff')

No brain surgery here, the problem is I get an error such as the following:

 File "/usr/local/django/myapp/views.py", line 18, in someview
   if 'client' in usergroup:

UnboundLocalError: local variable 'usergroup' referenced before assignment

My question here is, why is usergroup unbound? If it's unbound, that means the try statement had an exception thrown at which point an HttpResponseRedirect should happen, but it never does. Instead I get an HTTP 500 error back, which is slightly confusing.

Yes I can write smarter code and ensure that the user logging in definitely has a group associated with them. But this isn't a production app, I'm just trying to understand / learn Python/Django. Why exactly is the above happening when a user that's not associated with a group logs in instead of a redirect to a login page?

In this case I'm intentionally logging in as a user that isn't part of a group. That means that the above code should throw an IndexError exception like the following:

>>> somelist = []
>>> print somelist[0]
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
IndexError: list index out of range

Answer 1

HttpResponseRedirect('/accounts/login')

You're creating it but not returning it. Flow continues to the next line, which references usergroup despite it never having been assigned due to the exception.

The except is also troublesome. In general you should never catch ‘everything’ (except: or except Exception:) as there are lots of odd conditions in there you could be throwing away, making debugging very difficult. Either catch the specific exception subclass that you think is going to happen when the user isn't logged on, or, better, use an if test to see if they're logged on. (It's not really an exceptional condition.)

eg. in Django normally:

if not request.user.is_authenticated():
    return HttpResponseRedirect('/accounts/login')

or if your concern is the user isn't in any groups (making the [0] fail):

groups= request.user.groups.all()
if len(groups)==0:
    return HttpResponseRedirect('/accounts/login')
usergroup= groups[0].name

Answer 2

Try moving you if 'client' part inside you try block. Either that or define usergroup = None right above the try.

Answer 3

In cases where you have a try…except suite and you want code to run iff no exceptions have occurred, it's a good habit to write the code as follows:

try:
    # code that could fail
except Exception1:
    # handle exception1
except Exception2:
    # handle exception2
else: # the code-that-could-fail didn't
    # here runs the code that depends
    # on the success of the try clause