"Nil object": really?

Hello,-

I am developing a rails app with this method in it:

# PUT /newsletters/1;send
  def sendmails
    newsletter = Newsletter.find_by_id_and_sent(params[:id], false)
    users = User.find(:all)
    raise users.to_yaml
    #debugger
    users.each do |user| #evaluates to nil.each - debug
      Notifier.deliver_newsletter(user, newsletter)
    end
    newsletter.update_attribute('sent', true)
    redirect_to newsletters_path
  end

I get an error saying that users.each do |user| evaluates to nil.each,
and that I probably expected an Array object there. This is true, I did
expect an Array object. Thing is, now I've tried to debug the code with
both debugger and raise to_yaml, and both times the users variable
evaluates to a perfect Array of 3 Users, exactly in correspondence with
the DB! I have also tried to debug the user variable and it evaluates to
a User with id 1, meaning the iterator fetches that user successfully
from the Array.

Yet after the first iteration, I get an error / crash.

I have tried to change users to @users, with the exact same result.
Debugging shows an Array of 3 user objects. The app crashes.

What is going on here?

Thanks in advance, Vahagn

Are you sure that the error message is on that each, not another one ?
What's the stack trace like?

Fred

Frederick Cheung wrote:

Hello,-

Yet after the first iteration, I get an error / crash.

I have tried to change users to @users, with the exact same result.
Debugging shows an Array of 3 user objects. The app crashes.

Are you sure that the error message is on that each, not another one ?
What's the stack trace like?

Fred

Hi Fred,-

here's the stack trace. Whatever it is that is causing the problem, I
cannot glean it from there:

app/controllers/newsletters_controller.rb:60:in `sendmails'
/Library/Ruby/Gems/1.8/gems/actionpack-2.0.2/lib/action_controller/base.rb:1158:in
`send'
/Library/Ruby/Gems/1.8/gems/actionpack-2.0.2/lib/action_controller/base.rb:1158:in
`perform_action_without_filters'
/Library/Ruby/Gems/1.8/gems/actionpack-2.0.2/lib/action_controller/filters.rb:697:in
`call_filters'
/Library/Ruby/Gems/1.8/gems/actionpack-2.0.2/lib/action_controller/filters.rb:689:in
`perform_action_without_benchmark'
/Library/Ruby/Gems/1.8/gems/actionpack-2.0.2/lib/action_controller/benchmarking.rb:68:in
`perform_action_without_rescue'
/Library/Ruby/Gems/1.8/gems/actionpack-2.0.2/lib/action_controller/benchmarking.rb:68:in
`perform_action_without_rescue'
/Library/Ruby/Gems/1.8/gems/actionpack-2.0.2/lib/action_controller/rescue.rb:199:in
`perform_action_without_caching'
/Library/Ruby/Gems/1.8/gems/actionpack-2.0.2/lib/action_controller/caching.rb:678:in
`perform_action'
/Library/Ruby/Gems/1.8/gems/activerecord-2.0.2/lib/active_record/connection_adapters/abstract/query_cache.rb:33:in
`cache'
/Library/Ruby/Gems/1.8/gems/activerecord-2.0.2/lib/active_record/query_cache.rb:8:in
`cache'
/Library/Ruby/Gems/1.8/gems/actionpack-2.0.2/lib/action_controller/caching.rb:677:in
`perform_action'
/Library/Ruby/Gems/1.8/gems/actionpack-2.0.2/lib/action_controller/base.rb:524:in
`send'
/Library/Ruby/Gems/1.8/gems/actionpack-2.0.2/lib/action_controller/base.rb:524:in
`process_without_filters'
/Library/Ruby/Gems/1.8/gems/actionpack-2.0.2/lib/action_controller/filters.rb:685:in
`process_without_session_management_support'
/Library/Ruby/Gems/1.8/gems/actionpack-2.0.2/lib/action_controller/session_management.rb:123:in
`process'
/Library/Ruby/Gems/1.8/gems/actionpack-2.0.2/lib/action_controller/base.rb:388:in
`process'
/Library/Ruby/Gems/1.8/gems/activesupport-2.0.2/lib/active_support/dependencies.rb:489:in
`load'
/Library/Ruby/Gems/1.8/gems/activesupport-2.0.2/lib/active_support/dependencies.rb:489:in
`load'
/Library/Ruby/Gems/1.8/gems/activesupport-2.0.2/lib/active_support/dependencies.rb:342:in
`new_constants_in'
/Library/Ruby/Gems/1.8/gems/activesupport-2.0.2/lib/active_support/dependencies.rb:489:in
`load'
/Library/Ruby/Gems/1.8/gems/rails-2.0.2/lib/commands/servers/mongrel.rb:64
/Library/Ruby/Gems/1.8/gems/activesupport-2.0.2/lib/active_support/dependencies.rb:496:in
`require'
/Library/Ruby/Gems/1.8/gems/activesupport-2.0.2/lib/active_support/dependencies.rb:342:in
`new_constants_in'
/Library/Ruby/Gems/1.8/gems/activesupport-2.0.2/lib/active_support/dependencies.rb:496:in
`require'
/Library/Ruby/Gems/1.8/gems/rails-2.0.2/lib/commands/server.rb:39

/ V.

And here's the output of raise users.to_yaml:

Vahagn Hayrapetyan wrote:

    users.each do |user| #evaluates to nil.each - debug

In general, you need to cut your action up into tiny methods and write unit tests on each one. They incredibly reduce the odds of exactly this kind of drama! Then the methods should run in the model layer, and the controller should only call into the model.

Specifically, check if 'users' is aliased anywhere else, such as to a method. Then try this:

  (users || []).each

That will force a nil users to behave like an empty array. .find(:all) should naturally return an empty array if you have no users, so you should not need this tweak. But it lets you manually debug more:

   (users || (p 'still nil!'; []).each

Hi Phlip, I appreciate the feedback. Firstly, I renamed users to
all_users and the error is still there; therefore, no aliasing occurs.

Secondly, I've tried as you wrote:

  (all_users || (p 'still nil!'; []).each
      Notifier.deliver_newsletter(user, newsletter)
   end

but that gives a syntax error ("Unexpected kEND"). I'm new to Ruby!

Lastly, do you mean to say that this method is best moved to the
Newsletter model instead of being in the controller?

/ Vahagn

Hello Vahagn,
I suggest you do it as follows:
a) old-fashioned way by using puts liberally &
b) take baby steps
until it is debugged.

Your code with puts statements inserted; just an example:

  def sendmails
    newsletter = Newsletter.find_by_id_and_sent(params[:id], false)
    users = User.find(:all)
    # do not raise anything... confuses Ruby interpretor
    puts users
    users.each do |user|
      puts user
      #Notifier.deliver_newsletter(user, newsletter)
    end
    #newsletter.update_attribute('sent', true)
    #redirect_to newsletters_path
  end

Make sure first that the each iterator is printing your object instances
correctly. You can use "pp" short for pretty-print if you want but
then you will have to require it. puts is built into the language and
most of the times, it is sufficient.

This way, you can be absolutely sure that nothing unexpected is
happening with your basic loop. Once you are sure then you can
uncomment each statement one by one, e.g., uncomment Notifier.... first
and test. If that works then go to newslettr.update...

Hope this helps.

Bharat

Thanks Bharat, that was excellent advice. It turns out the real trouble
is being caused by this line:

Notifier.deliver_newsletter(user, newsletter)

Now, the stack trace looks like this:

NameError in Newsletters#sendmails

Showing notifier/newsletter.text.plain.rhtml where line #3 raised:

undefined local variable or method `user' for
#<ActionView::Base:0x23df2a0>

Extracted source (around line #3):

1: My Newsletter
2:
3: <%= render :inline => @body %>

I have a Notifier class that looks like this:

class Notifier < ActionMailer::ARMailer
  def newsletter(user, newsletter)
    recipients user.email
    from "MyApp <system@app.net>"
    subject newsletter.subject
    body :body => newsletter.body, :user => user
  end
end

...and this is the file the stack trace refers to:

/app/views/notifier/newsletter.text.plain.rhtml:

My Newsletter

<%= render :inline => @body %>

Still - user is not being referenced from this file at all, so why the
error?

Thanks again,
/ Vahagn

Glad to be of help. I am no expert on ActiveRecord Mailer, but when I
run into problems like this, I turn to:
1. Agile Web Development book -- Excellent discussion on this topic
2. Rails Way by Obie Fernandez -- Anyone doing any serious
development should have this book
I am sure you can figure this out by a bit of reading and research.

Hi Bharat,-

there's hardly a Rails book that is not near the top of my Amazon wish
list - so you're right on :wink:

Cheers,
Vahagn

Bharat Ruparel wrote: