Understanding try..catch in Javascript

Question

I have this try and catch problem. I am trying to redirect to a different page. But sometimes it does and some times it doesnt. I think the problem is in try and catch . can someone help me understand this. Thanks

var pg = new Object();
var da = document.all;
var wo = window.opener;

pg.changeHideReasonID = function(){
 if(pg.hideReasonID.value == 0 && pg.hideReasonID.selectedIndex > 0){
  pg.otherReason.style.backgroundColor = "ffffff";
  pg.otherReason.disabled = 0;
  pg.otherReason.focus();
 } else {
  pg.otherReason.style.backgroundColor = "f5f5f5";
  pg.otherReason.disabled = 1;
 }
}

pg.exit = function(pid){

 try {
  if(window.opener.hideRecordReload){
   window.opener.hideRecordReload(pg.recordID, pg.recordTypeID);

  } else {
   window.opener.pg.hideRecord(pg.recordID, pg.recordTypeID);

  }
 } catch(e) {}
 try {
  window.opener.pg.hideEncounter(pg.recordID);

 } catch(e) {}
 try {
  window.opener.pg.hideRecordResponse(pg.hideReasonID.value == 0 ? pg.otherReason.value : pg.hideReasonID.options[pg.hideReasonID.selectedIndex].text);

 } catch(e) {}
 try {
  window.opener.pg.hideRecord_Response(pg.recordID, pg.recordTypeID);

 } catch(e) {}
 try {
  window.opener.pg.hideRecord_Response(pg.recordID, pg.recordTypeID);

 } catch(e) {}
 try {
  window.opener.window.parent.frames[1].pg.loadQualityMeasureRequest();

 } catch(e) {}
 try {
  window.opener.pg.closeWindow();

 } catch(e) {} 



   parent.loadCenter2({reportName:'redirectedpage',patientID:pid});          
 parent.$.fancybox.close();

}

pg.hideRecord = function(){
      var pid = this.pid;

 pg.otherReason.value = pg.otherReason.value.trim();
 if(pg.hideReasonID.selectedIndex == 0){
  alert("You have not indicated your reason for hiding this record.");
  pg.hideReasonID.focus();
 } else if(pg.hideReasonID.value == 0 && pg.hideReasonID.selectedIndex > 0 && pg.otherReason.value.length < 2){
  alert("You have indicated that you wish to enter a reason\nnot on the list, but you have not entered a reason.");
  pg.otherReason.focus();
 } else {
  pg.workin(1);
  var n = new Object();
  n.noheaders = 1;
  n.recordID = pg.recordID;
  n.recordType = pg.recordType;
  n.recordTypeID = pg.recordTypeID;
  n.encounterID = request.encounterID;
  n.hideReasonID = pg.hideReasonID.value;
  n.hideReason = pg.hideReasonID.value == 0 ? pg.otherReason.value : pg.hideReasonID.options[pg.hideReasonID.selectedIndex].text;

  Connect.Ajax.Post("/emr/hideRecord/act_hideRecord.php", n, pg.exit(pid));


 }
}

pg.init = function(){
 pg.blocker = da.blocker;
 pg.hourglass = da.hourglass;

 pg.content = da.pageContent;

 pg.recordType = da.recordType.value;
 pg.recordID = parseInt(da.recordID.value);
 pg.recordTypeID = parseInt(da.recordTypeID.value);

 pg.information = da.information;

 pg.hideReasonID = da.hideReasonID;
 pg.hideReasonID.onchange = pg.changeHideReasonID;
 pg.hideReasonID.tabIndex = 1;

 pg.otherReason = da.otherReason;
 pg.otherReason.tabIndex = 2;
 pg.otherReason.onblur = function(){
  this.value = this.value.trim();
 }
 pg.otherReason.onfocus = function(){
  this.select();
 }

 pg.btnCancel = da.btnCancel;
 pg.btnCancel.tabIndex = 4;
 pg.btnCancel.title = "Close this window";
 pg.btnCancel.onclick = function(){
  //window.close();
  parent.$.fancybox.close(); 
 }

 pg.btnHide = da.btnHide;
 pg.btnHide.tabIndex = 3;
 pg.btnHide.onclick = pg.hideRecord;
 pg.btnHide.title = "Hide " + pg.recordType.toLowerCase() + " record";

 document.body.onselectstart = function(){
  if(event.srcElement.tagName.search(/INPUT|TEXT/i)){
   return false;
  }
 }

 pg.workin(0);
}

pg.workin = function(){
 var n = arguments.length ? arguments[0] : 1;
 pg.content.disabled = pg.hideReasonID.disabled = n;
 pg.blocker.style.display = pg.hourglass.style.display = n ? "block" : "none";
 if(n){
  pg.otherReason.disabled = 1;
  pg.otherReason.style.backgroundColor = "f5f5f5";
 } else {
  pg.otherReason.disabled = !(pg.hideReasonID.value == 0 && pg.hideReasonID.selectedIndex > 0);
  pg.otherReason.style.backgroundColor = pg.otherReason.disabled ? "f5f5f5" : "ffffff";
  pg.hideReasonID.focus();
 }
}

Answer 1

I think your main problem is that you're swallowing exceptions, which is very bad. This is why "it works sometimes". Something is throwing an exception, and you're catching it, but then you're not doing anything else after that. At the very least I would display some sort of error message in your catch block.

A few other problems:

• Are you sure you need those multiple try..catch blocks? The current assumption in your code is that each line that is wrapped in a try..catch is independent of the others, and execution can still proceed if something goes wrong in any one (or more) of those statements. Are you sure this is what you want? If so, there is definitely a better way of handling this.
• If the statements are not independent of each other, and if a failure at any point means that execution cannot proceed, then you can wrap all of those statements in a single try..catch block and display an error message in the catch
• Like I said before, swallowing exceptions is very bad! You're hiding the problem and not achieving anything. It also makes debugging extremely hard, because things will stop working and you will have no idea why (no exception, no logging, no error messages). Exceptions are used when something unexpected happens that interrupts normal program flow. It is something you definitely want to handle.

I think what you want can be done this way:

try {
    if(window.opener.hideRecordReload){
        window.opener.hideRecordReload(pg.recordID, pg.recordTypeID);

    } else {
        window.opener.pg.hideRecord(pg.recordID, pg.recordTypeID);

    }

    window.opener.pg.hideEncounter(pg.recordID);
    window.opener.pg.hideRecordResponse(pg.hideReasonID.value == 0 ? pg.otherReason.value : pg.hideReasonID.options[pg.hideReasonID.selectedIndex].text);
    window.opener.pg.hideRecord_Response(pg.recordID, pg.recordTypeID);
    window.opener.pg.hideRecord_Response(pg.recordID, pg.recordTypeID);
    window.opener.window.parent.frames[1].pg.loadQualityMeasureRequest();
    window.opener.pg.closeWindow();

}

catch(e) {
   console.log(e);
}   

This way, if an exception occurs anywhere along those series of statements, the catch block will handle it.

Javascript also doesn't have true checked-exceptions. You can get around it by having a single try block, and inspecting the exception object that you receive^*.

Expanding on what I talked about earlier, there are two ways of handling exceptions. The first way, like I showed earlier, assumes that when an exception happens, the code is in an invalid/undefined state and this therefore means that the code encountered an unrecoverable error. Another way of handling exceptions is if you know it is something you can recover from. You can do this with a flag. So:

try {
   doSomething();
}

catch(e) {
   error = true;
}

if(error) {
   doStuffToRecoverFromError();
}

else {
   doOtherStuff();
}

In this case the flow of your logic depends on an exception being thrown. The important thing is that the exception is recoverable, and depending on whether it was thrown or not, you do different things.

^*Here is a somewhat contrived example that demonstrates checked-exceptions. I have two exceptions called VeryBadException and ReallyBadException that can be thrown (randomly) from two functions. The catch block handles the exception and figures out what type of exception it is by using the instanceof operator):

function VeryBadException(message) {
   this.message = message; 
}

function ReallyBadException(message) {
   this.message = message;
}

function foo() {
   var r = Math.floor(Math.random() * 4);
   if(r == 2) {
      throw new VeryBadException("Something very bad happened!");
   }
}

function bar() {
   var r = Math.floor(Math.random() * 4);
   if(r == 1) {
      throw new ReallyBadException("Something REALLY bad happened!");
   }
}

try {
  foo();
  bar();
}

catch(e) {
   if(e instanceof VeryBadException) {
      console.log(e.message);
   }

   else if(e instanceof ReallyBadException) {
      console.log(e.message);
   }
}

Answer 2

It's good practice do something with the caught exceptions.

What's happening here is that if there's an error (say loading a page fails) an exception is thrown inside one of your try blocks. The corresponding catch block grabs it and says "that exception has been dealt with" but in actuality you've not done anything with it.

Try putting a print(e.Message); inside your catch blocks to find out exactly what error is causing the page not to load and then add code to your catch block to deal with this error.