Django message framework called with None

[merb]

Currently, this is a very small bug and i don't now the first occurance, since i first used the message framework at the last evening.

I used Django 1.6 and installed everything. I put the Message Framework into the middleware, the context preprocessor and the INSTALLED APPS.
And then I did something which happend, cause i was very sleepy.

I made a generic.FormView like this:

class CreateUserView(generic.FormView):
    template_name = 'envisia_backup/create_user.html'
    form_class = UserForm

    def get_success_url(self):
        return reverse('envisia:index')

    def form_valid(self, form):
        data = form.cleaned_data
        user = User.objects.create(**data)
        user.is_staff = True
        user.is_superuser = True
        user.save()
        messages.success(request, 'User erfolgreich erstellt!')
        return HttpResponseRedirect(self.get_success_url())

Since I was sleepy I really didn't know where the mistake was.
The error message always was that the message framework wasn't installed and I should put it in the middleware and in the INSTALLED_APPS.

Why did this happen? Currently it happend cause I called messages.success with None.
request wasn't defined. so it was None. It took a while since I found the error since I never got a correct error message.

Currently the fix would be really simple. But I don't want to make a patch until know since I don't know if it is wished that the messages framework should make a assert if we call it with a Request object or not.
If it is wished that the message framework could only be called with a request object, than i would make a patch with a corresponding test. But only if this is really wished.

[Baptiste Mispelon]

Hi,

The code you showed would actually raise a NameError (it's the error python raises when you use a variable without defining it first).

The code is wrong but the behavior you describe is actually correct: passing anything but a request object (actually, anything without a _messages attribute) to add_message raises an error telling you to install the message middleware.

I can see this being confusing, so if you have ideas on how to improve the situation, please do share them.

Thanks.

[merb]

Thats correct, normally it would raise NameError, but it didn't raised the NameError so it always raised that the middleware was not installed.
I was very sleepy thats why i found out, that something is really wrong.

I would make assertion checks on

messages.add_message
messages.debug
messages.info
messages.success
messages.warning
messages.error

but that would raise another problem, what happens if somebody makes a custom Request that inerhits from the actual request object, would this work with a assertion check, too?

[Claude Paroz]

If this didn't raise NameError, it is probably because request was defined somewhere in your file. If we can find a rather common possible use case where request is None, then it might be worth asserting request is not None in add_message.

[Denis Cornehl]

will work on a patch for this

[Baptiste Mispelon]

Looks good. I'll make sure the test suite passes and commit it.

[Baptiste Mispelon]

Fixed in 1e970584178c44d561d54dc8bf0c3837b89bad5a.

[Raffaele Salmaso]

Hi,
I'm using django-rest-framework Request object, and this patch make impossible to use it as replacement: I must use request._request, which is a bit ugly.

Would it be better as

def add_message(request, level, message, extra_tags='', fail_silently=False):
    """
    Attempts to add a message to the request using the 'messages' app.
    """
    if request and hasattr(request, '_messages'):
        return request._messages.add(level, message, extra_tags)

    if not fail_silently:
        from django.conf import settings
        if 'django.contrib.messages.middleware.MessageMiddleware' not in settings.MIDDLEWARE:
            raise MessageFailure(
                'You cannot add messages without installing '
                'django.contrib.messages.middleware.MessageMiddleware'
            )
    raise TypeError("add_message() argument must be an HttpRequest like object, "
                    "not '%s'." % request.__class__.__name__)

?

[Tim Graham]

Perhaps. Generally we prefer ducktyping over isinstance.

[Chris Portela]

I am also using Django Rest Framework and came here to second Raffaele's proposal here.

[Tim Graham]

Please open a new ticket rather than commenting on a closed one. Thanks.