Restore old exception handling for #2738
Conversation
| } catch (Throwable e) { | ||
| throw new RuntimeException(e); | ||
| } catch (CoreException e) { | ||
| Exceptions.sneakyThrow(e); |
There was a problem hiding this comment.
excedption handling is still bogus in
org.eclipse.xtext.builder.builderState.MarkerUpdaterImpl.processDelta(Delta, ResourceSet, IProgressMonitor)
case where delete markers is dirctly called. wonder if we should catch and log there too
There was a problem hiding this comment.
@szarnekow could you please have a look?
There was a problem hiding this comment.
wonder how to run in the delta.getNew() == case? uninstalling the language? but how get we the contributor then?
There was a problem hiding this comment.
https://ci.eclipse.org/xtext/job/xtext/job/cd_restore_deleteMarkers_b/1/
seems to be ran by some tests
There was a problem hiding this comment.
exception will fall though, be caught and rethrown here
and finally handled here
this makes limited sense to me. @szarnekow what do you think?
There was a problem hiding this comment.
other code like
org.eclipse.xtext.ui.validation.MarkerEraser.deleteValidationMarkers(IFile, CheckMode, IProgressMonitor)
org.eclipse.xtext.ui.validation.MarkerEraser.deleteValidationMarkers(IFile, CheckMode, IProgressMonitor)
directly catch and log
=> propose to change to
@Override
public void deleteMarkers(IFile file, IProgressMonitor monitor) {
try {
file.deleteMarkers(TaskMarkerTypeProvider.XTEXT_TASK_TYPE, true, IResource.DEPTH_ZERO);
} catch (CoreException e) {
LOG.error(e.getMessage(), e);
}
}
```
There was a problem hiding this comment.
I agree. We shouldn't fail miserably because some marker wasn't created
Signed-off-by: Christian Dietrich <chriTaskMarkerContributorstian.dietrich@itemis.de>
cdietrich
force-pushed
the
cd_restore_deleteMarkers
branch
from
88a8cd6
to
4cebfc2
Compare
Restore old exception handling for