Search code examples
javaexceptiontry-catchscriptengine

ScriptEngineManager extra cautious exception handling


I wanted to consult about JDK code exception handling,

In ScriptEngineManager from lines 120 there are unused secondary catch for ServiceConfigurationError which can't be thrown as I understand it

   try {
        while (itr.hasNext()) {
            try {
                ScriptEngineFactory fact = (ScriptEngineFactory) itr.next();
                facList.add(fact);
            } catch (ServiceConfigurationError err) {
                System.err.println("ScriptEngineManager providers.next(): "
                             + err.getMessage());
                if (DEBUG) {
                    err.printStackTrace();
                }
                // one factory failed, but check other factories...
                continue;
            }
        }
    } catch (ServiceConfigurationError err) {
        System.err.println("ScriptEngineManager providers.hasNext(): "
                        + err.getMessage());

Is there a reason why a second catch will be necessary? it seems it effect only while (itr.hasNext()) which doesn't throw any exception

Or is it just overly cautious to ensure method not throwing exception in any case, as commented

// do not throw any exception here. 

Actually java allows you to duplicate such try-catch without any error/warning:

 try {
     try {
            ScriptEngineFactory fact = itr.next();
            engineSpis.add(fact);
        } catch (ServiceConfigurationError err) {
            err.printStackTrace();
        }
    } catch (ServiceConfigurationError err) {
        err.printStackTrace();
    }

If I'll concatenate catches in same try I would get compilation error

Unreachable catch block for ServiceConfigurationError. It is already handled by the catch block for ServiceConfigurationError

Solution

  • Minor misconception: the second catch does not only cover the while loop. It would also take care about such exceptions thrown from within the first catch block.

    But you are correct: that catch block, as well as the loop "header" should not throw such an exception. It seems rather odd that simply iterating an iterator needs "protection" in such a way.

    Thus: maybe this is a leftover when other code existed in that method. Or it is overdoing. Or worst case, the code that we don't see (that creates that iterator) can in fact throwing that kind of error. Which, as said, would be odd and a very bizarre design, to say the least.