[Feature Request] When performing find by multiple ids, return partial results on RecordNotFound error.

When I perform an ActiveRecord::Base.find(…) operation with multiple ids, and there is at least one result that is not found, ActiveRecord raises a RecordNotFound error. The error message lists the requested ids and the number of results requested and found. An example:

class Book < ActiveRecord::Base; end

3.times { Book.create }

Creates books with ids 0, 1, 2

begin

Book.find(0, 1, 5)

rescue ActiveRecord::RecordNotFound => exception

puts exception.message

end

“Couldn’t find all Books with ‘id’: (0, 1, 5) (found 2 results, but was looking for 3)”

``

As a developer, I want to know which results were returned and which were not. In our example, ActiveRecord::FinderMethods#find_some actually does have the found results, which it checks against the expected size. However, the result itself is discarded and only the result size is passed into the error via the #raise_record_not_found_exception! method.

My proposal is to add an additional, optional #result property to the RecordNotFound error, which is set to the result of the query. This allows downstream code to know which records were found (and deduce which records were not found by comparing the found records with the expected ids). For example, this could allow a controller to display a more helpful error message to the end user, or allow a library to wrap the RecordNotFound with additional logic around found/missing records.

I’ve built a quick example branch, see https://github.com/sleepingkingstudios/rails/commit/e11a70ef60bbb60e57a081c6e313580880b4570a.

Any feedback would be appreciated.

Regards,

Rob Smith

I think we can achieve this with ActiveRecord#where method, e.g.:

3.times { Book.create! } #=> which returned 3 records, id = 1, 2, 3

``

then I want to find Book records with ID 1, 2, and 5

so

expected_records = [1,2,5]
books = Book.where id: expected_records

``

then to get to know which results were missing is by

missing_records = expected_records - books.pluck(:id)

``

Pada Selasa, 21 Juni 2016 11.34.23 UTC+7, Robert Smith menulis:

It can be done via ActiveRecord#where, but you lose the semantics of using #find. Under the hood, I suppose ActiveRecord just does a #where and then compares the results, so you’re not technically losing any optimization that way either (at least for the current implementation of #find).

On the other hand, it basically forces you to reimplement the #find method yourself if you want the additional information. I think there’s a valid use case here, but if status quo is the community preference, I’ll graciously concede the point.

In order to make use of the additional information in the exception, as a developer, you’d be delving into exception based control flow (i.e. rescuing the exception and performing conditional logic on the payload). Rails admittedly does this for a couple things (like catching RecordNotFound exceptions and 404ing in web endpoints), but only when there’s big leverage in terms of simplifying common use cases. To pack programmatically readable information into an exception object would encourage developers to think about solving problems that way more generally, which to me seems like it would cause more problems than it solves.

-john

John, I was trying to formulate those same thoughts. Thanks.

I think there’s a reasonable case for enhancing the error message to include which were found (or missing). I don’t see a big downside, and it would assist troubleshooting the unexpected problem.

In addition to the behavior-incentivizing nature of the structured result attribute, I also wonder if attaching the full results to the error could have unintended consequences in security/data leakage, serialization, or memory/GC. If this path were chosen, an array of result ID’s might be a safer choice.

-Al

I’m getting a “lean no” impression from the comments regarding adding the objects, and a “lean yes” for adding the ids to the message and/or as an accessor.

One thing that’s not immediately obvious from the rails-core list is that a lot of the chatter is from people like me who have no actual say. :slight_smile:

But if my vote mattered, I’d lean strongly against any form of programmatically readable value (even an array of IDs) encapsulated in the exception to discourage exception based control flow. There might be debugging value in presenting the missing IDs in the exception message, but that could lead to exceptionally wide messages in some cases.

Given the relative ease of set comparison in ruby and the availability of non-raisey finder methods, if you need programmatic access to what’s missing, Saiqul is definitely putting you on the right track. If your goal is debugging convenience, you’d have to show that the developer time saved (either a little time for a lot of people or a lot of time for a few people) would be worth the cost of carrying even a small amount of additional code in rails core. It’s a pretty high bar at this point.

Often if you get the ear of core team members, they’ll recommend that you prototype your desired feature as a gem, put road miles on it, and check back in if it appears to add significant value and/or generate lots of interest in the community.

-john