Bug #6715
BufferManager.validate throws IllegalStateException if a validation fails via a ValidateException
0%
History
#2 Updated by Constantin Asofiei over 1 year ago
If validation fails in BufferManager.validate
, this abends:
catch (Exception e) { throw new IllegalStateException("Exception at dirtyBuffers.flagIterating block", e); }
with this exception:
Caused by: java.lang.IllegalStateException: Exception at dirtyBuffers.flagIterating block at com.goldencode.p2j.persist.BufferManager.validate(BufferManager.java:1021) at com.goldencode.p2j.util.TransactionManager.processValidate(TransactionManager.java:6959) at com.goldencode.p2j.util.TransactionManager.access$10500(TransactionManager.java:658) at com.goldencode.p2j.util.TransactionManager$TransactionHelper.processValidate(TransactionManager.java:9742) at com.goldencode.p2j.util.BlockManager.lambda$processBody$0(BlockManager.java:8608) at com.goldencode.p2j.util.BlockManager.executeStateManaged(BlockManager.java:11347) at com.goldencode.p2j.util.BlockManager.processBody(BlockManager.java:8608) at com.goldencode.p2j.util.BlockManager.doBlockWorker(BlockManager.java:9688) at com.goldencode.p2j.util.BlockManager.doBlock(BlockManager.java:1467) ...... Caused by: com.goldencode.p2j.util.ErrorConditionException: com.goldencode.p2j.persist.ValidationException: <record> already exists with <f1> "" <f2> "" <f3> "" Caused by: com.goldencode.p2j.persist.ValidationException: <record> already exists with <f1> "" <f2> "" <f3> ""
#3 Updated by Greg Shah over 1 year ago
Converting a legacy error into an abend seems like something easily avoided.
#4 Updated by Eric Faulhaber over 1 year ago
Yes, ErrorConditionException
should be allowed through; it should not be wrapped in IllegalStateException
. I think the catch block was added to handle Exception
thrown by AutoCloseable.close()
, but that's too broad of a net, and it doesn't help us to throw IllegalStateException
from here; that's an exception that we probably should just log and eat. My bad, I missed this in code review. I'm surprised we haven't hit this sooner.
We could catch and rethrow ConditionException
in another catch block inserted before the existing one, or (uglier) remove the catch block and manually close the AutoCloseable
in a finally block (thus defeating the "auto" aspect).
Tijs, could you please check the files you committed related to the TailMap
implementation? I just noticed that at least TailMapAutoCloseable.java
has CRLF line endings, which is inconsistent with the rest of the source. I didn't check all the other files from that update.
#5 Updated by Constantin Asofiei over 1 year ago
In some cases, a ValidationException (which is from a bug) is wrapped in a ErrorConditionException, like this:
Caused by: com.goldencode.p2j.util.ErrorConditionException: com.goldencode.p2j.persist.ValidationException: Requested to persist DMO of type com.app.Tt_1_1__Impl__ with recid 14. A different DMO instance of type com.app.Tt_1_1__Impl__ is already bound to this session with the same recid
I think we should distinguish from expected validation errors (like 'already exists') and these kind of abends.
#6 Updated by Eric Faulhaber over 1 year ago
Constantin Asofiei wrote:
In some cases, a ValidationException (which is from a bug) is wrapped in a ErrorConditionException, like this:
[...]I think we should distinguish from expected validation errors (like 'already exists') and these kind of abends.
Yes, that is what my proposed approach of catching/rethrowing ConditionException
would do. I don't think there is necessarily a real case for the Exception
catch block, but the exception needs to be handled to satisfy the compiler, at least.
#7 Updated by Tijs Wickardt over 1 year ago
Eric Faulhaber wrote:
Tijs, could you please check the files you committed related to the
TailMap
implementation? I just noticed that at leastTailMapAutoCloseable.java
has CRLF line endings, which is inconsistent with the rest of the source. I didn't check all the other files from that update.
That's not intentional of course. Thanks for spotting, I'll check all files and correct where needed.
#8 Updated by Tijs Wickardt over 1 year ago
Tijs Wickardt wrote:
Eric Faulhaber wrote:
Tijs, could you please check the files you committed related to the
TailMap
implementation? I just noticed that at leastTailMapAutoCloseable.java
has CRLF line endings, which is inconsistent with the rest of the source. I didn't check all the other files from that update.That's not intentional of course. Thanks for spotting, I'll check all files and correct where needed.
Eric, fixed in 3821c
rev 14298.
.
I aslo did a quick scan of the src
folder. The list below should be checked as well (not related to TailMap):
tw@lpt-tw-gcd:~/z/p2j$ find src -type f -name '*.java' -exec grep -qU $'\015' {} \; -ls 77605258 32 -rw-rw-r-- 1 tw tw 31873 Oct 14 20:50 src/com/goldencode/p2j/util/longchar.java 78250283 8 -rw-rw-r-- 1 tw tw 7072 Oct 14 20:50 src/com/goldencode/p2j/oo/net/http/filter/auth/BasicAuthenticationFilter.java 78250594 16 -rw-rw-r-- 1 tw tw 15127 Oct 14 20:50 src/com/goldencode/p2j/oo/net/http/filter/auth/DigestAuthenticationFilter.java 78250960 20 -rw-rw-r-- 1 tw tw 16825 Oct 14 20:50 src/com/goldencode/p2j/oo/net/http/filter/payload/JsonBodyWriter.java 78250735 12 -rw-rw-r-- 1 tw tw 8939 Oct 14 20:50 src/com/goldencode/p2j/oo/net/http/filter/payload/FormDataBodyWriter.java 78250238 16 -rw-rw-r-- 1 tw tw 15924 Oct 14 20:50 src/com/goldencode/p2j/oo/net/http/AuthenticatedRequest.java