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. :)

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

On Thu, Jun 23, 2016 at 10:37 AM Robert Smith <
mer...@sleepingkingstudios.com> wrote:

> 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.
>
>
> On Tuesday, June 21, 2016 at 3:28:19 PM UTC-4, Al Tenhundfeld wrote:
>
>> 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
>>
> On Tue, Jun 21, 2016 at 3:17 PM, John Mileham <jmil...@gmail.com> wrote:
>>
> 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
>>>
>> On Tue, 21 Jun 2016 at 15:04 Robert Smith <mer...@sleepingkingstudios.com>
>>> wrote:
>>>
>> 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.
>>>>
>>>> On Tuesday, June 21, 2016 at 10:15:04 AM UTC-4, Saiqul Haq wrote:
>>>>>
>>>>> 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:
>>>>>>
>>>>>> 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
>>>>>>
>>>>> --
>>>> You received this message because you are subscribed to the Google
>>>> Groups "Ruby on Rails: Core" group.
>>>>
>>> To unsubscribe from this group and stop receiving emails from it, send
>>>> an email to rubyonrails-co...@googlegroups.com.
>>>> To post to this group, send email to rubyonra...@googlegroups.com.
>>>
>>>
>>>> Visit this group at https://groups.google.com/group/rubyonrails-core.
>>>> For more options, visit https://groups.google.com/d/optout.
>>>>
>>> --
>>> You received this message because you are subscribed to the Google
>>> Groups "Ruby on Rails: Core" group.
>>>
>> To unsubscribe from this group and stop receiving emails from it, send an
>>> email to rubyonrails-co...@googlegroups.com.
>>> To post to this group, send email to rubyonra...@googlegroups.com.
>>>
>>
>>> Visit this group at https://groups.google.com/group/rubyonrails-core.
>>> For more options, visit https://groups.google.com/d/optout.
>>>
>> --
> You received this message because you are subscribed to the Google Groups
> "Ruby on Rails: Core" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to rubyonrails-core+unsubscr...@googlegroups.com.
> To post to this group, send email to rubyonrails-core@googlegroups.com.
> Visit this group at https://groups.google.com/group/rubyonrails-core.
> For more options, visit https://groups.google.com/d/optout.
>

-- 
You received this message because you are subscribed to the Google Groups "Ruby 
on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to rubyonrails-core+unsubscr...@googlegroups.com.
To post to this group, send email to rubyonrails-core@googlegroups.com.
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.

Reply via email to