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

Bug 458050

Summary: Have DAnalysisSessionImpl.doSave() return status of the catched exception during save
Product: [Modeling] Sirius Reporter: Esteban DUGUEPEROUX <esteban.dugueperoux>
Component: CoreAssignee: Esteban DUGUEPEROUX <esteban.dugueperoux>
Status: CLOSED FIXED QA Contact: Laurent Fasani <laurent.fasani>
Severity: normal    
Priority: P3 CC: laurent.fasani, maxime.porhel, pierre-charles.david
Version: 2.0.0Keywords: triaged
Target Milestone: 3.0.0M7   
Hardware: PC   
OS: Linux   
See Also: https://git.eclipse.org/r/43212
https://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=8bf7b72c5ab56fb559ae5b0078d49f3337548ff0
Whiteboard:
Bug Depends on:    
Bug Blocks: 458052    

Description Esteban DUGUEPEROUX CLA 2015-01-21 10:06:51 EST 
In our Sirius based product, we have a SavingPolicy which throw specific Exceptions to cancel the save and inform the user about the reasons of this cancel.
But since Bug 445603 these exceptions are ignored, this is a regression.
Comment 1 Esteban DUGUEPEROUX CLA 2015-01-21 10:07:58 EST 
A gerrit to fix that https://git.eclipse.org/r/40042
Comment 2 Esteban DUGUEPEROUX CLA 2015-01-22 03:35:48 EST 
Fixed as a9bcbef96881c92b485d05856a3dc9f2220371b3
Comment 3 Maxime Porhel CLA 2015-03-05 04:51:51 EST 
When the save is no in an Exclusive transaction, the TransactionalEditingDomainImpl.runExclusive will put Status.OK_STATUS as status except in case of a rollabck. So the patch provided by Esteban will take this OK_Status whereas we know that something wrong happened. 

Furtermore, we have lost the error dialog saying that a problem occurred during save. If the user has its error lof view closed, he might no see that his save has failed.

 org.eclipse.gmf.runtime.diagram.ui.resources.editor.parts.DiagramDocumentEditor.performSave(boolean, IProgressMonitor) intercept core exceptions. The Saver could throw something to make the document editor react to the problem.
Comment 4 Eclipse Genie CLA 2015-03-05 05:01:01 EST 
New Gerrit change created: https://git.eclipse.org/r/43212
Comment 5 Esteban DUGUEPEROUX CLA 2015-04-20 10:11:17 EDT 
As the DAnalysis.doSave():IStatus is internal and only used by Saver which doesn't use the returned IStatus, a better fix could be to not catch Throwable and not compute this useless IStatus. I will update the change-set to do that.
Comment 6 Esteban DUGUEPEROUX CLA 2015-04-20 11:15:22 EDT 
Scenario to reproduce use case of comment 3 :

1. Have an opened diagram
2. Have a SavingPolicy which throw a NPE
3. Save from the diagram editor
4. We should have a popup displaying the error coming from WorkbenchErrorHandler.handle()

The use of IStatus from DAnalysisSessionImpl.doSave() is useless to support this scenario. But there is yet the issue of session status : the session is sync after a failing save.
Comment 8 Esteban DUGUEPEROUX CLA 2015-04-21 04:07:38 EDT 
Fixed with 8bf7b72c5ab56fb559ae5b0078d49f3337548ff0
Comment 9 Laurent Fasani CLA 2015-05-26 09:18:43 EDT 
technical issue
tested with DAnalysisSessionTests.testSaveSessionWithErrorDuringSave()
Comment 10 Pierre-Charles David CLA 2015-06-24 11:16:21 EDT 
Available in Sirius 3.0.0. See https://wiki.eclipse.org/Sirius/3.0.0.