find_by inconsistent return message

User.find(1) for no record with gives:

def test_error_message

begin

User.find(1)

rescue ActiveRecord::RecordNotFound => e

message = e.message end

assert_equal message, “Couldn’t find User with ‘id’=1”

end

I expected same behavior wiht find_by! method but it doesn’t give so descriptive message.

For example expected behavior should be:

def test_error_message_for_find_by!

begin

User.find_by(first_name: ‘foo’, last_name: ‘bar’)

rescue ActiveRecord::RecordNotFound => e

message = e.message end

assert_equal message, “Couldn’t find User with ‘first_name’=‘foo’ and ‘last_name’=‘bar’”

end

but it doesn’t work like this instead e.message is just general “Couldn’t find User”.

My use case is that I have API and on application_controller level I defined rescue from:

ApplicationController < ActionController::Base

rescue_from ActiveRecord::RecordNotFound, with: :record_not_found

def record_not_found(exception)

render json: { errors: [exception.message] }, status: 404

end

end

But in first case clients of API could guess what’s error. But on second example couldn’t .

If maintainers agree I can submit pull request to change behavior.

I think that sounds reasonable. Please submit a PR.

What about the security ramifications of logging potentially sensitive information? IDs can be sensitive, but often far less than other fields. There are legitimate use cases for using find_by on values that you’d prefer not to be logged or known by the end user. Making more-informative error presentation code the responsibility of the application would seem like a good way for the developer to vet their own code and decide what level of logging or return value would be appropriate.

-john

See also discussion on this PR:

https://github.com/rails/rails/pull/15791

and the related issue:

https://github.com/rails/rails/issues/15792

—Matt Jones

John Mileham wrote:

What about the security ramifications of logging potentially sensitive information? IDs can be sensitive, but often far less than other fields. There are legitimate use cases for using find_by on values that you’d prefer not to be logged or known by the end user.

Sounds like we should extend filter_parameters to the generation of this message.