feat: make Mongodb plugin error messages more readable. (#7857)

* Make MongoDB plugin's error messages more readable. A Client side change is required before this change becomes visible to the end user.
This commit is contained in:
Sumit Kumar 2021-09-27 20:40:14 +05:30 committed by GitHub
parent b5488b01ca
commit 611ff3b776
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 283 additions and 18 deletions

View File

@ -8,11 +8,17 @@ import lombok.Setter;
@Getter
@Setter
public class AppsmithPluginException extends BaseException {
private final Throwable externalError;
private final AppsmithPluginError error;
private final Object[] args;
public AppsmithPluginException(AppsmithPluginError error, Object... args) {
this(null, error, args);
}
public AppsmithPluginException(Throwable externalError, AppsmithPluginError error, Object... args) {
super(error.getMessage(args));
this.externalError = externalError;
this.error = error;
this.args = args;
}

View File

@ -2,6 +2,7 @@ package com.appsmith.external.models;
import com.appsmith.external.exceptions.BaseException;
import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginException;
import com.appsmith.external.plugins.AppsmithPluginErrorUtils;
import com.fasterxml.jackson.databind.JsonNode;
import lombok.Getter;
import lombok.NoArgsConstructor;
@ -22,6 +23,7 @@ public class ActionExecutionResult {
String errorType;
JsonNode headers;
Object body;
String readableError;
Boolean isExecutionSuccess = false;
/*
@ -36,16 +38,24 @@ public class ActionExecutionResult {
List<WidgetSuggestionDTO> suggestedWidgets;
public void setErrorInfo(Throwable error) {
public void setErrorInfo(Throwable error, AppsmithPluginErrorUtils pluginErrorUtils) {
this.body = error.getMessage();
if (error instanceof AppsmithPluginException) {
this.statusCode = ((AppsmithPluginException) error).getAppErrorCode().toString();
this.title = ((AppsmithPluginException) error).getTitle();
this.errorType = ((AppsmithPluginException) error).getErrorType();
if (((AppsmithPluginException) error).getExternalError() != null && pluginErrorUtils != null) {
this.readableError = pluginErrorUtils.getReadableError(error);
}
} else if (error instanceof BaseException) {
this.statusCode = ((BaseException) error).getAppErrorCode().toString();
this.title = ((BaseException) error).getTitle();
}
}
public void setErrorInfo(Throwable error) {
this.setErrorInfo(error, null);
}
}

View File

@ -0,0 +1,13 @@
package com.appsmith.external.plugins;
/**
* Defines a common interface that plugin agnostic code flow can use to work with plugin related error objects. Each
* plugin must override the common methods to provide plugin specific functionality. One use case is to extract
* human-readable strings from otherwise huge error messages. For this use case, each plugin must override
* `getReadableError` method to define its own way of extracting the readable error message.
*/
public abstract class AppsmithPluginErrorUtils {
public String getReadableError(Throwable error) {
return error.getMessage();
}
}

View File

@ -24,6 +24,7 @@ import com.appsmith.external.models.SSLDetails;
import com.appsmith.external.plugins.BasePlugin;
import com.appsmith.external.plugins.PluginExecutor;
import com.appsmith.external.plugins.SmartSubstitutionInterface;
import com.external.plugins.utils.MongoErrorUtils;
import com.fasterxml.jackson.databind.node.ArrayNode;
import com.fasterxml.jackson.databind.node.ObjectNode;
import com.mongodb.MongoCommandException;
@ -65,13 +66,13 @@ import java.util.regex.Pattern;
import java.util.stream.Collectors;
import static com.appsmith.external.constants.ActionConstants.ACTION_CONFIGURATION_BODY;
import static com.external.plugins.MongoPluginUtils.convertMongoFormInputToRawCommand;
import static com.external.plugins.MongoPluginUtils.generateTemplatesAndStructureForACollection;
import static com.external.plugins.MongoPluginUtils.getDatabaseName;
import static com.external.plugins.utils.MongoPluginUtils.convertMongoFormInputToRawCommand;
import static com.external.plugins.utils.MongoPluginUtils.generateTemplatesAndStructureForACollection;
import static com.external.plugins.utils.MongoPluginUtils.getDatabaseName;
import static com.appsmith.external.helpers.PluginUtils.getValueSafelyFromFormData;
import static com.external.plugins.MongoPluginUtils.isRawCommand;
import static com.external.plugins.utils.MongoPluginUtils.isRawCommand;
import static com.appsmith.external.helpers.PluginUtils.setValueSafelyInFormData;
import static com.external.plugins.MongoPluginUtils.urlEncode;
import static com.external.plugins.utils.MongoPluginUtils.urlEncode;
import static com.appsmith.external.helpers.PluginUtils.validConfigurationPresentInFormData;
import static com.external.plugins.constants.FieldName.AGGREGATE_PIPELINE;
import static com.external.plugins.constants.FieldName.COUNT_QUERY;
@ -156,6 +157,8 @@ public class MongoPlugin extends BasePlugin {
UPDATE_OPERATION
));
private static final MongoErrorUtils mongoErrorUtils = MongoErrorUtils.getInstance();
public MongoPlugin(PluginWrapper wrapper) {
super(wrapper);
}
@ -276,6 +279,7 @@ public class MongoPlugin extends BasePlugin {
.onErrorMap(
MongoCommandException.class,
error -> new AppsmithPluginException(
error,
AppsmithPluginError.PLUGIN_EXECUTE_ARGUMENT_ERROR,
error.getErrorMessage()
)
@ -383,7 +387,7 @@ public class MongoPlugin extends BasePlugin {
}
ActionExecutionResult actionExecutionResult = new ActionExecutionResult();
actionExecutionResult.setIsExecutionSuccess(false);
actionExecutionResult.setErrorInfo(error);
actionExecutionResult.setErrorInfo(error, mongoErrorUtils);
return Mono.just(actionExecutionResult);
})
// Now set the request in the result to be returned back to the server
@ -804,7 +808,7 @@ public class MongoPlugin extends BasePlugin {
return Mono.just(new DatasourceTestResult());
}
return Mono.just(new DatasourceTestResult(error.getMessage()));
return Mono.just(new DatasourceTestResult(mongoErrorUtils.getReadableError(error)));
})
.subscribeOn(scheduler);
}

View File

@ -16,7 +16,7 @@ import java.util.ArrayList;
import java.util.Map;
import static com.appsmith.external.helpers.PluginUtils.getValueSafelyFromFormData;
import static com.external.plugins.MongoPluginUtils.parseSafely;
import static com.external.plugins.utils.MongoPluginUtils.parseSafely;
import static com.appsmith.external.helpers.PluginUtils.validConfigurationPresentInFormData;
import static com.external.plugins.constants.FieldName.AGGREGATE_PIPELINE;

View File

@ -9,7 +9,7 @@ import org.pf4j.util.StringUtils;
import java.util.Map;
import static com.appsmith.external.helpers.PluginUtils.getValueSafelyFromFormData;
import static com.external.plugins.MongoPluginUtils.parseSafely;
import static com.external.plugins.utils.MongoPluginUtils.parseSafely;
import static com.appsmith.external.helpers.PluginUtils.validConfigurationPresentInFormData;
import static com.external.plugins.constants.FieldName.COUNT_QUERY;

View File

@ -15,7 +15,7 @@ import java.util.List;
import java.util.Map;
import static com.appsmith.external.helpers.PluginUtils.getValueSafelyFromFormData;
import static com.external.plugins.MongoPluginUtils.parseSafely;
import static com.external.plugins.utils.MongoPluginUtils.parseSafely;
import static com.appsmith.external.helpers.PluginUtils.setValueSafelyInFormData;
import static com.appsmith.external.helpers.PluginUtils.validConfigurationPresentInFormData;
import static com.external.plugins.constants.FieldName.COLLECTION;

View File

@ -10,7 +10,7 @@ import org.pf4j.util.StringUtils;
import java.util.Map;
import static com.appsmith.external.helpers.PluginUtils.getValueSafelyFromFormData;
import static com.external.plugins.MongoPluginUtils.parseSafely;
import static com.external.plugins.utils.MongoPluginUtils.parseSafely;
import static com.appsmith.external.helpers.PluginUtils.validConfigurationPresentInFormData;
import static com.external.plugins.constants.FieldName.DISTINCT_QUERY;

View File

@ -14,7 +14,7 @@ import java.util.List;
import java.util.Map;
import static com.appsmith.external.helpers.PluginUtils.getValueSafelyFromFormData;
import static com.external.plugins.MongoPluginUtils.parseSafely;
import static com.external.plugins.utils.MongoPluginUtils.parseSafely;
import static com.appsmith.external.helpers.PluginUtils.setValueSafelyInFormData;
import static com.appsmith.external.helpers.PluginUtils.validConfigurationPresentInFormData;
import static com.external.plugins.constants.FieldName.COLLECTION;

View File

@ -22,7 +22,7 @@ import java.util.Map;
import java.util.stream.Collectors;
import static com.appsmith.external.helpers.PluginUtils.getValueSafelyFromFormData;
import static com.external.plugins.MongoPluginUtils.parseSafely;
import static com.external.plugins.utils.MongoPluginUtils.parseSafely;
import static com.appsmith.external.helpers.PluginUtils.setValueSafelyInFormData;
import static com.appsmith.external.helpers.PluginUtils.validConfigurationPresentInFormData;
import static com.external.plugins.constants.FieldName.COLLECTION;

View File

@ -15,7 +15,7 @@ import java.util.List;
import java.util.Map;
import static com.appsmith.external.helpers.PluginUtils.getValueSafelyFromFormData;
import static com.external.plugins.MongoPluginUtils.parseSafely;
import static com.external.plugins.utils.MongoPluginUtils.parseSafely;
import static com.appsmith.external.helpers.PluginUtils.setValueSafelyInFormData;
import static com.appsmith.external.helpers.PluginUtils.validConfigurationPresentInFormData;
import static com.external.plugins.constants.FieldName.COLLECTION;

View File

@ -0,0 +1,107 @@
package com.external.plugins.utils;
import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginException;
import com.appsmith.external.plugins.AppsmithPluginErrorUtils;
import com.mongodb.MongoCommandException;
import com.mongodb.MongoSecurityException;
import lombok.AccessLevel;
import lombok.NoArgsConstructor;
@NoArgsConstructor(access = AccessLevel.PRIVATE)
public class MongoErrorUtils extends AppsmithPluginErrorUtils {
private static MongoErrorUtils mongoErrorUtils;
public static MongoErrorUtils getInstance() {
if (mongoErrorUtils == null) {
mongoErrorUtils = new MongoErrorUtils();
}
return mongoErrorUtils;
}
/**
* Extract small readable portion of error message from a larger less comprehensible error message.
* @param error - any error object
* @return readable error message
*/
@Override
public String getReadableError(Throwable error) {
Throwable externalError;
// If the external error is wrapped inside Appsmith error, then extract the external error first.
if (error instanceof AppsmithPluginException) {
if (((AppsmithPluginException) error).getExternalError() == null) {
return error.getMessage();
}
externalError = ((AppsmithPluginException) error).getExternalError();
}
else {
externalError = error;
}
if (externalError instanceof MongoCommandException) {
MongoCommandException mongoCommandError = (MongoCommandException) externalError;
int errorCode = mongoCommandError.getCode();
// Error codes ref: https://github.com/mongodb/mongo/blob/50dc6dbe394c42d03659aa3410954f1e3ff46740/src/mongo/base/error_codes.err#L12
switch (errorCode) {
case 9: // FailedToParse error
/**
* Sample external error message:
* Failed to parse: { find: "newAction", limit: [ 10 ], $db: "mobtools", ... }. 'limit' field must
* be numeric.
*
* Return string: 'limit' field must be numeric.
*/
return getLast(mongoCommandError.getErrorMessage().split("\\.")).trim() + ".";
default:
/**
* Sample external error message:
* Error getting filter : Expected 'filter' to be BSON (or equivalent), but got string instead.
* Doc = [{find newAction} {filter filterx} {limit 10} {$db mobtools} ...]
*
* Return string: Error getting filter : Expected 'filter' to be BSON (or equivalent), but got
* string instead.
*/
return mongoCommandError.getErrorMessage().split("\\.")[0].trim() + ".";
}
}
else if (externalError instanceof MongoSecurityException) {
MongoSecurityException mongoSecurityError = (MongoSecurityException) externalError;
int errorCode = mongoSecurityError.getCode();
switch (errorCode) {
default:
/**
* Sample external error message:
* Exception authenticating MongoCredential{mechanism=SCRAM-SHA-1, userName='username',
* source='admin', password=<hidden>, mechanismProperties=<hidden>}
*
* Return string: Exception authenticating MongoCredential.
*/
return mongoSecurityError.getMessage().split("\\{")[0].trim() + ".";
}
}
/**
* Sample external error message:
* Error getting filter : Expected 'filter' to be BSON (or equivalent), but got string instead.
* Doc = [{find newAction} {filter filterx} {limit 10} {$db mobtools} ...]
*
* Return string: Error getting filter : Expected 'filter' to be BSON (or equivalent), but got
* string instead.
*/
return error.getMessage().split("\\.")[0].trim() + ".";
}
// Get last element from array.
private String getLast(String[] messageArray) {
if (messageArray.length == 0) {
return "";
}
return messageArray[messageArray.length - 1];
}
}

View File

@ -1,4 +1,4 @@
package com.external.plugins;
package com.external.plugins.utils;
import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginError;
import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginException;

View File

@ -21,6 +21,7 @@ import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.node.ArrayNode;
import com.fasterxml.jackson.databind.node.ObjectNode;
import com.mongodb.MongoCommandException;
import com.mongodb.MongoSecurityException;
import com.mongodb.reactivestreams.client.MongoClient;
import com.mongodb.reactivestreams.client.MongoClients;
import com.mongodb.reactivestreams.client.MongoCollection;
@ -87,14 +88,13 @@ public class MongoPluginTest {
private static String address;
private static Integer port;
private JsonNode value;
@SuppressWarnings("rawtypes")
@ClassRule
public static GenericContainer mongoContainer = new GenericContainer(CompletableFuture.completedFuture("mongo:4.4"))
.withExposedPorts(27017);
private JsonNode value;
@BeforeClass
public static void setUp() {
address = mongoContainer.getContainerIpAddress();
@ -1691,4 +1691,129 @@ public class MongoPluginTest {
dsConnectionMono.flatMap(conn -> pluginExecutor.executeParameterized(conn, new ExecuteActionDTO(), dsConfig, actionConfiguration)).block();
}
@Test
public void testReadableErrorWithFilterKeyError() {
DatasourceConfiguration dsConfig = createDatasourceConfiguration();
Mono<MongoClient> dsConnectionMono = pluginExecutor.datasourceCreate(dsConfig);
ActionConfiguration actionConfiguration = new ActionConfiguration();
// Set bad attribute for limit key
actionConfiguration.setBody("{\n" +
" find: \"users\",\n" +
" filter: \"filter\",\n" +
" limit: 10,\n" +
" }");
Map<String, Object> configMap = new HashMap<>();
setValueSafelyInFormData(configMap, SMART_SUBSTITUTION, Boolean.TRUE);
setValueSafelyInFormData(configMap, COMMAND, "RAW");
actionConfiguration.setFormData(configMap);
Mono<Object> executeMono = dsConnectionMono.flatMap(conn -> pluginExecutor.executeParameterized(conn,
new ExecuteActionDTO(), dsConfig, actionConfiguration));
StepVerifier.create(executeMono)
.assertNext(obj -> {
ActionExecutionResult result = (ActionExecutionResult) obj;
assertNotNull(result);
assertFalse(result.getIsExecutionSuccess());
assertNotNull(result.getBody());
// Verify readable error.
String expectedReadableError = "'filter' field must be of BSON type object.";
assertEquals(expectedReadableError, result.getReadableError());
})
.verifyComplete();
}
@Test
public void testReadableErrorWithMongoFailedToParseError() {
DatasourceConfiguration dsConfig = createDatasourceConfiguration();
Mono<MongoClient> dsConnectionMono = pluginExecutor.datasourceCreate(dsConfig);
ActionConfiguration actionConfiguration = new ActionConfiguration();
// Set bad attribute for limit key
actionConfiguration.setBody("{\n" +
" find: \"users\",\n" +
" limit: [10],\n" +
" }");
Map<String, Object> configMap = new HashMap<>();
setValueSafelyInFormData(configMap, SMART_SUBSTITUTION, Boolean.TRUE);
setValueSafelyInFormData(configMap, COMMAND, "RAW");
actionConfiguration.setFormData(configMap);
Mono<Object> executeMono = dsConnectionMono.flatMap(conn -> pluginExecutor.executeParameterized(conn,
new ExecuteActionDTO(), dsConfig, actionConfiguration));
StepVerifier.create(executeMono)
.assertNext(obj -> {
ActionExecutionResult result = (ActionExecutionResult) obj;
assertNotNull(result);
assertFalse(result.getIsExecutionSuccess());
assertNotNull(result.getBody());
// Verify readable error.
String expectedReadableError = "'limit' field must be numeric.";
assertEquals(expectedReadableError, result.getReadableError());
})
.verifyComplete();
}
@Test
public void testReadableErrorWithMongoBadKeyError() {
DatasourceConfiguration dsConfig = createDatasourceConfiguration();
Mono<MongoClient> dsConnectionMono = pluginExecutor.datasourceCreate(dsConfig);
ActionConfiguration actionConfiguration = new ActionConfiguration();
// Set unrecognized key limitx
actionConfiguration.setBody("{\n" +
" find: \"users\",\n" +
" limitx: 10,\n" +
" }");
Map<String, Object> configMap = new HashMap<>();
setValueSafelyInFormData(configMap, SMART_SUBSTITUTION, Boolean.TRUE);
setValueSafelyInFormData(configMap, COMMAND, "RAW");
actionConfiguration.setFormData(configMap);
Mono<Object> executeMono = dsConnectionMono.flatMap(conn -> pluginExecutor.executeParameterized(conn,
new ExecuteActionDTO(), dsConfig, actionConfiguration));
StepVerifier.create(executeMono)
.assertNext(obj -> {
ActionExecutionResult result = (ActionExecutionResult) obj;
assertNotNull(result);
assertFalse(result.getIsExecutionSuccess());
assertNotNull(result.getBody());
// Verify readable error.
String expectedReadableError = "Unrecognized field 'limitx'.";
assertEquals(expectedReadableError, result.getReadableError());
})
.verifyComplete();
}
@Test
public void testReadableErrorOnTestDatasourceFailWithBadCredentials() {
// Mock exception on authentication failure.
MongoSecurityException mockMongoSecurityException = mock(MongoSecurityException.class);
when(mockMongoSecurityException.getCode()).thenReturn(-4);
when(mockMongoSecurityException.getMessage()).thenReturn("Exception authenticating " +
"MongoCredential{mechanism=SCRAM-SHA-1, userName='username', source='admin', password=<hidden>," +
" mechanismProperties=<hidden>}");
// Throw mock error on datasource create method call.
MongoPlugin.MongoPluginExecutor spyMongoPluginExecutor = spy(pluginExecutor);
doReturn(Mono.error(mockMongoSecurityException)).when(spyMongoPluginExecutor).datasourceCreate(any());
DatasourceConfiguration dsConfig = createDatasourceConfiguration();
StepVerifier.create(spyMongoPluginExecutor.testDatasource(dsConfig))
.assertNext(datasourceTestResult -> {
assertNotNull(datasourceTestResult);
assertFalse(datasourceTestResult.isSuccess());
// Verify readable error.
String expectedReadableError = "Exception authenticating MongoCredential.";
assertEquals(expectedReadableError, datasourceTestResult.getInvalids().toArray()[0]);
})
.verifyComplete();
}
}