Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#20320 closed defect (fixed)

Avoid running into an IOException and logging a warning for it

Reported by: karsten Owned by: karsten
Priority: Low Milestone: metrics-lib 1.5.0
Component: Metrics/Library Version:
Severity: Normal Keywords:
Cc: iwakeh Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

When we recently switched from System.err printing to slf4j logging, we started logging an IOException that we shouldn't be running into and that we simply ignored before. This exception gets thrown when DescriptorReaderImpl attempts to read a parse history file that doesn't exist (yet). We should simply check whether that files exists before attempting to read it. Trivial patch:

diff --git a/src/main/java/org/torproject/descriptor/impl/DescriptorReaderImpl.java b/src/main/java/org/torproject/descriptor/impl/DescriptorReaderImpl.java
index fac9475..020cdd7 100644
--- a/src/main/java/org/torproject/descriptor/impl/DescriptorReaderImpl.java
+++ b/src/main/java/org/torproject/descriptor/impl/DescriptorReaderImpl.java
@@ -200,7 +200,7 @@ public class DescriptorReaderImpl implements DescriptorReader {
     }
 
     private void readOldHistory() {
-      if (this.historyFile == null) {
+      if (this.historyFile == null || !this.historyFile.exists()) {
         return;
       }
       try {

I'd say this is a minor issue, because it prints a warning message that might confuse users, but only on the first run. No need to put out a (point) release just for this, but it would be good to fix this in master.

Child Tickets

Change History (6)

comment:1 Changed 3 years ago by karsten

Status: newneeds_review

comment:2 Changed 3 years ago by iwakeh

Coincidence? While testing the sync-functionality I ran into that exact IOE today, and solved it similarly.
The patch is fine.
Can it wait for the big patch?

comment:3 Changed 3 years ago by iwakeh

Status: needs_reviewmerge_ready

Forgot that the sync-implementation is CollecTor.

So, patch is merge ready for metrics-lib.
And, it can wait for a next release.

comment:4 Changed 3 years ago by karsten

Resolution: fixed
Status: merge_readyclosed

Cool! Added a change log entry and merged. Thanks for looking! Closing.

comment:5 Changed 3 years ago by iwakeh

Milestone: metrics-lib 1.4.1

comment:6 Changed 3 years ago by iwakeh

Milestone: metrics-lib 1.4.1metrics-lib 1.5.0
Note: See TracTickets for help on using tickets.