This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.
Created attachment 119274 [details] Proposed patch WSIT code calls XAM Model.endTransaction(), which may throw IllegalStateException. The exception is not handled in WSIT code. Please review and correct/apply the attached patch, which attempts to improve the ISE handling. For more information, please seee issue #177636.
Hi Denis, would you be able to apply the attached patch?
(In reply to comment #1) > Hi Denis, would you be able to apply the attached patch? Sure , I can. But I don't like the unchecked exceptions. So I don't like proposed design for the endTransaction() method. Is it really necessary to throw ISE in Model.endTransaction() ? May be Model.endTransaction() implementation should not throw ISE at all ? What is the benefit of catching ISE in the patch ?
re.: benefit of handling ISE: if the ISE is thrown - because of very 'common' reason like concurrent modification of the document, or an external refresh, your code has a chance to present the situation to the user nicely, not as an exception dialog with "report defect" button. re. not throwing anything at all: that would not be a good idea; if endTransaction fails and the operation does not succeed, the caller must be informed somehow. re. design: there are 2 'schools' for exception handling. One of them insists on that exceptions which are thrown because of 'external, unforeseeable' circumstances, like concurrent external modification to the document (e.g. from VCS) in this case, SHOULD be reported as unchecked exceptions - since most of the client code cannot really do anything about it, but cleanly abort and report to the end user. The reporting is best done in some broad top-level handler, instead of declaring a checked exception all along the execution path. Exceptions which are part of the computation, although on exceptional paths, should remain checked. Such design leads to much cleaner and pleasant code, although you may argue that people may forget to work with the unchecked exceptions. But definitely typing the exception as ISE is a bad idea, since it's quite overloaded. Such unchecked 'uncoordinated error' exception should have its own subtype of Runtime. In this case, I am not 100% sure that endTransaction() should throw a checked exception; but unlike the released API, it must somehow inform about a failure from lower layers, or from some out-of-sync state. re.: the actual patch contents: Please take into consideration that I do not deeply understand your code; so reporting used in the patch may not be necessarily the best one for the specific case. But somehow the user should be informed.
web-main#2dad699cd3a2
Integrated into 'main-golden', will be available in build *201205150400* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress) Changeset: http://hg.netbeans.org/main-golden/rev/2dad699cd3a2 User: Denis Anisimov <ads@netbeans.org> Log: Fix for BZ#212299 - Handle IllegalStateException from XAM Model.endTransaction()