Bug 272882

Summary:

[api] Handle exceptions in IService.initService()

Product:

[Tools] Target Management

Reporter:

Martin Oberhuber <mober.at+eclipse>

Component:

RSE

Assignee:

David McKnight <dmcknigh>

Status:

RESOLVED FIXED

QA Contact:

Martin Oberhuber <mober.at+eclipse>

Severity:

enhancement

Priority:

CC:

eugene, uwe.st

Version:

3.1

Keywords:

api

Target Milestone:

3.1 M7

Hardware:

All

OS:

All

Whiteboard:

Bug Depends on:

271244

Bug Blocks:

227135

Attachments:

Description	Flags
patch with initService() and initSubSystem() throwing SystemMessageException	none

Description Martin Oberhuber

2009-04-20 08:49:57 EDT

+++ This bug was initially created as a clone of Bug #271244 +++

A recent change to the Javadocs in IService#initService brought up the fact that IService#initService may also initialize remote agents, miners or channels as part of its work -- that is, after the ConnectorService has connected a shared communication channel, initService() may perform additional work on the remote.

The current API contract of IService#initService() does not throw any checked exceptions. This is problematic in the case where remote operations may be performed, since detection and proper reporting of errors to the user is awkward without exceptions.

As per the Binary Compatibility Guide, adding checked exceptions to a method breaks contract compatibility and source compatibility, but does not break binary compatibility:

http://wiki.eclipse.org/Evolving_Java-based_APIs_2#Evolving_API_interfaces_-_API_methods

Since binary compatibility is not compromised, I'm in favor of adding
   throws SystemMessageException
to IService#initService() in order to better handle exceptional conditions (such as remote service unavailable) during IService#initService().

There are other possible options to fix the problem (such as logging failures, or storing away exceptions and retrieving them via some new IService#getLastFailure() call), but to me these seem awkward compared to the small change of adding a checked exception to initService and thus making it behave as we think it should behave.

Any thoughts ?

Comment 1 Martin Oberhuber

2009-04-20 08:50:39 EDT

Should decide and fix this for M7 in order to properly fix bug 227135.

Comment 2 David McKnight

2009-04-24 11:52:30 EDT

If we throw a SystemMessageException for IService.initService() do you think we should also throw a SystemMessageException for ISubSystem.initSubSystem()?

Comment 3 David McKnight

2009-04-24 12:15:02 EDT

Created attachment 133144 [details]
patch with initService() and initSubSystem() throwing SystemMessageException

With this patch, the services and subsystems allow for a SystemMessageException to get propagated back to the caller of connect().

Comment 4 David McKnight

2009-04-28 10:52:30 EDT

I've committed most of the changes to cvs.  

I wasn't able to commit DeveloperSubSystem from org.eclipse.rse.examples.tutorial.  I get the following message when I try:

The server reported an error while performing the "cvs commit" command.
  org.eclipse.rse.examples.tutorial: cvs [server aborted]: "commit" requires write access to the repository

Comment 5 David McKnight

2009-04-28 15:38:31 EDT

Anna was able to commit the fix for the DeveloperSubSystem.