Bug 400060

Summary:

[misc] Errors in log from fetching Javadoc when working disconnected

Product:

[Eclipse Project] JDT

Reporter:

Dani Megert <daniel_megert>

Component:

Core

Assignee:

Martin Mathew <manju656>

Status:

VERIFIED FIXED

QA Contact:

Severity:

normal

Priority:

CC:

anchakrk, daniel_megert, david_audel, jarthana, john.arthorne, manju656, Olivier_Thomann

Version:

3.5

Flags:

daniel_megert: review+

Target Milestone:

4.3 M6

Hardware:

All

OS:

All

Whiteboard:

Bug Depends on:

247845, 400062

Bug Blocks:

Attachments:

Description	Flags
Fix.	daniel_megert: review+

Description Dani Megert

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

2013-02-06 03:29:45 EST

Manju will prepare a patch.

Comment 2 Martin Mathew

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

2013-02-22 12:25:35 EST

Jay, the patch is good.

Comment 4 Jay Arthanareeswaran

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

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

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

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

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

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

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

2013-03-05 00:49:14 EST

Marking as Resolved.

Comment 12 ANIRBAN CHAKRABORTY

2013-03-12 05:13:58 EDT

Verified for M6 build eclipse-SDK-I20130310-2000