Skip to content

Commit

Permalink
Fixed #80, where we no longer return stack traces for issues of the t…
Browse files Browse the repository at this point in the history
…ype: JavaObjectInaccessibleException, NonExistentJavaObjectException
  • Loading branch information
baubakg committed Mar 17, 2024
1 parent c472f4d commit d7272ed
Show file tree
Hide file tree
Showing 11 changed files with 109 additions and 36 deletions.
3 changes: 2 additions & 1 deletion ReleaseNotes.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
## 2.11.15 - SNAPSHOT
* [#71 Adding step name when throwing exceptions](https://github.com/adobe/bridgeService/issues/71). When an exception happens, include the step in which it occurred.
* [#72 Sending root stack trace](https://github.com/adobe/bridgeService/issues/72). With issue #9 we discovered that the stack trace should be that of the original cause.
* [#78 Trimming error messages](https://github.com/adobe/bridgeService/issues/78). We now trim the error messages.
* [#78 Trimming error messages](https://github.com/adobe/bridgeService/issues/78). We now trim the error messages.
* [#80 Removing stack trace for certain errors](https://github.com/adobe/bridgeService/issues/80). We removed the stack trace for errors, where the method or class cannot be found or accessed.


## 2.11.14
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ public class BridgeServiceFactory {
* @return A Java Call Object
*/
public static JavaCalls createJavaCalls(String in_requestJSON) throws JsonProcessingException {
LogManagement.logStep(LogManagement.STD_STEPS.ANALYZING_PAYLOAD);
ObjectMapper mapper = new ObjectMapper();
return mapper.readValue(in_requestJSON, JavaCalls.class);
}
Expand All @@ -31,6 +32,8 @@ public static JavaCalls createJavaCalls(String in_requestJSON) throws JsonProces
* @throws JsonProcessingException when failing to parse the JavaCallResults object
*/
public static String transformJavaCallResultsToJSON(JavaCallResults in_callResults) throws JsonProcessingException {
LogManagement.logStep(LogManagement.STD_STEPS.GENERATING_RESPONSE);

ObjectMapper mapper = new ObjectMapper();
return mapper.writeValueAsString(in_callResults);
}
Expand All @@ -41,6 +44,7 @@ public static String transformJavaCallResultsToJSON(JavaCallResults in_callResul
* @return A Service Access Object
*/
public static ServiceAccess createServiceAccess(String in_requestJSON) throws JsonProcessingException {
LogManagement.logStep(LogManagement.STD_STEPS.ANALYZING_PAYLOAD);
ObjectMapper mapper = new ObjectMapper();
Map<String, String> serviceMap = mapper.readValue(in_requestJSON, Map.class);
ServiceAccess lr_sa = new ServiceAccess();
Expand All @@ -56,6 +60,7 @@ public static ServiceAccess createServiceAccess(String in_requestJSON) throws Js
*/
public static String transformServiceAccessResult(Map<String, Boolean> in_callResults)
throws JsonProcessingException {
LogManagement.logStep(LogManagement.STD_STEPS.GENERATING_RESPONSE);
ObjectMapper mapper = new ObjectMapper();
return mapper.writeValueAsString(in_callResults);
}
Expand All @@ -78,8 +83,6 @@ public static String transformMapTosResult(Map<String, String> in_testPayLoad)
* @return A string that represents the error payload
*/
public static String createExceptionPayLoad(ErrorObject in_errorObject) {


return getErrorPayloadAsString(in_errorObject);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,7 @@
*/
package com.adobe.campaign.tests.bridge.service;

import com.adobe.campaign.tests.bridge.service.exceptions.AmbiguousMethodException;
import com.adobe.campaign.tests.bridge.service.exceptions.ClassLoaderConflictException;
import com.adobe.campaign.tests.bridge.service.exceptions.NonExistentJavaObjectException;
import com.adobe.campaign.tests.bridge.service.exceptions.TargetJavaMethodCallException;
import com.adobe.campaign.tests.bridge.service.exceptions.*;
import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonProperty;

Expand Down Expand Up @@ -173,7 +170,7 @@ public Object call(IntegroBridgeClassLoader iClassLoader) {
"The given method " + this.getFullName() + " could not accept the given arguments..");

} catch (IllegalAccessException e) {
throw new RuntimeException("We do not have the right to execute the given class. Original message : "+e.getMessage(), e);
throw new JavaObjectInaccessibleException("We do not have the right to execute the given class. Original message : "+e.getMessage(), e);
} catch (InvocationTargetException e) {
if (e.getCause() instanceof LinkageError) {
throw new ClassLoaderConflictException("Linkage Error detected. This can be corrected by enriching the "+ConfigValueHandlerIBS.STATIC_INTEGRITY_PACKAGES.systemName+" property", e.getCause());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public class ErrorObject {

private List<String> stackTrace;

public ErrorObject(Exception in_exception, String in_title, int in_errorCode) {
public ErrorObject(Exception in_exception, String in_title, int in_errorCode, boolean in_includeStackTrace) {
this.setTitle(in_title);
this.setCode(in_errorCode);
this.setDetail(in_exception.getMessage());
Expand All @@ -42,23 +42,27 @@ public ErrorObject(Exception in_exception, String in_title, int in_errorCode) {
this.setStackTrace(new ArrayList<>());

//Check if we have a different root cause
if (originalExceptionClass==null) {
this.setOriginalException(STD_NOT_APPLICABLE );
this.setOriginalMessage(STD_NOT_APPLICABLE);
Arrays.stream(in_exception.getStackTrace()).forEach(i -> this.stackTrace.add(i.toString()));
} else {
//Fetch the data from the root cause
this.setOriginalException(originalExceptionClass.getClass().getTypeName());
this.setOriginalMessage(originalExceptionClass.getMessage());
Arrays.stream(originalExceptionClass.getStackTrace()).forEach(i -> this.stackTrace.add(i.toString()));
this.setOriginalException((originalExceptionClass != null) ? originalExceptionClass.getClass()
.getTypeName() : STD_NOT_APPLICABLE);
this.setOriginalMessage(
(originalExceptionClass != null) ? originalExceptionClass.getMessage() : STD_NOT_APPLICABLE);

if (in_includeStackTrace) {
Arrays.stream(Optional.ofNullable(originalExceptionClass).orElse(in_exception).getStackTrace())
.forEach(i -> this.stackTrace.add(i.toString()));
}


}

public ErrorObject(Exception in_exception) {
this(in_exception, "Not Set", -1);
}

public ErrorObject(Exception in_exception, String in_title, int in_errorCode) {
this(in_exception, in_title, in_errorCode, true);
}

/**
* Extracts the exception originating the current one.
* @param in_exception The exception we want to parse
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ public class IntegroAPI {
protected static final String ERROR_IBS_RUNTIME = "Problems with payload.";
protected static final String ERROR_AMBIGUOUS_METHOD = "No unique method could be identified that matches your request.";
private static final Logger log = LogManager.getLogger();
protected static final String ERROR_JAVA_OBJECT_NOT_ACCESSIBLE = "The java object you want to call is inaccessible. This is very possibly a scope problem.";

public static void startServices(int port) {

Expand Down Expand Up @@ -94,7 +95,7 @@ public static void startServices(int port) {
res.status(statusCode);
res.type(ERROR_CONTENT_TYPE);
res.body(BridgeServiceFactory.createExceptionPayLoad(
new ErrorObject(e, ERROR_AMBIGUOUS_METHOD, statusCode)));
new ErrorObject(e, ERROR_AMBIGUOUS_METHOD, statusCode,false)));
});

exception(IBSConfigurationException.class, (e, req, res) -> {
Expand Down Expand Up @@ -124,14 +125,22 @@ public static void startServices(int port) {
res.status(statusCode);
res.type(ERROR_CONTENT_TYPE);
res.body(BridgeServiceFactory.createExceptionPayLoad(
new ErrorObject(e, ERROR_JAVA_OBJECT_NOT_FOUND, statusCode)));
new ErrorObject(e, ERROR_JAVA_OBJECT_NOT_FOUND, statusCode, false)));
});

exception(JavaObjectInaccessibleException.class, (e, req, res) -> {
int statusCode = 404;
res.status(statusCode);
res.type(ERROR_CONTENT_TYPE);
res.body(BridgeServiceFactory.createExceptionPayLoad(
new ErrorObject(e, ERROR_JAVA_OBJECT_NOT_ACCESSIBLE, statusCode, false)));
});

exception(IBSTimeOutException.class, (e, req, res) -> {
int statusCode = 408;
res.status(statusCode);
res.type(ERROR_CONTENT_TYPE);
res.body(BridgeServiceFactory.createExceptionPayLoad(new ErrorObject(e, ERROR_CALL_TIMEOUT, statusCode)));
res.body(BridgeServiceFactory.createExceptionPayLoad(new ErrorObject(e, ERROR_CALL_TIMEOUT, statusCode, false)));
});

//Internal exception
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@ public Object call(String in_key) {
throw new TargetJavaMethodCallException(e.getCause().getMessage(), e.getCause().getCause());
} else if (e.getCause() instanceof ClassLoaderConflictException) {
throw new IBSConfigurationException(e.getCause().getMessage(), e.getCause());
} else if (e.getCause() instanceof JavaObjectInaccessibleException) {
throw new JavaObjectInaccessibleException(e.getCause().getMessage(), e.getCause().getCause());

} else {
throw new IBSRunTimeException(e.getMessage());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,11 @@ public static String fetchCurrentStep() {
}

protected enum STD_STEPS {
ENVVARS("Setting Environment Variables"), SEND_RESULT("Returning result"), NOT_IN_A_STEP("Not in a Step");
ENVVARS("Setting Environment Variables"),
SEND_RESULT("Returning result"),
NOT_IN_A_STEP("Not in a Step"),
ANALYZING_PAYLOAD("Analyzing Payload"),
GENERATING_RESPONSE("Generating Response");

protected String value;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package com.adobe.campaign.tests.bridge.service.exceptions;

public class JavaObjectInaccessibleException extends RuntimeException {
private static final long serialVersionUID = -6618553477788381745L;

public JavaObjectInaccessibleException(String message, Throwable cause) {
super(message, cause);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,8 @@ public void testMainError_Case2AmbiguousMethodException() {
"We could not find a unique method for"))
.body("code", Matchers.equalTo(404))
.body("bridgeServiceException", Matchers.equalTo(AmbiguousMethodException.class.getTypeName()))
.body("originalException", Matchers.equalTo(ErrorObject.STD_NOT_APPLICABLE));
.body("originalException", Matchers.equalTo(ErrorObject.STD_NOT_APPLICABLE))
.body("stackTrace", Matchers.empty());
}

/**
Expand Down Expand Up @@ -228,7 +229,8 @@ public void testMainEror_Case4A_NonExistantJavaException() {
"The given class com.adobe.campaign.tests.bridgeservice.testdata.SimpleStaticMethodsNonExisting could not be found."))
.body("code", Matchers.equalTo(404))
.body("bridgeServiceException", Matchers.equalTo(NonExistentJavaObjectException.class.getTypeName()))
.body("originalException", Matchers.equalTo(ErrorObject.STD_NOT_APPLICABLE));
.body("originalException", Matchers.equalTo(ErrorObject.STD_NOT_APPLICABLE))
.body("stackTrace", Matchers.empty());

}

Expand Down Expand Up @@ -273,7 +275,8 @@ public void testMainEror_passingNull() {
+ "}";

given().body(l_jsonString).post(EndPointURL + "call").then().assertThat().statusCode(404)
.body("title", Matchers.equalTo(IntegroAPI.ERROR_JSON_TRANSFORMATION));
.body("title", Matchers.equalTo(IntegroAPI.ERROR_JSON_TRANSFORMATION))
.body("failureAtStep", Matchers.equalTo(LogManagement.STD_STEPS.ANALYZING_PAYLOAD.value));

}

Expand Down Expand Up @@ -559,10 +562,13 @@ public void test_issue35_callToClassWithNoModifiers() {
l_cc.setMethodName("hello");
jc.getCallContent().put("one", l_cc);

given().body(jc).post(EndPointURL + "call").then().assertThat().statusCode(500)
.body("title", Matchers.equalTo(IntegroAPI.ERROR_IBS_RUNTIME))
given().body(jc).post(EndPointURL + "call").then().assertThat().statusCode(404)
.body("title", Matchers.equalTo(IntegroAPI.ERROR_JAVA_OBJECT_NOT_ACCESSIBLE))
.body("bridgeServiceException", Matchers.equalTo(JavaObjectInaccessibleException.class.getTypeName()))
.body("detail", Matchers.startsWith(
"java.lang.RuntimeException: We do not have the right to execute the given class."));
"We do not have the right to execute the given class."))
.body("originalMessage", Matchers.startsWith(
"class com.adobe.campaign.tests.bridge.service.CallContent cannot access a member of class"));
}

@Test(groups = "E2E", enabled = false)
Expand Down Expand Up @@ -619,9 +625,23 @@ public void testInternalErrorCall() {

}

/**
* We run two paralle threads (as much as possible), and make sure the failed step is correct
*/
@Test(groups = "E2E")
public void testIBSInternalErrorCall() {

JavaCalls l_call = new JavaCalls();
CallContent myContent = new CallContent();
myContent.setClassName("com.adobe.campaign.tests.bridge.testdata.one.SimpleStaticMethods");
myContent.setMethodName("hkjdhghj");
myContent.setReturnType("java.lang.String");
l_call.getCallContent().put("call1PL", myContent);

given().body(l_call).post(EndPointURL + "call").then().assertThat().statusCode(404).body("title",
Matchers.equalTo(
"Could not find the given class or method."))
.body("stackTrace", Matchers.empty());

}

@Test(groups = "E2E")
public void testExternalErrorCall() {

Expand All @@ -644,6 +664,10 @@ public void testExternalErrorCall() {
}

//Testing Error Step detection

/**
* We run two parallel threads (as much as possible), and make sure the failed step is correct
*/
@Test(groups = "E2E")
public void correctlyDetectErrorSteps() {
//Method 1 throws exception
Expand Down Expand Up @@ -696,10 +720,14 @@ public void run() {
e.printStackTrace();
}

assertThat("We should be able to get the body", l_call1Result[0].get("title"), Matchers.equalTo(IntegroAPI.ERROR_IBS_RUNTIME));
assertThat("We should be able to get the body", l_call1Result[0].get("failureAtStep"), Matchers.equalTo("step1"));
assertThat("We should be able to get the body", l_call2Result[0].get("title"), Matchers.equalTo(IntegroAPI.ERROR_IBS_RUNTIME));
assertThat("We should be able to get the body", l_call2Result[0].get("failureAtStep"), Matchers.equalTo("step2"));
assertThat("We should be able to get the body", l_call1Result[0].get("title"),
Matchers.equalTo(IntegroAPI.ERROR_JAVA_OBJECT_NOT_ACCESSIBLE));
assertThat("We should be able to get the body", l_call1Result[0].get("failureAtStep"),
Matchers.equalTo("step1"));
assertThat("We should be able to get the body", l_call2Result[0].get("title"),
Matchers.equalTo(IntegroAPI.ERROR_JAVA_OBJECT_NOT_ACCESSIBLE));
assertThat("We should be able to get the body", l_call2Result[0].get("failureAtStep"),
Matchers.equalTo("step2"));
}

@AfterGroups(groups = "E2E", alwaysRun = true)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -242,4 +242,19 @@ public void testFactory() throws JsonProcessingException {
new ErrorObject(new ClassNotFoundException(), "A", 404)),
Matchers.equalTo("Problem creating error payload. Original error is " + "A"));
}

@Test
public void testDiscardingStackTraceInInternalIssues() {
String detail = "ABC";
ClassNotFoundException cnfe = new ClassNotFoundException(detail);
String message = "Highlevel message";
int errorCode = 404;

ErrorObject eo = new ErrorObject(cnfe, message, errorCode, false);
assertThat("We should not have a stracktrace", eo.getStackTrace().size(), Matchers.equalTo(0));

ErrorObject eo2 = new ErrorObject(cnfe, message, errorCode, true);
assertThat("We should not have a stracktrace", eo2.getStackTrace().size(), Matchers.greaterThan(0));

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1805,7 +1805,7 @@ public void test_issue35_callToClassWithNoModifiers() {
IllegalAccessException.class));
}
assertThat("We should have thrown an exception here", expectedException, Matchers.notNullValue());
Assert.assertThrows(IBSRunTimeException.class, () -> jc.submitCalls());
Assert.assertThrows(JavaObjectInaccessibleException.class, () -> jc.submitCalls());

}

Expand Down

0 comments on commit d7272ed

Please sign in to comment.