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

[Tor Bug Tracker & Wiki]

#16225: Unify exception/error handling in metrics-lib
---------------------------------+-----------------------------------
 Reporter:  karsten              |          Owner:  karsten
     Type:  enhancement          |         Status:  needs_information
 Priority:  Medium               |      Milestone:  metrics-lib 1.7.0
Component:  Metrics/metrics-lib  |        Version:
 Severity:  Normal               |     Resolution:
 Keywords:                       |  Actual Points:
Parent ID:                       |         Points:
 Reviewer:                       |        Sponsor:
---------------------------------+-----------------------------------

Comment (by iwakeh):

 Replying to [comment:11 karsten]:
 > [snip]
 > > > How about this: whenever we can't parse a descriptor, we include a
 `Descriptor` instance with the raw contents we cannot parse in the
 descriptor queue.  And we include a new method `Descriptor#getException()`
 that returns the `DescriptorParseException` that we ran into, or `null` if
 we didn't run into an exception while parsing.
 > > >
 > > > Similarly, if we run into an exception while downloading files from
 a remote server or while reading files from the file system, so before
 splitting contents into descriptor-sized chunks and attempting to parse
 those, we include a `Descriptor` instance without descriptor contents and
 with just the exception.
 > >
 > > If we were using java8, I'd suggest using
 java.util.Optional<Descriptor>.
 >
 > It seems like we won't have Java 8 in the near future (see #19622), but
 can you elaborate how that would help here?

 I didn't sink to much time into the possible design options.  It'll also
 mean a change from the blocking method calls to always returning
 something.   Actually, that would be achievable with `Future`s much
 better, where the API user decides when to wait.  But, just some random
 thoughts here, no change proposal intended.

 >
 > > Actually your suggestions goes in that direction, too.
 > > Maybe, a new descriptor class InvalidDescriptor could be useful here?
 It would be accessible via the instanceof-method and have limited
 information: at least the Exception and maybe also its origin (url, path),
 in case of a file also bytes.
 >
 > I briefly thought about such a new type for this case, too.  But the
 only reason I found was that there wouldn't be a method
 `Descriptor#getException()` that sometimes returns `null`.  However,
 adding such a type would mean that whoever is interested in
 exceptions/errors would have to do another `instanceof` check and
 downcast.  Are there other aspects?

 Yes, the usefulness of one or the other approach depends on the
 programming style of the API using client.  I don't feel strongly about
 either.

 Regarding the methods returning a collection type (like unrecognized
 fields for torperf/onionperf or the get-exception method above), I think
 it would be more convenient to return an empty (immutable) collection of
 the wanted type (like
 [https://docs.oracle.com/javase/7/docs/api/java/util/Collections.html#emptySet%28%29
 emptySet] instead of `null`.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/16225#comment:12>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online