Bug 400060 - [misc] Errors in log from fetching Javadoc when working disconnected
Summary: [misc] Errors in log from fetching Javadoc when working disconnected
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.5   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 4.3 M6   Edit
Assignee: Martin Mathew CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on: 247845 400062
Blocks:
  Show dependency tree
 
Reported: 2013-02-06 03:24 EST by Dani Megert CLA
Modified: 2013-03-12 06:21 EDT (History)
7 users (show)

See Also:
daniel_megert: review+


Attachments
Fix. (2.52 KB, patch)
2013-02-22 07:47 EST, Martin Mathew CLA
daniel_megert: review+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dani Megert CLA 2013-02-06 03:24:43 EST
+++ This bug was initially created as a clone of Bug #247845 +++

While it is good that JDT Core does not log those exceptions, it should pass them back to the client.

Currently, we can't show the user that the attached Javadoc could not be accessed.


This part of 'JavaElement.getURLContents(String)' needs to be changed:

		} catch (FileNotFoundException e) {
			// ignore. see bug https://bugs.eclipse.org/bugs/show_bug.cgi?id=120559
		} catch(SocketException e) {
			// ignore. see bug https://bugs.eclipse.org/bugs/show_bug.cgi?id=247845
		} catch(UnknownHostException e) {
			// ignore. see bug https://bugs.eclipse.org/bugs/show_bug.cgi?id=247845
		} catch(ProtocolException e) {
			// ignore. see bug https://bugs.eclipse.org/bugs/show_bug.cgi?id=247845
Comment 1 Dani Megert CLA 2013-02-06 03:29:45 EST
Manju will prepare a patch.
Comment 2 Martin Mathew CLA 2013-02-22 07:47:58 EST
Created attachment 227455 [details]
Fix.

Previously ignored exceptions in JavaElement is now thrown using =>
throw new JavaModelException(e, IJavaModelStatusConstants.CANNOT_RETRIEVE_ATTACHED_JAVADOC);

Clients will have to handle these exceptions.
Comment 3 Dani Megert CLA 2013-02-22 12:25:35 EST
Jay, the patch is good.
Comment 4 Jay Arthanareeswaran CLA 2013-02-26 12:49:24 EST
This fix breaks couple of existing tests, one of them added as part of bug 398272. Both the failures are about JME being thrown for packages that don't have a javadoc file. In other words, package fragment is not handling the exception. Perhaps we should leave out the FileNotFoundException or mark the JME in case of FNFE in a way PackageFragment could recognize and handle it?
Comment 5 Jay Arthanareeswaran CLA 2013-02-27 01:47:09 EST
(In reply to comment #4)
> Perhaps we should leave out the FileNotFoundException or mark the
> JME in case of FNFE in a way PackageFragment could recognize and handle it?

Sorry, I didn't look at the patch close enough. The JME indeed wraps the original exception and that should allow clients to handle accordingly to the the cause.

I will fix the failing code to handle FileNotFoundException.
Comment 6 Jay Arthanareeswaran CLA 2013-02-27 02:18:09 EST
I have released the patch and the other changes required. The current HEAD is here:

http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=843baf8e7455ebd3d6835c29e35eba1ff6fd62f0
Comment 7 Dani Megert CLA 2013-02-27 03:20:17 EST
(In reply to comment #5)
> (In reply to comment #4)
> > Perhaps we should leave out the FileNotFoundException or mark the
> > JME in case of FNFE in a way PackageFragment could recognize and handle it?
> 
> Sorry, I didn't look at the patch close enough. The JME indeed wraps the
> original exception and that should allow clients to handle accordingly to
> the the cause.
> 
> I will fix the failing code to handle FileNotFoundException.

The code you added in PackageFragment is wrong and destroys the purpose of the fix. With that change we again do not get the exception on the client side and this is  wrong. If that lets a test fail, then you need to adjust the test, not the code.
Comment 8 Jay Arthanareeswaran CLA 2013-02-27 03:56:42 EST
So, it looks like we are revisiting the fix for bug 398272 and moving the fix to the client. I thought I should preserve the fix made for aforementioned fix but this bug was actually trying to roll back it in a way.

I have reverted the changes to the PackageFragment and introduced new changes to the tests now.
Comment 9 Dani Megert CLA 2013-02-27 04:00:16 EST
(In reply to comment #8)
> So, it looks like we are revisiting the fix for bug 398272 and moving the
> fix to the client. I thought I should preserve the fix made for
> aforementioned fix but this bug was actually trying to roll back it in a way.
> 
> I have reverted the changes to the PackageFragment and introduced new
> changes to the tests now.

The Javadoc says it all. There are two cases:
1. there *is* no Javadoc (e.g. no URL specified) ==> 'null'
2. then there is the case when an exception happens, and this must be reported
   to the client as specified
Comment 10 Dani Megert CLA 2013-02-27 04:13:53 EST
(In reply to comment #8)
> I have reverted the changes to the PackageFragment and introduced new
> changes to the tests now.

Thanks Jay. All our old and new tests pass now.
Comment 11 Jay Arthanareeswaran CLA 2013-03-05 00:49:14 EST
Marking as Resolved.
Comment 12 ANIRBAN CHAKRABORTY CLA 2013-03-12 05:13:58 EDT
Verified for M6 build eclipse-SDK-I20130310-2000