colelloa / django-pagination

Automatically exported from code.google.com/p/django-pagination
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

django-pagination causes 500 error on invalid page #28

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
What steps will reproduce the problem?
1. Install django-pagination as documented
2. Navigate to an invalid page number

What is the expected output? What do you see instead?

If django-pagination is installed according to the documentation, you will
see a 500 error (TemplateSyntaxError).  What you should see instead is,
perhaps, arguable.

In issue 1 the fix chosen was to set an (undocumented) context variable
"invalid_page".  If this is the preferred route, the use of that context
variable needs to be clearly documented in the usage docs, as the standard
setup of django-pagination should not open up a site to 500 errors.

In my opinion, the fix for issue 1 was insufficient, as it's ugly to always
have to include invalid_page-checking boilerplate in any template that uses
django-pagination.  I would prefer to see an invalid page simply return the
first page instead.

Some options I see:

1. Invalid page returns first page.  Very easy to use, idiot-proof. 
Backwards-incompatible

2. Keep current behavior, but add documentation of invalid_page context
variable.  Makes templates using pagination uglier and more complex than
they need to be if you want to avoid 500s.

3. Introduce a setting to choose between 1 and 2. 

What version of the product are you using? On what operating system?

Most recent SVN trunk, with Django 1.0.x branch.

Please provide any additional information below.

I'm happy to write a patch for any of the above solutions, once one of them
is chosen.

Original issue reported on code.google.com by carl.j.meyer on 6 Nov 2008 at 4:27

GoogleCodeExporter commented 8 years ago
just hit the same problem. I remember I get 404 page when I access page out of 
the range. I think 404 page is 
appropriate... 

Original comment by yash...@gmail.com on 7 Nov 2008 at 6:38

GoogleCodeExporter commented 8 years ago
I get this a lot too. It's be nice to be able to handle this error in the 
template context somehow. 

Original comment by npbor...@gmail.com on 7 Nov 2008 at 3:38

GoogleCodeExporter commented 8 years ago
Agreed, I think the solution is to insert an invalid_page value or something 
into the
context, so that it's up to the template author to gracefully handle that and 
not
attempt to use pagination if there is an invalid_page == True.  Of course the 
default
template will handle it correctly as well.

Original comment by flo...@gmail.com on 7 Nov 2008 at 5:59

GoogleCodeExporter commented 8 years ago
The autopaginate template tag already sets the invalid_page context variable.  
The
problem is that the paginate template tag doesn't check for it, and so the 500 
error
occurs in that function (when trying to access paginator and page_obj) before 
you
ever get into the pagination template.

The problem with the invalid_page solution is that it leaves you with only one
realistic option of how to "gracefully handle" an invalid page: display an error
message.  Options like using page 1 as the default (as I prefer) or raising a 
404 are
not possible to do from a template without ugly hackery.  Both of these would be
trivial to do in code within the autopaginate tag, if either a setting or 
arguments
to the template tag were available to select the preferred option.

Original comment by carl.j.meyer on 7 Nov 2008 at 8:22

GoogleCodeExporter commented 8 years ago

Original comment by flo...@gmail.com on 29 Nov 2008 at 5:46

GoogleCodeExporter commented 8 years ago
Fixed in r45

Original comment by flo...@gmail.com on 4 Dec 2008 at 9:49

GoogleCodeExporter commented 8 years ago
I believe that raising an exception on the template tag render method is against
Django policy:

" (..) render() should never raise TemplateSyntaxError or any other exception. 
It
should fail silently, just as template filters should. "

http://docs.djangoproject.com/en/dev/howto/custom-template-tags/#writing-the-ren
derer

I suggest deprecating the PAGINATION_INVALID_PAGE_RAISES_404 option. The 
autopaginate
template fails silently, but sets a flag on the request indicating the problem. 
After
the template is rendered, the pagination middleware checks this flag and 
decides what
to do. Enter the PAGINATION_INVALID_PAGE_RESPONSE option.

 * If PAGINATION_INVALID_PAGE_RESPONSE = '404', the middleware returns a 404 page.
 * If PAGINATION_INVALID_PAGE_RESPONSE = '302', it returns a 302 redirection to the
same view, first page.
 * Otherwise it returns the rendered template.

I've attached a small patch for SVN revision r50 with this logic.

Original comment by rvio...@gmail.com on 29 Nov 2009 at 1:53

Attachments: