Skip to content

Commit

Permalink
Fixed: Fix Default or Empty Catch block in Java and Groovy files
Browse files Browse the repository at this point in the history
(OFBIZ-8341)

In many Java and Groovy files we have auto generated catch blocks or empty catch 
blocks. 
To avoid such exception swallowing this should be improved to at least log the 
error and also return error in case of service.

Last ones :)

git-svn-id: https://svn.apache.org/repos/asf/ofbiz/ofbiz-framework/trunk@1866958 13f79535-47bb-0310-9956-ffa450edef68
  • Loading branch information
JacquesLeRoux committed Sep 15, 2019
1 parent 28dfdf5 commit cfff42c
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,9 @@ if (action) {
if (prodsEli != null) {
try {
prodsEli.close()
} catch (Exception exc) {}
} catch (Exception exc) {
Debug.logError(exception, module);
}
}
// only commit the transaction if we started one... this will throw an exception if it fails
TransactionUtil.commit(beganTransaction)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,13 @@
import java.util.List;
import java.util.Map;

import org.apache.ofbiz.base.util.Assert;
import org.junit.Test;

/**
* Assert tests {@link org.apache.ofbiz.base.util.Assert}.
*/
public class AssertTests {
public static final String module = AssertTests.class.getName();

@Test
public void testAssert(){
Expand All @@ -44,7 +44,10 @@ public void testAssert(){
try {
Assert.notNull("foo", null);
fail("notNull - IllegalArgumentException not thrown");
} catch (IllegalArgumentException e) {}
} catch (IllegalArgumentException e) {
Debug.logError(e, module);
}


//-----------------------------------------------------------------------
try {
Expand All @@ -55,7 +58,10 @@ public void testAssert(){
try {
Assert.notNull("foo", testObject, "bar", null);
fail("notNull (argument list) - IllegalArgumentException not thrown");
} catch (IllegalArgumentException e) {}
} catch (IllegalArgumentException e) {
Debug.logError(e, module);
}


//-----------------------------------------------------------------------
try {
Expand All @@ -66,7 +72,10 @@ public void testAssert(){
try {
Assert.notEmpty("foo", "");
fail("notEmpty(String) - IllegalArgumentException not thrown");
} catch (IllegalArgumentException e) {}
} catch (IllegalArgumentException e) {
Debug.logError(e, module);
}


//-----------------------------------------------------------------------
String[] strArray = {"foo", "bar"};
Expand All @@ -78,14 +87,20 @@ public void testAssert(){
try {
Assert.notEmpty("foo", new String[0]);
fail("notEmpty(Array) - IllegalArgumentException not thrown");
} catch (IllegalArgumentException e) {}
} catch (IllegalArgumentException e) {
Debug.logError(e, module);
}


//-----------------------------------------------------------------------
List<String> strList = new ArrayList<>();
try {
Assert.notEmpty("foo", strList);
fail("notEmpty(Collection) - IllegalArgumentException not thrown");
} catch (IllegalArgumentException e) {}
} catch (IllegalArgumentException e) {
Debug.logError(e, module);
}

strList.add("foo");
try {
Assert.notEmpty("foo", strList);
Expand All @@ -98,7 +113,10 @@ public void testAssert(){
try {
Assert.notEmpty("foo", strMap);
fail("notEmpty(Map) - IllegalArgumentException not thrown");
} catch (IllegalArgumentException e) {}
} catch (IllegalArgumentException e) {
Debug.logError(e, module);
}

strMap.put("foo", "foo");
try {
Assert.notEmpty("foo", strMap);
Expand All @@ -115,7 +133,10 @@ public void testAssert(){
try {
Assert.isInstanceOf("foo", strMap, AssertTests.class);
fail("isInstanceOf - IllegalArgumentException not thrown");
} catch (IllegalArgumentException e) {}
} catch (IllegalArgumentException e) {
Debug.logError(e, module);
}


//-----------------------------------------------------------------------
try {
Expand All @@ -126,7 +147,10 @@ public void testAssert(){
try {
Assert.isInstanceOf("foo", strMap, String.class, AssertTests.class);
fail("isInstanceOf (argument list) - IllegalArgumentException not thrown");
} catch (IllegalArgumentException e) {}
} catch (IllegalArgumentException e) {
Debug.logError(e, module);
}


//-----------------------------------------------------------------------
try {
Expand All @@ -137,7 +161,10 @@ public void testAssert(){
try {
Assert.isNotInstanceOf("foo", strMap, Map.class);
fail("isNotInstanceOf - IllegalArgumentException not thrown");
} catch (IllegalArgumentException e) {}
} catch (IllegalArgumentException e) {
Debug.logError(e, module);
}


//-----------------------------------------------------------------------
try {
Expand All @@ -148,7 +175,10 @@ public void testAssert(){
try {
Assert.isNotInstanceOf("foo", strMap, String.class, Map.class);
fail("isNotInstanceOf (argument list) - IllegalArgumentException not thrown");
} catch (IllegalArgumentException e) {}
} catch (IllegalArgumentException e) {
Debug.logError(e, module);
}


//-----------------------------------------------------------------------
try {
Expand All @@ -159,6 +189,9 @@ public void testAssert(){
try {
Assert.isAssignableTo("foo", strArray, Map[].class);
fail("isNotInstanceOf (argument list) - IllegalArgumentException not thrown");
} catch (IllegalArgumentException e) {}
} catch (IllegalArgumentException e) {
Debug.logError(e, module);
}

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1275,7 +1275,9 @@ public void testOneBigTransactionIsFasterThanSeveralSmallOnes() {
} catch (GenericEntityException e) {
try {
TransactionUtil.rollback(transactionStarted, "", e);
} catch (Exception e2) {}
} catch (Exception e2) {
Debug.logError(e2, module);
}
noErrors = false;
}
endTime = System.currentTimeMillis();
Expand Down

0 comments on commit cfff42c

Please sign in to comment.