Catching user entered RecordNotFound errors about ohloh-ui HOT 3 CLOSED

The second wouldn't solve the problem. All requests have a controller on the stack. We want to filter the not found exceptions by where the parameter came from (our code vs user's url param). It can only be handled at the call site.

The first is closer to what I'd suggest. I'd probably have it as an instance method on ApplicationController as that enforces the notion that it is only to be used at the controller level for user params. Something like:

def find_model_by_user_param(model, id)
  model.find(id)
rescue ActiveRecord::RecordNotFound
   raise ParamRecordNotFound
end

from ohloh-ui.

The regexp in the second solution includes the pattern find_ to differentiate between the two types of RecordNotFound. For example, if you do a Link.find(-1) inside links_controllers's edit action, the backtrace would not contain find_. The only line from ohloh-ui's source would be ...ohloh-ui/app/controllers/links_controller.rb:26:in 'edit'.
This solution is assumes that all user entered params are read inside methods like find_model or set_model.

from ohloh-ui.

Thank you @notalex for raising the issue, which is at the heart of the ResourcesController conversation we had earlier this week. The reason we wanted to differentiate between those RecordNotFound errors raised by parameter values and those raised by the remaining processing is that our monitoring solutions, New Relic and AppNeta, report the RecordNotFound exception raised by users requesting non-existent accounts at the same level as internal processing exceptions. While it is true that statements like Account.find(params[:id]) raise ActiveRecord::RecordNotFound exceptions, we have decided that this is not an error state; that is not an error caused by a code defect.

@BoboFraggins also proposed a solution with the Resources approach, which took the approach of encapsulating this behavior in a base class from which all controllers that need this behavior can derive.

We talked about the possibility of not deriving from such a common base class for those controllers that may not need this behavior as a solution for addressing the concern that every controller would otherwise have this behavior. One would like to observe that the make_resourceful plugin in Ohloh current forces every controller to have the set of resourceful methods, so it may not be critical to protect ourselves from that particular scenario.

We have three proposals that can be categorized as:

1. Replace make_resourceful with an inheritance model
2. Call out specifically when we want to mask ActiveRecord::RecordNotFound exceptions with our own exception
3. Detect a Controller in the backtrace of exceptions and behave differently

The author had the same observation as Peter Fry about option 3; every call stack would have a controller in the stack.

The problem is that we see too many exception reports for ActiveRecord::RecordNotFound for Accounts. While the proposals described above may indeed each solve that problem, there are other options, such as simply not making an ActiveRecord call that raises an error. For example, Account.find_by_login(params[:id]) if params[:id].to_i > 0 would also solve the problem with no additional overhead.

This approach (let's call it "Proposal 4") introduces other questions, such as "do we want a mechanism to automatically find and load resources?". One feature of the make_resourceful plugin was that one could always use @current_objects and @current_object instance variables. The author also prefers meaningful variable names so that the AccountsController has variables @accounts and @account, but this is achievable through some reflective metaprogramming to identify the current controller and set the variable name appropriately.

If we are going to consider the overhead of explicitly making <class_name>.find calls for index, show, update, and delete actions, then why don't we simply use finder forms that won't raise exceptions when we don't want to track them. If we want the convenience of having automatic object retrieval, then we will have to accept the additional complexity of a module or inheritance model solution.

For the record, the authors preferences are:

• Minimize code duplication (DRY)
• Maximize readability and maintainability (meaningful variable names)
• Support flexibility (DRY, but not exclusively, dictatorially so)
• Make the solution as simple as possible, but no simpler

from ohloh-ui.