views:

325

answers:

4

I'm testing out a piece of code to ping a bunch of websites I own on a regular basis, to make sure they're up.

I'm using rails and so far I have this hideous test action that I'm using to try it out (see below).
The problem though, is that sometimes it works, and other times it won't ... sometimes it runs through the code just fine, other times, it seems to completely ignore the begin/rescue block ...

a. I need help figuring out what the problem is b. And refactoring this to make it look respectable.

Your help is much appreciated.

edit 1: Here is the updated code, sorry it took so long, pastie.org was down since yesterday http://pastie.org/927201

Its still doing the same thing ... skipping the begin block (because it only updates up_check_time) ... however if one of the sites times out, it actually updates everything (check_msg, code etc) correctly ... confusing, yeah?

require 'net/http'
require 'uri'

def ping
    @sites = NewsSource.all

    @sites.each do |site|
        if site.uri and !site.uri.empty?
            uri = URI.parse(site.uri)
            response = nil
            path = uri.path.blank? ? '/' : uri.path
            path = uri.query.blank? ? path : "#{path}?#{uri.query}"

            begin
                Net::HTTP.start(uri.host, uri.port) {|http|
                http.open_timeout = 30
                http.read_timeout = 30
                response = http.head(path)
                }

                if response.code.eql?('200') or response.code.eql?('301') or response.code.eql?('302')
                site.up = true
                else
                site.up = false
                end

                site.up_check_msg = response.message
                site.up_check_code = response.code
            rescue Errno::EBADF
            rescue Timeout::Error
                site.up = false
                site.up_check_msg = 'timeout'
                site.up_check_code = '408'
            end
            site.up_check_time = 0.seconds.ago
            site.save
        end
    end
end
+1  A: 

Here's a snippet from one of my programs, maybe it helps:

urls.each_with_index do |url, idx|
  print "Processing URL #%04d: " % (idx+1)
  uri = URI.parse(url)
  response = nil

  begin
    Net::HTTP.start(uri.host, uri.port) do |http|
      response = http.head(uri.path.size > 0 ? uri.path : "/")
    end
  rescue => e
    puts "#{e.message} - #{url}"
    next
  end

  # handle redirects
  if response.is_a?(Net::HTTPRedirection)
    new_uri = URI.parse(response['location'])
    puts "URI redirects to #{new_uri}"
    next
  end

  puts case response.code
    when '200' then ...
    when '404' then ...
    else ...
  end
end
Michael Kohl
+3  A: 

You currently have an empty rescue block for Errno::EBADF so if that exception is raised then you will not be setting site.up to false.

Also, a couple of other minor improvements:

Instead of if site.uri and !site.uri.empty? you can use:

next if site.uri.nil? or site.uri.empty?

to skip that iteration of the each loop and avoid indenting the code by an additional level.

And:

if response.code.eql?('200') or response.code.eql?('301') or response.code.eql?('302')
  site.up = true
else
  site.up = false
end

can be written more concisely:

site.up = ['200', '301', '302'].include? response.code

If you tidy up the code with some of these tips then it might help narrow down the problem.

mikej
NICE!!!Made the edits ... will repost in a bit, but I had one more question? Is it possible to rescue multiple exceptions at once ... thats what I was trying to do (clumsily) with the rescue blocks ...
concept47
Yes, I thought you might have been trying to rescue multiple exceptions at once. I just wasn't sure. To do it separate them with a comma e.g. `rescue Errno::EBADF, Timeout::Error`
mikej
Thanks Mike, I figured it out yesterday by looking at how Ruby case statements did it, but thanks anyway :D. Here is the updated code ... http://pastie.org/927201. Its still doing the same thing ... skipping the begin block (I say that because it generally only updates up_check_time) ... however if one of the sites times out, it actually updates everything (check_msg, code etc) correctly ... confusing, yeah?
concept47
A: 

The only thing that I can think of is that you are getting some other exception in your begin block. Since you are only explicitly rescuing Errno::EBADF, Timeout::Error it would appear that your begin and rescue got skipped. You might be able to verify this by getting rid of Errno::EBADF, Timeout::Error and just having a plain rescue, then put the following in your rescue block

logger.info(">>Exception was: "+$!)

Then look in your logs to see what exceptions you are getting.

mpd
A: 

If you are monitoring your servers why not use Nagios? it's free and also has some Ruby support, Here and Here.

EDIT:

Ruby GEM: http://hobodave.com/2010/01/10/simple-nagios-probes-in-ruby/

Phill Pafford
won't work because I need it to do this from within the rails app itself
concept47
The Ruby Links didn't offer this?
Phill Pafford
I'm not monitoring servers, I'm trying to see if external websites are up using a head command. This solution does nothing for me as far as I can see.
concept47