Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I modified Amit Agarwal's Script which checks the uptime of one website so that it now checks several sites. The script runs every 5 minutes, however, I kept receiving mails that a site was down and then 5 mins later that it was up again.

I adapted my code to only log a change if it persisted for two checks, like this:

function siteUptimeCheck(url, row) {
    var response, error;
    var status = "status"+row.toString();         // status1, status2..
    if (!ScriptProperties.getProperty(status))
      ScriptProperties.setProperty(status, 200);
    try {                                         // try, catch, else
        var success = true;
        response = UrlFetchApp.fetch(url);
    } catch(error) {
        success = false;    
        insertData(status, url, row, error, response.getResponseCode(), "Site down: ");
        return;
    } if(success) {
        insertData(status, url, row, "Up", response.getResponseCode(), "Site up: ");
    }
}

function insertData(status, url, row, error, code, msg) {
      if (ScriptProperties.getProperty(status) == code) {    
          return;                                  // Return if there are no changes
      } else if (ScriptProperties.getProperty(status) == 1) {
          sheet.getRange(row,2).setValue(code);
          if (code != 200) {
            //create newdate log entry
            sheet.getRange(row,3).setValue(newdate +" "+ error);
          }
          ScriptProperties.setProperty(status, code);
          MailApp.sendEmail(email, msg, error);
     } else {
          ScriptProperties.setProperty(status, 1);  // Marker for "something changed"
  }
}

I'm fairly new to programming, are there better ways to solve this?

share|improve this question

2 Answers 2

There is no // try, catch, else in javascript.

Instead I would do:

var success = false, state = null;
try {
    response = UrlFetchApp.fetch(url);                                        
    success = true;
    error = "Up";
    state = "Site up: ";
} catch(ex) {
    success = false;    
    error = ex;
    state = "Site down: ";
}

insertData(status, url, row, error, response.getResponseCode(), state);
share|improve this answer
    
Thanks James, I'll have a look at it in the next days and tick it, if it works for me! Much appreciated! –  Christian Macht Dec 16 '13 at 21:55
  • ScriptProperties has been depreciated and should be replaced with the Properties Service.
  • Functions should have verb-noun names. Thus, siteUptimeCheck would be better named checkSiteUptime.
  • 200 is a magic number and should be given a meaningful variable name. (Preferably a constant, but I don't think GAS has constants.)
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.