Issue 3123. Catch exceptions in Source.exists()/initProperties()

compiler/java/com/google/dart/compiler/UrlSource.java

Index: compiler/java/com/google/dart/compiler/UrlSource.java
diff --git a/compiler/java/com/google/dart/compiler/UrlSource.java b/compiler/java/com/google/dart/compiler/UrlSource.java
index 317a38450031c30dfb70787f758c0bf8dbf2f547..b89e4b5ccff87b49d8828f64aa4c14cee28c8ff8 100644
--- a/compiler/java/com/google/dart/compiler/UrlSource.java
+++ b/compiler/java/com/google/dart/compiler/UrlSource.java
@@ -12,7 +12,6 @@ import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.io.Reader;
 import java.net.JarURLConnection;
-import java.net.MalformedURLException;
 import java.net.URI;
 import java.net.URL;
 import java.nio.charset.Charset;
@@ -125,47 +124,48 @@ public abstract class UrlSource implements Source {
   }
 
   private void initProperties() {
-    if (!propertiesInitialized) {
-      synchronized(this) {
-        if (!propertiesInitialized) {
-          try {
-            URI resolvedUri = BASE_URI.resolve(translatedUri);
-            String scheme = resolvedUri.getScheme();
-            if (scheme == null || FILE_PROTOCOL.equals(scheme)) {
-              // Faster than using URLConnection
-              File file = new File(resolvedUri);
-              lastModified = file.lastModified();
-              exists = file.exists();
-              sourceFile = file;
-            } else {
-              try {
-                URL url = translatedUri.toURL();
-                if (JAR_PROTOCOL.equals(url.getProtocol())) {
-                  getJarEntryProperties(url);
-                } else {
-                  /*
-                   * TODO(jbrosenberg): Flesh out the support for other
-                   * protocols, like http, etc. Note, calling
-                   * URLConnection.getLastModified() can be dangerous, some
-                   * URLConnection sub-classes don't have a way to close a
-                   * connection opened by this call. Return 0 for now.
-                   */
-                  lastModified = 0;
-                  // Default this to true for now.
-                  exists = true;
-                }
-              } catch (MalformedURLException e) {
-                return;
-              }
-            }
-          } finally {
-            propertiesInitialized = true;
-          }
+    synchronized (this) {
+      if (!propertiesInitialized) {
+        try {
+          initPropertiesEx();
+        } catch (Throwable e) {

[Brian Wilkerson, 2012/05/18 17:02:52]

Is there a reason for expanding this to catch all exceptions? (And for that
matter throwing more than MalformedURIException from initPropertiesEx()?) In
general, I think it's better to be more specific, but this might make sense.

Should we be fixing the state of the object if an exception is thrown, or will a
partially initialized source be good enough? (If these questions are bigger than
we need to address at this point feel free to ignore them.)

[scheglov, 2012/05/18 17:21:54]

1. There is reason to throw and catch specific exceptions if you are going to do
something interesting with them. Here we don't.
2. I look on this situation in the following way: this Source is invalid, i.e.
if exists() tells you "false", you should not continue to work with it, all
other methods return undefined values.

+        } finally {
+          propertiesInitialized = true;
         }
       }
     }
   }
 
+  /**
+   * Implementation of {@link #initProperties()} which can throw exceptions.
+   */
+  private void initPropertiesEx() throws Exception {
+    URI resolvedUri = BASE_URI.resolve(translatedUri);
+    String scheme = resolvedUri.getScheme();
+    if (scheme == null || FILE_PROTOCOL.equals(scheme)) {
+      File file = new File(resolvedUri);
+      lastModified = file.lastModified();
+      exists = file.exists();
+      sourceFile = file;
+    } else {
+      URL url = translatedUri.toURL();
+      if (JAR_PROTOCOL.equals(url.getProtocol())) {
+        getJarEntryProperties(url);
+      } else {
+        /*
+         * TODO(jbrosenberg): Flesh out the support for other
+         * protocols, like http, etc. Note, calling
+         * URLConnection.getLastModified() can be dangerous, some
+         * URLConnection sub-classes don't have a way to close a
+         * connection opened by this call. Return 0 for now.
+         */
+        lastModified = 0;
+        // Default this to true for now.
+        exists = true;
+      }
+    }
+  }
+
   private void getJarEntryProperties(URL url) {
     try {
       jarConn = (JarURLConnection) url.openConnection();