Project

General

Profile

Bug #6715

BufferManager.validate throws IllegalStateException if a validation fails via a ValidateException

Added by Constantin Asofiei almost 4 years ago. Updated almost 4 years ago.

Status:
New
Priority:
Normal
Assignee:
-
Target version:
-
Start date:
Due date:
% Done:

0%

billable:
No
vendor_id:
GCD
case_num:
version_reported:
version_resolved:
reviewer:
production:
No
env_name:
topics:

History

#2 Updated by Constantin Asofiei almost 4 years 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 almost 4 years ago

Converting a legacy error into an abend seems like something easily avoided.

#4 Updated by Eric Faulhaber almost 4 years 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 almost 4 years 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 almost 4 years 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 almost 4 years ago

Eric Faulhaber wrote:

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.

That's not intentional of course. Thanks for spotting, I'll check all files and correct where needed.

#8 Updated by Tijs Wickardt almost 4 years 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 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.

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

Also available in: Atom PDF