Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 209066

Summary: [concurrency] RequestMonitor should preserve the error message intact when propagating errors.
Product: [Tools] CDT Reporter: Pawel Piech <pawel.1.piech>
Component: cdt-debug-dsfAssignee: Marc Khouzam <marc.khouzam>
Status: CLOSED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: cdtdoug, marc.khouzam
Version: 0 DD 1.0   
Target Milestone: DD 1.0   
Hardware: PC   
OS: Linux   
Whiteboard:
Attachments:
Description Flags
Removal of setMultiStatus
none
Updated patch for the bug
cdtdoug: iplog-
Updaetd change to RequestMonitor cdtdoug: iplog-

Description Pawel Piech CLA 2007-11-07 12:59:45 EST 
The RequestMonitor.handleError() currently modifies the error message when propagating an error from a lower level request to a higher level request.  This results in the top-level client having an un-readable error message which cannot be internationalized.

The RequestMonitor.handleError() method should pass the error status to the parent without modification.

Alse see: https://bugs.eclipse.org/bugs/show_bug.cgi?id=208920#c2
Comment 1 Pawel Piech CLA 2007-11-07 13:00:38 EST 
Also setMultiStatus() can be removed it clutters the API.
Comment 2 Marc Khouzam CLA 2007-11-08 09:13:49 EST 
Unless Pawel has already taken this, I will take it.
Comment 3 Marc Khouzam CLA 2007-11-08 09:36:42 EST 
Created attachment 82443 [details]
Removal of setMultiStatus

This patch fixes this bug.

I was wondering if we should still use the MultiStatus for the log, in the case there is not parentRequestMonitor?  I believe that the extra information provided may be more justified for a log...  But I'm not convinced :-)
Comment 4 Marc Khouzam CLA 2007-11-08 10:12:35 EST 
Created attachment 82449 [details]
Updated patch for the bug

I had missed that the previous file broke the compilation :-O

This is a fix.  However, the code that needed to be changed, in VMCache.java, was different than the pattern I am used to seeing.  After trying to figure it out, I couldn't see why it didn't use the standard parttern, so I changed it accordingly.  However, I am wondering if the changes I made broke a specific behaviour in VMCache, which justified this different pattern?
Comment 5 Pawel Piech CLA 2007-11-08 13:56:08 EST 
We talked about the different pattern in VMCache recently, and Ted doesn't remember why he did it that way.  So go ahead and change it to the standard pattern.

One change to the patch I would request is that in handleRejectedExecutionException(), the request should create a its own error status and set it to the parent, rather than propagating the old status.  Rejected execution exception is an error in its own right that should be handled.
Thanks
Pawel
Comment 6 Marc Khouzam CLA 2007-11-08 14:48:24 EST 
Created attachment 82484 [details]
Updaetd change to RequestMonitor

I committed VMCache.java as per patch 82449.
I updated RequestMonitor as per Pawel's comment as can be seen in this new patch.
I have also committed this patch.
Comment 7 Marc Khouzam CLA 2007-11-08 14:54:41 EST 
Code was checked in
Comment 8 Pawel Piech CLA 2008-08-13 13:05:08 EDT 
Closing out 1.0 bugs.