code to ping websites works sometimes

Question

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

Answer 1

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.