CODEWITHSUNDEEP
java
frazman
frazman

Reputation: 33223

propagating error upwards in java

I have a main class where I have something like

void FooBar(String s){

  try {
    parseString(s);
  } catch (Exception e) {
    e.printStackTrace();
    System.err.println("Error: " + e.getMessage());
    context.getCounter(Counters.ERROR).increment(1); // this increment doesnt increases
  }
}

parseString is 

void ParseString(String s){
  if (matcher.matches()) {

  } else {
    //throw exception
    try {
      throw new Exception("bad formatted N-triples");
    } catch (Exception e) {
      // TODO Auto-generated catch block
      e.printStackTrace();
    }
  }
}

But for some reason, the error is not propagated upwards. In my FooBar method the error counters are not incremented even if the function gets bad formatted data.

How do I propagate this exception upwards?

Upvotes: 4

Views: 388

Answers (4)

Stephen C
Stephen C

Reputation: 718788

But for some reason, the error is not propagated upwards...

The reason it is not propagated upwards is that you caught it. Exceptions stop propagating when they are caught.

Either don't catch it in parseString, or rethrow it in the handler; e.g. e.printStackTrace(); throw e;

However, this is likely to get you into more problems, specifically because of the exception you are catching / throwing here. The problem is that Exception is the root of all checked exceptions:

  • Since it is a checked exception, the method parseString must declare that it throws the Exception if you want to the exception to propagate.

  • But throws Exception is saying that this method could throw any possible checked exception ... which makes life difficult for the caller. (Not in this example ... but in general.)

My advice is as follows:

  • Avoid creating / throwing Exception. Pick a more specific (checked or unchecked) exception that reflects the meaning of the "exceptional event" you are trying to report ... or implement your own exception class. In this case throwing IllegalArgumentException would probably be better, though that is an unchecked exception.

  • Avoid situations where you need to propagate Exception.

  • Be careful when you catch Exception. It catches every (non-Error) exception, including all of the unchecked ones; i.e. RuntimeExecption and its subclasses.

Upvotes: 9

BlackHatSamurai
BlackHatSamurai

Reputation: 23483

You shouldn't surround your error with try/catch:

void ParseString(String s){
  if (matcher.matches()) {

        }
        else{
            //throw exception
            throw new Exception("bad formatted N-triples");}
        }
}

When you throw the error, it's caught in the catch statement within parseString method, which is why isn't isn't propagated to the top.

Ideally, you'd do:

void ParseString(String s) throws Exception  {
  if (matcher.matches()) {

        }
        else{
            //throw exception
            throw new Exception("bad formatted N-triples");}
        }
}

Upvotes: 2

Nathaniel Ford
Nathaniel Ford

Reputation: 21220

Examine what you're doing here:

try {
  throw new Exception("bad formatted N-triples");//You throw an exception!
} catch (Exception e) {//And immediately catch it!
  e.printStackTrace();
}

Because the exception is caught, it will not propagate. Instead, remove the try/catch block and simply throw the exception:

void ParseString(String s){
  if (matcher.matches()) {
    //code for reasons
  } else {
    //throw exception
    throw new Exception("bad formatted N-triples");
  }
}

Note that this is actually bad practice. You want to say something about your exception, and declare it:

void ParseString(String s) throws IllegalArgumentException {
  if (matcher.matches()) {
    //code for reasons
  } else {
    //throw exception
    throw new IllegalArgumentException("bad formatted N-triples");
  }
}

The surrounding function should know how to cope with that exception explicitly, rather than just throwing it's hands up because it's a general exception.

Upvotes: 4

Louis Wasserman
Louis Wasserman

Reputation: 198033

You either don't catch it in ParseString, or you rethrow it with throw e;

Once an exception is caught, it doesn't get propagated unless you throw it again.

Upvotes: 3

Related Questions