[tor-commits] [metrics-lib/master] Log more, throw fewer RuntimeExceptions.
karsten at torproject.org
karsten at torproject.org
Thu Jan 5 09:57:32 UTC 2017
commit 9a6198305599d86e8f76d6b3f5bd6cdb8e15925c
Author: Karsten Loesing <karsten.loesing at gmx.net>
Date: Tue Jan 3 16:15:34 2017 +0100
Log more, throw fewer RuntimeExceptions.
---
.../descriptor/index/DescriptorIndexCollector.java | 65 +++++++++++++++++-----
.../org/torproject/descriptor/index/FileNode.java | 6 +-
.../index/DescriptorIndexCollectorTest.java | 19 ++-----
3 files changed, 59 insertions(+), 31 deletions(-)
diff --git a/src/main/java/org/torproject/descriptor/index/DescriptorIndexCollector.java b/src/main/java/org/torproject/descriptor/index/DescriptorIndexCollector.java
index d32f811..9b269fe 100644
--- a/src/main/java/org/torproject/descriptor/index/DescriptorIndexCollector.java
+++ b/src/main/java/org/torproject/descriptor/index/DescriptorIndexCollector.java
@@ -43,14 +43,23 @@ public class DescriptorIndexCollector implements DescriptorCollector {
public void collectDescriptors(String collecTorIndexUrlString,
String[] remoteDirectories, long minLastModified,
File localDirectory, boolean deleteExtraneousLocalFiles) {
+ log.info("Starting descriptor collection.");
if (minLastModified < 0) {
throw new IllegalArgumentException("A negative minimum "
+ "last-modified time is not permitted.");
}
- if (null == localDirectory || localDirectory.isFile()) {
- throw new IllegalArgumentException("Local directory already exists "
- + "and is not a directory.");
+ if (null == collecTorIndexUrlString || null == remoteDirectories
+ || null == localDirectory) {
+ throw new IllegalArgumentException("Null values are not permitted for "
+ + "CollecTor base URL, remote directories, or local directory.");
}
+ if (localDirectory.isFile()) {
+ throw new IllegalArgumentException("A non-directory file exists at {} "
+ + "which is supposed to be the local directory for storing remotely "
+ + "fetched files. Move this file away or delete it. Aborting "
+ + "descriptor collection.");
+ }
+ log.info("Indexing local directory {}.", localDirectory.getAbsolutePath());
SortedMap<String, Long> localFiles = statLocalDirectory(localDirectory);
SortedMap<String, FileNode> remoteFiles = null;
IndexNode index = null;
@@ -61,20 +70,29 @@ public class DescriptorIndexCollector implements DescriptorCollector {
if (indexUrl.getPath().isEmpty()) {
indexUrlString += "/index/index.json";
}
+ log.info("Fetching remote index file {}.", indexUrlString);
index = IndexNode.fetchIndex(indexUrlString);
remoteFiles = index.retrieveFilesIn(remoteDirectories);
} catch (Exception ex) {
- throw new RuntimeException("Cannot fetch index from '"
- + indexUrlString + "'.", ex);
+ log.warn("Cannot fetch index file {} and hence cannot determine which "
+ + "remote files to fetch. Aborting descriptor collection.",
+ indexUrlString, ex);
+ return;
+ }
+ log.info("Fetching remote files from {}.", index.path);
+ if (!this.fetchRemoteFiles(index.path, remoteFiles, minLastModified,
+ localDirectory, localFiles)) {
+ return;
}
- this.fetchRemoteFiles(index.path, remoteFiles, minLastModified,
- localDirectory, localFiles);
if (deleteExtraneousLocalFiles) {
- this.deleteExtraneousLocalFiles(remoteFiles, localDirectory, localFiles);
+ log.info("Deleting extraneous files from local directory {}.",
+ localDirectory);
+ deleteExtraneousLocalFiles(remoteFiles, localDirectory, localFiles);
}
+ log.info("Finished descriptor collection.");
}
- void fetchRemoteFiles(String baseUrl, SortedMap<String, FileNode> remotes,
+ boolean fetchRemoteFiles(String baseUrl, SortedMap<String, FileNode> remotes,
long minLastModified, File localDir, SortedMap<String, Long> locals) {
for (Map.Entry<String, FileNode> entry : remotes.entrySet()) {
String filepathname = entry.getKey();
@@ -88,10 +106,17 @@ public class DescriptorIndexCollector implements DescriptorCollector {
continue;
}
if (!filepath.exists() && !filepath.mkdirs()) {
- throw new RuntimeException("Cannot create dir: " + filepath);
+ log.warn("Cannot create local directory {} to store remote file {}. "
+ + "Aborting descriptor collection.", filepath, filename);
+ return false;
}
File destinationFile = new File(filepath, filename);
File tempDestinationFile = new File(filepath, "." + filename);
+ log.debug("Fetching remote file {} with expected size of {} bytes from "
+ + "{}, storing locally to temporary file {}, then renaming to {}.",
+ filepathname, entry.getValue().size, baseUrl,
+ tempDestinationFile.getAbsolutePath(),
+ destinationFile.getAbsolutePath());
try (InputStream is = new URL(baseUrl + "/" + filepathname)
.openStream()) {
Files.copy(is, tempDestinationFile.toPath());
@@ -99,13 +124,18 @@ public class DescriptorIndexCollector implements DescriptorCollector {
tempDestinationFile.renameTo(destinationFile);
destinationFile.setLastModified(lastModifiedMillis);
} else {
- log.warn("File sizes don't match. Expected {}, but received {}.",
- entry.getValue().size, tempDestinationFile.length());
+ log.warn("Fetched remote file {} from {} has a size of {} bytes "
+ + "which is different from the expected {} bytes. Not storing "
+ + "this file.",
+ filename, baseUrl, tempDestinationFile.length(),
+ entry.getValue().size);
}
} catch (IOException e) {
- log.error("Cannot fetch remote file: {}", filename, e);
+ log.warn("Cannot fetch remote file {} from {}. Skipping that file.",
+ filename, baseUrl, e);
}
}
+ return true;
}
static void deleteExtraneousLocalFiles(
@@ -113,7 +143,10 @@ public class DescriptorIndexCollector implements DescriptorCollector {
File localDir, SortedMap<String, Long> locals) {
for (String localPath : locals.keySet()) {
if (!remoteFiles.containsKey(localPath)) {
- new File(localDir, localPath).delete();
+ File extraneousLocalFile = new File(localDir, localPath);
+ log.debug("Deleting extraneous local file {}.",
+ extraneousLocalFile.getAbsolutePath());
+ extraneousLocalFile.delete();
}
}
}
@@ -137,7 +170,9 @@ public class DescriptorIndexCollector implements DescriptorCollector {
}
});
} catch (IOException ioe) {
- log.error("Cannot stat local directory.", ioe);
+ log.warn("Cannot index local directory {} to skip any remote files that "
+ + "already exist locally. Continuing with an either empty or "
+ + "incomplete index of local files.", ioe);
}
return locals;
}
diff --git a/src/main/java/org/torproject/descriptor/index/FileNode.java b/src/main/java/org/torproject/descriptor/index/FileNode.java
index 02e82da..6af5aa3 100644
--- a/src/main/java/org/torproject/descriptor/index/FileNode.java
+++ b/src/main/java/org/torproject/descriptor/index/FileNode.java
@@ -67,8 +67,10 @@ public class FileNode implements Comparable<FileNode> {
try {
lastModifiedMillis = dateTimeFormat.parse(this.lastModified).getTime();
} catch (ParseException ex) {
- log.warn("Cannot parse date-time. Setting lastModifiedMillis to -1L.",
- ex);
+ log.warn("Cannot parse last-modified time {} of remote file entry {}. "
+ + "Fetching remote file regardless of configured last-modified "
+ + "time. The following error message provides more details.",
+ this.lastModified, this.path, ex);
this.lastModifiedMillis = -1L;
}
}
diff --git a/src/test/java/org/torproject/descriptor/index/DescriptorIndexCollectorTest.java b/src/test/java/org/torproject/descriptor/index/DescriptorIndexCollectorTest.java
index c5a6a5c..8886c85 100644
--- a/src/test/java/org/torproject/descriptor/index/DescriptorIndexCollectorTest.java
+++ b/src/test/java/org/torproject/descriptor/index/DescriptorIndexCollectorTest.java
@@ -210,11 +210,12 @@ public class DescriptorIndexCollectorTest {
assertTrue("found " + res, res.isEmpty());
}
- @Test(expected = RuntimeException.class)
+ @Test()
public void testMinimalArgs() throws IOException {
File fakeDir = tmpf.newFolder("fantasy-dir");
new DescriptorIndexCollector()
- .collectDescriptors(null, new String[]{}, 100L, fakeDir, true);
+ .collectDescriptors("http://localhost", new String[]{}, 100L, fakeDir,
+ true);
}
@Test(expected = IllegalArgumentException.class)
@@ -227,7 +228,7 @@ public class DescriptorIndexCollectorTest {
public void testIllegalDirectory() throws IOException {
File fakeDir = tmpf.newFile("fantasy-dir");
new DescriptorIndexCollector().collectDescriptors(
- null, new String[]{}, 100L, fakeDir, false);
+ "", new String[]{}, 100L, fakeDir, false);
}
@Test(expected = IllegalArgumentException.class)
@@ -236,24 +237,14 @@ public class DescriptorIndexCollectorTest {
null, new String[]{}, 100L, null, false);
}
- @Test(expected = IllegalArgumentException.class)
- public void testExistingFile() throws IOException {
- File fakeDir = tmpf.newFile("fantasy-dir");
- new DescriptorIndexCollector()
- .collectDescriptors(null, null, 100L, fakeDir, false);
- }
-
@Test()
public void testExistingDir() throws IOException {
File dir = tmpf.newFolder();
dir.setWritable(false);
SortedMap<String, FileNode> fm = new TreeMap<>();
fm.put("readonly", new FileNode("w", 2L, "2100-01-01 01:01"));
- thrown.expect(RuntimeException.class);
- thrown.expectMessage("Cannot create dir: " + dir.toString()
- + "/readonly");
new DescriptorIndexCollector()
- .fetchRemoteFiles(null, fm, 100L, dir, new TreeMap<String, Long>());
+ .fetchRemoteFiles("", fm, 100L, dir, new TreeMap<String, Long>());
}
}
More information about the tor-commits
mailing list