pagy: Raise OutOfRangeError for page input below min

OutOfRangeError works great when page input is larger than max page (page=11 when max is 10). But when setting page input to 0 (as will happen when input is not a number) it results in ArgumentError when calling pagy() which is not so easy to work with.

page=0 => ArgumentError: expected :page >= 1; got 0

About this issue

  • Original URL
  • State: closed
  • Created 6 years ago
  • Comments: 22 (18 by maintainers)

Most upvoted comments

@espen, @workgena FYI: from version 3.3.1 all the exceptions are Pagy:: namespaced (i.e no more generic ArgumentError exceptions). They are all sub-classes of Pagy::VariableError (included the Pagy::OverfolowError) and since the Pagy::VariableError is a sub-class of ArgumentError it will not break any legacy rescues.

The Pagy::VariableError provides also the variable symbol of the offending variable and its value as accessors of the exception object.

IMO that’s not a dramatic improvement, but it will be simpler to rescue even very specific errors, even for the most pointless reasons 😃

HTH

I read all the discussion, and, actually, don’t think it is a problem.

But, I do understand @espen 's point of view(had similar issue with “Postgres integer out of range”). ArgumentError is not nice. If pagy in this case provides the Pagy::ArgumentError, very few programmers will notice and appreciate it. Rare situation.

Because, if user somehow managed to enter “foo”, it seems that he wants to brake the logic and not to receive real data. So this case may be ignored by the programmer. Unless it is requirement of a customer.

If you care about serializing user input for params[:page], it is easy to do with this example:

@pagy, @collection = pagy(scope, page: (params[:page].to_i rescue 1))

This is “bullet proof” handling of user input, including floating numbers.

Such example may be mentioned in “FAQ”, and it will be enough. Is such solution OK for you project, @espen ?

Both API clients and user behaviour can either result in invalid input due to bugs or malicious behaviour.

Thanks for this, because you save me the time to answer you point by point. As you just wrote, the whole discussion is focused on the only 2 possible cause of that kind of error: bugs and malicious behaviour, and both of them are VERY worth an exception.

  • Bugs: please, do raise exceptions to give someone the possibility to fix them. Regardless whether bugs happen inside the app or in an API client, the developers should be able to get an exception, and that could help fixing even bigger problems, discovered (and not ignored) thanks to the exception. I am tired to scratch my head when I get unexpected results and no exceptions and no log entries in the legacy apps I am working on, because some smart-ass implemented a rescue do_something_else somewhere!!!

  • Malicious behavior: in this case if I could, I would even “spit in the eye” of the wannabe hacker (very Italian way to despise someone 😃), but since it is quite difficult to do so over a connection, the very next thing is giving the guy an exception to suck!

I think that should be handled by the gem due to arguments listed above.

The gem offers the overflow extra that provides a few pre-defined ways to handle OverflowError exceptions, which - as explained - are neither bugs nor argument errors, but it does raise the exceptions in all the other cases.