[tor-bugs] #16225 [metrics-lib]: Unify exception/error handling in metrics-lib

["Tor Bug Tracker & Wiki" <blackhole@xxxxxxxxxxxxxx>]

#16225: Unify exception/error handling in metrics-lib
-------------------------+-------------------------
 Reporter:  karsten      |          Owner:  karsten
     Type:  enhancement  |         Status:  new
 Priority:  normal       |      Milestone:
Component:  metrics-lib  |        Version:
 Keywords:               |  Actual Points:
Parent ID:               |         Points:
-------------------------+-------------------------
 There are now four different descriptor sources in metrics-lib:
 `DescriptorParser`, `DescriptorReader`, `DescriptorCollector`, and
 `DescriptorDownloader`.

 We should think about unifying how we're handling exceptions and errors
 and telling the user about them.  These thoughts should include whether or
 not to log exceptions and errors, though just doing that is probably not
 sufficient.

 Let's go through the interfaces one by one:

 {{{
 public interface DescriptorParser {
   public List<Descriptor> parseDescriptors(byte[] rawDescriptorBytes,
       String fileName, boolean failUnrecognizedDescriptorLines)
       throws DescriptorParseException;
 }
 }}}

 The (slightly modified, as compared to current master) `parseDescriptors`
 method handles a single file and throws an exception whenever something
 goes wrong.  That works quite well, because that method blocks the caller,
 so they can easily wrap the call inside a `try/catch` block.

 That's somewhat different in the next interface:

 {{{
 public interface DescriptorCollector {
   public void collectRemoteFiles(String collecTorBaseUrl,
       String[] remoteDirectories, long minLastModified,
       File localDirectory, boolean deleteExtraneousLocalFiles);
 }
 }}}

 This method (which we should have called `collectDescriptors`, in
 retrospect) blocks the caller, too, but it's unclear whether it should
 abort its operation when it hits the first exception.  Right now it's
 rather silent about problems and only prints stack traces to `System.err`.
 But that doesn't enable the caller to handle problems, neither.  At least
 the method makes sure that it doesn't delete extraneous local files that
 might still exist remotely in case of an error.

 The next interface is trickier:

 {{{
 public interface DescriptorReader {
   public Iterator<Descriptor> readDescriptors(File[] directories,
       File[] tarballs, SortedMap<String, Long> excludeFiles,
       boolean failUnrecognizedDescriptorLines, int maxDescriptorsInQueue);
 }
 }}}

 Note that this interface does not yet exist, but it's what I could imagine
 doing to simplify the current interface and make it more similar to the
 `DescriptorCollector` interface.  I could also imagine overloading this
 method.

 The idea here is that the map passed in `excludeFiles` would be updated
 while reading descriptors, so that the caller could use that to update a
 local history file.  (This would be documented, of course.)

 However, it's unclear how we would handle problems at all here.  What we
 ''could'' do is extend the implementation of the returned
 `Iterator<Descriptor>` to return a (runtime) exception whenever there was
 a problem reading the next descriptor that would be added next.  Or maybe
 we should write our own `Iterator`-like interface for returning
 descriptors and have its `hasNextDescriptor()` and `nextDescriptor()`
 methods throw `DescriptorParseException`.

 By the way, the current `DescriptorReader` interface has a
 `getExceptions()` method in the additional `DescriptorFile` interface that
 is returned instead of `Descriptor`.  But that's something I'd like to get
 rid of, too.  I also don't expect many users to look at those exceptions.

 The last interface is the mostly dysfunctional `DescriptorDownloader`
 interface.  I could imagine that we handle exceptions/errors there similar
 to `DescriptorReader`.  Or we throw out that class, because it's only used
 by CollecTor, and generalizing its functionality might take more effort
 than writing it cleanly as part of CollecTor.

 Thinking about the future, we might want to add another interface
 `DescriptorWriter`, and we should think about ways to handle
 exceptions/errors there, too.

 There, I mostly sketched out the problem here, without having good
 solutions.  But maybe we can come up with good answers in this discussion?

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/16225>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs