Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: stack trace of exceptions are not being logged #1974 #3157

Merged
merged 4 commits into from
Oct 11, 2020
Merged

Fix: stack trace of exceptions are not being logged #1974 #3157

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
15d1f83
throw RuntimeException instead of log.error() to make test fail
nitram509 Oct 9, 2020
c4ece01
add exception parameter to the log...() method calls
nitram509 Oct 9, 2020
7e058dd
fixed test-setup for sampleFunction3
nitram509 Oct 10, 2020
d9b2a55
make explicit import statements, as per convention for this project.
nitram509 Oct 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...mon/src/main/java/org/wso2/carbon/identity/application/common/model/IdentityProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ public static IdentityProvider build(OMElement identityProviderOM) {
} catch (IdentityApplicationManagementException e) {
log.error("Error while building provisioningConnectorConfig for IDP " + identityProvider
.getIdentityProviderName() + ". Cause : " + e.getMessage() + ". Building rest of the " +
"IDP configs");
"IDP configs", e);
}
if (proConConfig != null) {
provisioningConnectorConfigsArrList.add(proConConfig);
Expand Down
2 changes: 1 addition & 1 deletion ...va/org/wso2/carbon/identity/application/authentication/framework/util/FrameworkUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1865,7 +1865,7 @@ public static String getMultiAttributeSeparator() {
} catch (UserStoreException e) {
log.warn("Error while retrieving MultiAttributeSeparator from UserRealm.");
if (log.isDebugEnabled()) {
log.debug("Error while retrieving MultiAttributeSeparator from UserRealm." + e);
log.debug("Error while retrieving MultiAttributeSeparator from UserRealm.", e);
}
}

Expand Down
32 changes: 16 additions & 16 deletions .../carbon/identity/thrift/authentication/internal/ThriftAuthenticationServiceComponent.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
import org.osgi.service.component.annotations.ReferencePolicy;

@Component(
name = "org.wso2.carbon.identity.thrift.authentication.internal.ThriftAuthenticationServiceComponent",
name = "org.wso2.carbon.identity.thrift.authentication.internal.ThriftAuthenticationServiceComponent",
immediate = true)
public class ThriftAuthenticationServiceComponent {

Expand Down Expand Up @@ -130,10 +130,10 @@ public static void setRealmServiceInstance(RealmService realmServiceInstance) {
}

@Reference(
name = "http.service",
service = org.osgi.service.http.HttpService.class,
cardinality = ReferenceCardinality.MANDATORY,
policy = ReferencePolicy.DYNAMIC,
name = "http.service",
service = org.osgi.service.http.HttpService.class,
cardinality = ReferenceCardinality.MANDATORY,
policy = ReferencePolicy.DYNAMIC,
unbind = "unsetHttpService")
protected void setHttpService(HttpService httpService) {
setHttpServiceInstance(httpService);
Expand All @@ -144,10 +144,10 @@ protected void unsetHttpService(HttpService httpService) {
}

@Reference(
name = "org.wso2.carbon.user.core",
service = org.wso2.carbon.user.core.service.RealmService.class,
cardinality = ReferenceCardinality.MANDATORY,
policy = ReferencePolicy.DYNAMIC,
name = "org.wso2.carbon.user.core",
service = org.wso2.carbon.user.core.service.RealmService.class,
cardinality = ReferenceCardinality.MANDATORY,
policy = ReferencePolicy.DYNAMIC,
unbind = "unsetRealmService")
protected void setRealmService(RealmService realmService) {
setRealmServiceInstance(realmService);
Expand All @@ -158,10 +158,10 @@ protected void unsetRealmService(RealmService realmService) {
}

@Reference(
name = "configuration.context",
service = org.wso2.carbon.utils.ConfigurationContextService.class,
cardinality = ReferenceCardinality.MANDATORY,
policy = ReferencePolicy.DYNAMIC,
name = "configuration.context",
service = org.wso2.carbon.utils.ConfigurationContextService.class,
cardinality = ReferenceCardinality.MANDATORY,
policy = ReferencePolicy.DYNAMIC,
unbind = "unsetConfigurationContext")
protected void setConfigurationContext(ConfigurationContextService configurationContext) {
this.configurationContext = configurationContext;
Expand All @@ -184,9 +184,9 @@ private void startThriftHttpAuthenticatorService(ThriftAuthenticatorService thri
TCompactProtocol.Factory outProtFactory = new TCompactProtocol.Factory();
getHttpServiceInstance().registerServlet("/thriftAuthenticator", new AuthenticatorServlet(authServiceProcessor, inProtFactory, outProtFactory), new Hashtable(), getHttpServiceInstance().createDefaultHttpContext());
} catch (ServletException e) {
log.error("Unable to start Thrift Authenticator Service." + e);
log.error("Unable to start Thrift Authenticator Service.", e);
} catch (NamespaceException e) {
log.error("Unable to start Thrift Authenticator Service" + e);
log.error("Unable to start Thrift Authenticator Service", e);
}
}

Expand Down Expand Up @@ -237,7 +237,7 @@ private void startThriftTcpAuthenticatorService(ThriftAuthenticatorService thrif
clientTimeout = Integer.parseInt(clientTimeoutElement.getText());
} catch (Throwable e) {
String msg = "Error, in Thrift Auth Client Timeout, hence using the default timeout: " + ThriftAuthenticationConstants.DEFAULT_CLIENT_TIMEOUT + "ms";
log.error(msg + e);
log.error(msg, e);
clientTimeout = ThriftAuthenticationConstants.DEFAULT_CLIENT_TIMEOUT;
}
} else {
Expand Down
2 changes: 1 addition & 1 deletion ...ntity.entitlement/src/main/java/org/wso2/carbon/identity/entitlement/EntitlementUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ public static boolean validatePolicy(PolicyDTO policy) {
log.error("Invalid Namespace in policy");
}
} catch (SAXException e) {
log.error("XACML policy is not valid according to the schema :" + e.getMessage());
log.error("XACML policy is not valid according to the schema :" + e.getMessage(), e);
} catch (IOException e) {
//ignore
} catch (ParserConfigurationException e) {
Expand Down
8 changes: 4 additions & 4 deletions ...tlement/src/main/java/org/wso2/carbon/identity/entitlement/pip/CarbonAttributeFinder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,29 +212,29 @@ public EvaluationResult findAttribute(URI attributeType, URI attributeId, String
}
}
} catch (ParsingException e) {
log.error("Error while parsing attribute values from EvaluationCtx : " + e);
log.error("Error while parsing attribute values from EvaluationCtx : ", e);
ArrayList<String> code = new ArrayList<String>();
code.add(Status.STATUS_MISSING_ATTRIBUTE);
Status status = new Status(code,
"Error while parsing attribute values from EvaluationCtx : " + e.getMessage());
return new EvaluationResult(status);
} catch (ParseException e) {
e.printStackTrace();
log.error("Error while parsing attribute values from EvaluationCtx : " + e);
log.error("Error while parsing attribute values from EvaluationCtx : ", e);
ArrayList<String> code = new ArrayList<String>();
code.add(Status.STATUS_MISSING_ATTRIBUTE);
Status status = new Status(code,
"Error while parsing attribute values from EvaluationCtx : " + e.getMessage());
return new EvaluationResult(status);
} catch (URISyntaxException e) {
log.error("Error while parsing attribute values from EvaluationCtx : " + e);
log.error("Error while parsing attribute values from EvaluationCtx : ", e);
ArrayList<String> code = new ArrayList<String>();
code.add(Status.STATUS_MISSING_ATTRIBUTE);
Status status = new Status(code,
"Error while parsing attribute values from EvaluationCtx :" + e.getMessage());
return new EvaluationResult(status);
} catch (Exception e) {
log.error("Error while retrieving attribute values from PIP attribute finder : " + e);
log.error("Error while retrieving attribute values from PIP attribute finder : ", e);
ArrayList<String> code = new ArrayList<String>();
code.add(Status.STATUS_MISSING_ATTRIBUTE);
Status status = new Status(code, "Error while retrieving attribute values from PIP"
Expand Down
2 changes: 1 addition & 1 deletion ...org/wso2/carbon/identity/functions/library/mgt/FunctionLibraryManagementAdminService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ private void evaluateScript(FunctionLibrary functionLibrary) throws FunctionLibr
engine.eval(code);
} catch (ScriptException e) {
log.error("Script library script of " + functionLibrary.getFunctionLibraryName() +
" contains errors." + e);
" contains errors.", e);
throw FunctionLibraryExceptionManagementUtil.handleClientException(
FunctionLibraryManagementConstants.ErrorMessage.ERROR_CODE_VALIDATE_SCRIPT_LIBRARY_SCRIPT,
functionLibrary.getFunctionLibraryName(), e);
Expand Down
2 changes: 1 addition & 1 deletion .../org/wso2/carbon/identity/functions/library/mgt/FunctionLibraryManagementServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ private void evaluateScript(FunctionLibrary functionLibrary) throws FunctionLibr
engine.eval(code);
} catch (ScriptException e) {
log.error("Script library script of " + functionLibrary.getFunctionLibraryName() +
" contains errors." + e);
" contains errors.", e);
throw FunctionLibraryExceptionManagementUtil.handleClientException(
FunctionLibraryManagementConstants.ErrorMessage.ERROR_CODE_VALIDATE_SCRIPT_LIBRARY_SCRIPT,
functionLibrary.getFunctionLibraryName(), e);
Expand Down
21 changes: 8 additions & 13 deletions .../org/wso2/carbon/identity/functions/library/mgt/FunctionLibraryManagementServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@

package org.wso2.carbon.identity.functions.library.mgt;

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.powermock.api.mockito.PowerMockito;
import org.powermock.core.classloader.annotations.PrepareForTest;
import org.testng.annotations.DataProvider;
Expand All @@ -34,17 +32,14 @@
import java.util.List;

import static org.powermock.api.mockito.PowerMockito.when;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertNotNull;
import static org.testng.Assert.assertTrue;
import static org.testng.Assert.*;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a practice, we avoid * imports. So, please revert this back to individual imports

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pulasthi7 Thank you for your review.
I did change the import as per convention.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @nitram509

import static org.wso2.carbon.identity.functions.library.mgt.FunctionLibraryMgtUtil.isRegexValidated;

@PrepareForTest({FunctionLibraryManagementServiceImpl.class})
public class FunctionLibraryManagementServiceTest extends PowerMockIdentityBaseTest {

private static final String SAMPLE_TENANT_DOMAIN = "carbon.super";
private static final String SAMPLE_TENANT_DOMAIN2 = "abc.com";
private static final Log log = LogFactory.getLog(FunctionLibraryManagementServiceTest.class);

@DataProvider(name = "createFunctionLibraryDataProvider")
public Object[][] createFunctionLibraryData() {
Expand All @@ -62,7 +57,7 @@ public Object[][] createFunctionLibraryData() {
FunctionLibrary functionLibrary3 = new FunctionLibrary();
functionLibrary3.setFunctionLibraryName("sample3");
functionLibrary3.setDescription("sample3");
functionLibrary3.setFunctionLibraryScript("samplefunction3");
functionLibrary3.setFunctionLibraryScript("function samplefunction3(){}");

FunctionLibrary functionLibrary4 = new FunctionLibrary();
functionLibrary4.setFunctionLibraryName("sample4");
Expand Down Expand Up @@ -120,7 +115,7 @@ public void createFunctionLibrary(Object functionLibrary, String tenantDomain) {
functionLibraryManagementService.deleteFunctionLibrary(
((FunctionLibrary) functionLibrary).getFunctionLibraryName(), tenantDomain);
} catch (Exception e) {
log.error("Error test Create script library " + e);
fail("Error test Create script library ", e);
}
}

Expand Down Expand Up @@ -174,7 +169,7 @@ public void getFunctionLibrary(Object functionLibrary, String tenantDomain) {
deleteFunctionLibraries(functionLibraryManagementService, Collections.singletonList(functionLibrary),
tenantDomain);
} catch (Exception e) {
log.error("Exception");
fail("Exception", e);
}
}

Expand Down Expand Up @@ -326,7 +321,7 @@ public void updateFunctionLibrary(Object functionLibrary, String tenantDomain) t
FunctionLibraryMgtUtil.FUNCTION_LIBRARY_NAME_VALIDATING_REGEX + ".");
}
} catch (Exception e) {
log.error("Exception");
fail("Exception", e);
}
}

Expand Down Expand Up @@ -404,7 +399,7 @@ public void testIsRegexValidated(Object functionLibrary, String tenantDomain) {
assertEquals(e.getMessage(), "The script library name is not valid! It is not adhering to the regex " +
FunctionLibraryMgtUtil.FUNCTION_LIBRARY_NAME_VALIDATING_REGEX + ".");
} catch (Exception e) {
log.error("Exception" + e);
fail("Exception", e);
}
}

Expand Down Expand Up @@ -452,7 +447,7 @@ public void testAlreadyExistFunctionLibrary(Object functionLibrary, String tenan
assertEquals(e.getMessage(), "Already a script library available with the name: " +
((FunctionLibrary) functionLibrary).getFunctionLibraryName() + ".");
} catch (Exception e) {
log.error("Exception" + e);
fail("Exception", e);
}
}

Expand Down Expand Up @@ -486,7 +481,7 @@ public void testFunctionLibraryNameRequired(Object functionLibrary, String tenan
} catch (FunctionLibraryManagementException e) {
assertEquals(e.getMessage(), "Script library name is required");
} catch (Exception e) {
log.error("Exception" + e);
fail("Exception", e);
}
}

Expand Down
6 changes: 3 additions & 3 deletions ...2.carbon.identity.base/src/main/java/org/wso2/carbon/identity/base/IdentityException.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public static <T extends IdentityException> T error(Class<T> exceptionClass, Str
try {
exception = exceptionClass.getConstructor(String.class, String.class).newInstance(errorCode, message);
} catch (Exception e) {
throw new IdentityRuntimeException("Invalid Exception Type, " + e.getMessage());
throw new IdentityRuntimeException("Invalid Exception Type, " + e.getMessage(), e);
}
return exception;
}
Expand All @@ -87,7 +87,7 @@ public static <T extends IdentityException> T error(Class<T> exceptionClass, Str
try {
exception = exceptionClass.getConstructor(String.class, Throwable.class).newInstance(message, cause);
} catch (Exception e) {
throw new IdentityRuntimeException("Invalid Exception Type, " + e.getMessage());
throw new IdentityRuntimeException("Invalid Exception Type, " + e.getMessage(), e);
}
return exception;
}
Expand All @@ -99,7 +99,7 @@ public static <T extends IdentityException> T error(Class<T> exceptionClass, Str
exception = exceptionClass.getConstructor(String.class, String.class, Throwable.class).
newInstance(errorCode, message, cause);
} catch (Exception e) {
throw new IdentityRuntimeException("Invalid Exception Type, " + e.getMessage());
throw new IdentityRuntimeException("Invalid Exception Type, " + e.getMessage(), e);
}
return exception;
}
Expand Down
8 changes: 4 additions & 4 deletions ...n.identity.base/src/main/java/org/wso2/carbon/identity/base/IdentityRuntimeException.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public static <T extends IdentityRuntimeException> T error(Class<T> exceptionCla
try {
exception = exceptionClass.getConstructor(String.class).newInstance(message);
} catch (Exception e) {
throw new IdentityRuntimeException("Invalid Exception Type, " + e.getMessage());
throw new IdentityRuntimeException("Invalid Exception Type, " + e.getMessage(), e);
}
return exception;
}
Expand All @@ -83,7 +83,7 @@ public static <T extends IdentityRuntimeException> T error(Class<T> exceptionCla
try {
exception = exceptionClass.getConstructor(String.class, String.class).newInstance(errorCode, message);
} catch (Exception e) {
throw new IdentityRuntimeException("Invalid Exception Type, " + e.getMessage());
throw new IdentityRuntimeException("Invalid Exception Type, " + e.getMessage(), e);
}
return exception;
}
Expand All @@ -93,7 +93,7 @@ public static <T extends IdentityRuntimeException> T error(Class<T> exceptionCla
try {
exception = exceptionClass.getConstructor(String.class, Throwable.class).newInstance(message, cause);
} catch (Exception e) {
throw new IdentityRuntimeException("Invalid Exception Type, " + e.getMessage());
throw new IdentityRuntimeException("Invalid Exception Type, " + e.getMessage(), e);
}
return exception;
}
Expand All @@ -105,7 +105,7 @@ public static <T extends IdentityRuntimeException> T error(Class<T> exceptionCla
exception = exceptionClass.getConstructor(String.class, String.class, Throwable.class).
newInstance(errorCode, message, cause);
} catch (Exception e) {
throw new IdentityRuntimeException("Invalid Exception Type, " + e.getMessage());
throw new IdentityRuntimeException("Invalid Exception Type, " + e.getMessage(), e);
}
return exception;
}
Expand Down
8 changes: 4 additions & 4 deletions ...2.carbon.identity.core/src/main/java/org/wso2/carbon/identity/core/util/IdentityUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
/*
* Copyright (c) 2005-2010, WSO2 Inc. (http://www.wso2.org) All Rights Reserved.
*
*
* WSO2 Inc. licenses this file to you under the Apache License,
* Version 2.0 (the "License"); you may not use this file except
* in compliance with the License.
* You may obtain a copy of the License at
*
*
* http://www.apache.org/licenses/LICENSE-2.0
*
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
Expand Down Expand Up @@ -332,7 +332,7 @@ public static String getHMAC(String secretKey, String baseString) throws Signatu
byte[] rawHmac = mac.doFinal(baseString.getBytes());
return Base64.getEncoder().encodeToString(rawHmac);
} catch (Exception e) {
throw new SignatureException("Failed to generate HMAC : " + e.getMessage());
throw new SignatureException("Failed to generate HMAC : " + e.getMessage(), e);
}
}

Expand Down
4 changes: 2 additions & 2 deletions ...n.identity.mgt/src/main/java/org/wso2/carbon/identity/mgt/store/ArtifactDeleteThread.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ private void deleteOldResourcesIfFound() {
collection = (Collection) registry.get(secretKeyPath.toLowerCase());
} catch (RegistryException e) {
log.error(
"Error while deleting the old confirmation code. Unable to find data collection in registry." + e);
"Error while deleting the old confirmation code. Unable to find data collection in registry.", e);
}

//Introduced property to fix resource not being introduced deleted when special characters are present.
Expand Down Expand Up @@ -129,7 +129,7 @@ public int compare(Resource r1, Resource r2) {
}
}
} catch (RegistryException e) {
log.error("Error while deleting the old confirmation code \n" + e);
log.error("Error while deleting the old confirmation code", e);
} finally {
PrivilegedCarbonContext.endTenantFlow();
}
Expand Down
2 changes: 1 addition & 1 deletion ...t/src/main/java/org/wso2/carbon/identity/workflow/mgt/WorkflowManagementAdminService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ private WorkflowWizard getWorkflow(org.wso2.carbon.identity.workflow.mgt.bean.Wo
} catch (InternalWorkflowException e) {
String errorMsg =
"Error occurred while reading workflow object details for given workflow id, " + e.getMessage();
log.error(errorMsg);
log.error(errorMsg, e);
throw new WorkflowException(errorMsg, e);
}

Expand Down
2 changes: 1 addition & 1 deletion .../java/org/wso2/carbon/identity/workflow/mgt/extension/AbstractWorkflowRequestHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ public boolean isAssociated() throws WorkflowException{
eventEngaged = WorkflowServiceDataHolder.getInstance().getWorkflowService().isEventAssociated(getEventId());
} catch (InternalWorkflowException e) {
String errorMsg = "Error occurred while checking any association for this event, " + e.getMessage() ;
log.error(errorMsg);
log.error(errorMsg, e);
throw new WorkflowException(errorMsg,e);
}
return eventEngaged ;
Expand Down
2 changes: 1 addition & 1 deletion ...ow.mgt/src/main/java/org/wso2/carbon/identity/workflow/mgt/template/AbstractTemplate.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public AbstractTemplate(String metaDataXML) throws WorkflowRuntimeException {
this.parametersMetaData = this.metaData.getTemplate().getParametersMetaData();
} catch (JAXBException e) {
String errorMsg = "Error occured while converting template parameter data to object : " + e.getMessage();
log.error(errorMsg);
log.error(errorMsg, e);
throw new WorkflowRuntimeException(errorMsg, e);
}
}
Expand Down
Loading