Bug 199854 - [archives][api][breaking] Improve error reporting for archive handlers
Summary: [archives][api][breaking] Improve error reporting for archive handlers
Status: RESOLVED FIXED
Alias: None
Product: Target Management
Classification: Tools
Component: RSE (show other bugs)
Version: 2.0   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: 3.0 M7   Edit
Assignee: Martin Oberhuber CLA
QA Contact: Martin Oberhuber CLA
URL:
Whiteboard:
Keywords: api
Depends on: 226374
Blocks: 199065 230917 221211
  Show dependency tree
 
Reported: 2007-08-14 07:28 EDT by Martin Oberhuber CLA
Modified: 2008-05-07 11:52 EDT (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Oberhuber CLA 2007-08-14 07:28:18 EDT
+++ This bug was initially created as a clone of Bug #199065 +++

The ArchiveHandlerManager and ISystemArchiveHandler APIs consist of methods that return a boolean (true or false) to indicate successful completion or error. In case an error happens, clients have no chance to find out what the reason for the error was (e.g. disk full, permission denied, ...)

This situation should be improved, by either having the archive handler APIs throw exceptions in case of error (IOException or SystemMessageException), or by providing a method to return the last error:
   public Throwable ISystemArchiveHandler#getLastError();
or, by providing an internal logging facility that can be queried:
   public Throwable[] ArchiveHandlerManager#getErrorLog();
or, by declaring a logging interface and installing it with the archive handlers:
   public void ArchiveHandlerManager#setErrorLog(ILogger log);

None of these methods can be backward compatible. To me, throwing exceptions seems the cleanest solution.
Comment 1 Martin Oberhuber CLA 2008-05-06 17:04:41 EDT
The dstore Miner, e.g.

   CreateFileThread.handleCreateVirtualFile()

currently doesn't handle errors in the archive handling at all. Such errors are currently printed to stdout, but the client is not at all informed about these errors. See also bug 199065.
Comment 2 Martin Oberhuber CLA 2008-05-06 23:01:01 EDT
Fixed 
   ISystemArchiveHandler
   ArchiveHandlerManager
   VirtualChild
as well as subclasses to throw SystemMessageException rather than printing to stdout. Some API changes were also required with the consumers:

   Miner.handleCommand() now throws Exception
   UniversalFileSystemMiner.handleCommand() now throws SystemMessageException

The new code should be equivalent to the old one, except for archive handler errors being forwarded now by means of exceptions rather than printing to stdout. I did not yet review all DStore usage of this, whether Exceptions are properly forwarded; I'll also still need to run the unit tests.

Fix committed:
[199854][api] Improve error reporting for archive handlers
   org.eclipse.dstore.core
   org.eclipse.rse.services/clientserver
   org.eclipse.rse.services.dstore/miners
   
Comment 3 Martin Oberhuber CLA 2008-05-07 04:26:34 EDT
On Local, all Unit Tests are green again. This refactoring was a MAJOR effort of approx 12 hours so far, and would not have been possible without Unit Tests.