diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/pluginExceptions/AppsmithPluginError.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/pluginExceptions/AppsmithPluginError.java new file mode 100644 index 0000000000..0e8eb17358 --- /dev/null +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/pluginExceptions/AppsmithPluginError.java @@ -0,0 +1,27 @@ +package com.appsmith.external.pluginExceptions; + +import lombok.Getter; + +import java.text.MessageFormat; + +@Getter +public enum AppsmithPluginError { + + PLUGIN_ERROR(500, 5000, "PluginExecution failed with error {0}"); + + private Integer httpErrorCode; + private Integer appErrorCode; + private String message; + + AppsmithPluginError(Integer httpErrorCode, Integer appErrorCode, String message, Object... args) { + this.httpErrorCode = httpErrorCode; + this.appErrorCode = appErrorCode; + MessageFormat fmt = new MessageFormat(message); + this.message = fmt.format(args); + } + + public String getMessage(Object... args) { + return new MessageFormat(this.message).format(args); + } + +} diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/pluginExceptions/AppsmithPluginException.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/pluginExceptions/AppsmithPluginException.java new file mode 100644 index 0000000000..3be7397c6a --- /dev/null +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/pluginExceptions/AppsmithPluginException.java @@ -0,0 +1,35 @@ +package com.appsmith.external.pluginExceptions; + +import lombok.Getter; +import lombok.Setter; + +@Getter +@Setter +public class AppsmithPluginException extends Exception { + + private AppsmithPluginError error; + private Object[] args; + + public AppsmithPluginException(String msg) { + super(msg); + } + + public AppsmithPluginException(AppsmithPluginError error, Object... args) { + super(error.getMessage(args)); + this.error = error; + this.args = args; + } + + public Integer getHttpStatus() { + return this.error.getHttpErrorCode(); + } + + public String getMessage(Object... args) { + return this.error.getMessage(this.args); + } + + public Integer getAppErrorCode() { + return this.error.getAppErrorCode(); + } + +} diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/plugins/PluginExecutor.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/plugins/PluginExecutor.java index e367122bdd..34e8ec7f96 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/plugins/PluginExecutor.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/plugins/PluginExecutor.java @@ -1,7 +1,6 @@ package com.appsmith.external.plugins; import com.appsmith.external.models.ActionConfiguration; -import com.appsmith.external.models.ActionExecutionResult; import com.appsmith.external.models.DatasourceConfiguration; import org.pf4j.ExtensionPoint; import reactor.core.publisher.Mono; @@ -16,7 +15,7 @@ public interface PluginExecutor extends ExtensionPoint { * @param actionConfiguration : These are the configurations which have been used to create an Action from a Datasource. * @return ActionExecutionResult : This object is returned to the user which contains the result values from the execution. */ - Mono execute(Object connection, DatasourceConfiguration datasourceConfiguration, ActionConfiguration actionConfiguration); + Mono execute(Object connection, DatasourceConfiguration datasourceConfiguration, ActionConfiguration actionConfiguration); /** * This function is responsible for creating the connection to the data source and returning the connection variable diff --git a/app/server/appsmith-plugins/mongoPlugin/src/main/java/com/external/plugins/MongoPlugin.java b/app/server/appsmith-plugins/mongoPlugin/src/main/java/com/external/plugins/MongoPlugin.java index 238d412167..003de5d0c4 100644 --- a/app/server/appsmith-plugins/mongoPlugin/src/main/java/com/external/plugins/MongoPlugin.java +++ b/app/server/appsmith-plugins/mongoPlugin/src/main/java/com/external/plugins/MongoPlugin.java @@ -45,9 +45,9 @@ public class MongoPlugin extends BasePlugin { * @return */ @Override - public Mono execute(Object connection, - DatasourceConfiguration datasourceConfiguration, - ActionConfiguration actionConfiguration) { + public Mono execute(Object connection, + DatasourceConfiguration datasourceConfiguration, + ActionConfiguration actionConfiguration) { ActionExecutionResult result = new ActionExecutionResult(); MongoClient mongoClient = (MongoClient) connection; diff --git a/app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/PostgresPlugin.java b/app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/PostgresPlugin.java index a104081234..dcd0306152 100644 --- a/app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/PostgresPlugin.java +++ b/app/server/appsmith-plugins/postgresPlugin/src/main/java/com/external/plugins/PostgresPlugin.java @@ -47,9 +47,9 @@ public class PostgresPlugin extends BasePlugin { public static class PostgresPluginExecutor implements PluginExecutor { @Override - public Mono execute(Object connection, - DatasourceConfiguration datasourceConfiguration, - ActionConfiguration actionConfiguration) { + public Mono execute(Object connection, + DatasourceConfiguration datasourceConfiguration, + ActionConfiguration actionConfiguration) { Connection conn = (Connection) connection; Assert.notNull(conn); diff --git a/app/server/appsmith-plugins/restApiPlugin/src/main/java/com/external/plugins/RestApiPlugin.java b/app/server/appsmith-plugins/restApiPlugin/src/main/java/com/external/plugins/RestApiPlugin.java index 8cba68f37e..001191108d 100644 --- a/app/server/appsmith-plugins/restApiPlugin/src/main/java/com/external/plugins/RestApiPlugin.java +++ b/app/server/appsmith-plugins/restApiPlugin/src/main/java/com/external/plugins/RestApiPlugin.java @@ -4,6 +4,8 @@ import com.appsmith.external.models.ActionConfiguration; import com.appsmith.external.models.ActionExecutionResult; import com.appsmith.external.models.DatasourceConfiguration; import com.appsmith.external.models.Property; +import com.appsmith.external.pluginExceptions.AppsmithPluginError; +import com.appsmith.external.pluginExceptions.AppsmithPluginException; import com.appsmith.external.plugins.BasePlugin; import com.appsmith.external.plugins.PluginExecutor; import com.fasterxml.jackson.core.JsonProcessingException; @@ -41,9 +43,9 @@ public class RestApiPlugin extends BasePlugin { public static class RestApiPluginExecutor implements PluginExecutor { @Override - public Mono execute(Object connection, - DatasourceConfiguration datasourceConfiguration, - ActionConfiguration actionConfiguration) { + public Mono execute(Object connection, + DatasourceConfiguration datasourceConfiguration, + ActionConfiguration actionConfiguration) { String requestBody = (actionConfiguration.getBody() == null) ? "" : actionConfiguration.getBody(); String path = (actionConfiguration.getPath() == null) ? "" : actionConfiguration.getPath(); @@ -70,7 +72,7 @@ public class RestApiPlugin extends BasePlugin { uri = createFinalUriWithQueryParams(url, actionConfiguration.getQueryParameters()); } catch (URISyntaxException e) { e.printStackTrace(); - return Mono.error(e); + return Mono.error(new AppsmithPluginException(AppsmithPluginError.PLUGIN_ERROR, e)); } WebClient client = webClientBuilder.build(); @@ -95,12 +97,14 @@ public class RestApiPlugin extends BasePlugin { headerInJsonString = objectMapper.writeValueAsString(headers); } catch (JsonProcessingException e) { e.printStackTrace(); + return Mono.defer(() -> Mono.error(new AppsmithPluginException(AppsmithPluginError.PLUGIN_ERROR, e))); } try { // Set headers in the result now result.setHeaders(objectMapper.readTree(headerInJsonString)); } catch (IOException e) { e.printStackTrace(); + return Mono.defer(() -> Mono.error(new AppsmithPluginException(AppsmithPluginError.PLUGIN_ERROR, e))); } // Find the media type of the response to parse the body as required. Currently only JSON @@ -119,6 +123,7 @@ public class RestApiPlugin extends BasePlugin { result.setBody(objectMapper.readTree(body)); } catch (IOException e) { e.printStackTrace(); + return Mono.defer(() -> Mono.error(new AppsmithPluginException(AppsmithPluginError.PLUGIN_ERROR, e))); } } else { // If the body is not of JSON type, just set it as is. @@ -126,7 +131,8 @@ public class RestApiPlugin extends BasePlugin { } } return result; - }); + }) + .doOnError(e -> Mono.error(new AppsmithPluginException(AppsmithPluginError.PLUGIN_ERROR, e))); } private Mono httpCall(WebClient webClient, HttpMethod httpMethod, URI uri, String requestBody, int iteration) { @@ -139,7 +145,9 @@ public class RestApiPlugin extends BasePlugin { .uri(uri) .body(BodyInserters.fromObject(requestBody)) .exchange() - .flatMap(response -> { + .doOnError(e -> Mono.error(new AppsmithPluginException(AppsmithPluginError.PLUGIN_ERROR, e))) + .flatMap(res -> { + ClientResponse response = (ClientResponse) res; if (response.statusCode().is3xxRedirection()) { String redirectUrl = response.headers().header("Location").get(0); URI redirectUri = null; diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/GlobalExceptionHandler.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/GlobalExceptionHandler.java index ec37899f8d..450959c900 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/GlobalExceptionHandler.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/GlobalExceptionHandler.java @@ -1,5 +1,6 @@ package com.appsmith.server.exceptions; +import com.appsmith.external.pluginExceptions.AppsmithPluginException; import com.appsmith.server.dtos.ResponseDTO; import com.rollbar.notifier.Rollbar; import com.segment.analytics.Analytics; @@ -77,6 +78,16 @@ public class GlobalExceptionHandler { AppsmithError.GENERIC_BAD_REQUEST.getMessage(e.getReason())))); } + @ExceptionHandler + @ResponseBody + public Mono> catchPluginException(AppsmithPluginException e, ServerWebExchange exchange) { + exchange.getResponse().setStatusCode(HttpStatus.INTERNAL_SERVER_ERROR); + log.error("", e); + rollbar.log(e); + return Mono.just(new ResponseDTO<>(HttpStatus.INTERNAL_SERVER_ERROR.value(), new ErrorDTO(AppsmithError.INTERNAL_SERVER_ERROR.getHttpErrorCode(), + e.getLocalizedMessage()))); + } + /** * This function catches the generic Exception class and is meant to be a catch all to ensure that we don't leak * any information to the client. Ideally, the function #catchAppsmithException should be used diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ActionServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ActionServiceImpl.java index c41142fde9..14a59bb1fa 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ActionServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ActionServiceImpl.java @@ -370,7 +370,11 @@ public class ActionServiceImpl extends BaseService obj) - .flatMap(result -> { + .flatMap(res -> { + ActionExecutionResult result = (ActionExecutionResult) res; Mono resultMono = Mono.just(result); if (actionFromDto.getId() == null) { // This is a dry-run. We shouldn't query the db because it'll throw NPE on null IDs diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/DatasourceServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/DatasourceServiceTest.java index 7b6c8eb50e..eda60992a8 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/DatasourceServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/DatasourceServiceTest.java @@ -1,7 +1,6 @@ package com.appsmith.server.services; import com.appsmith.external.models.ActionConfiguration; -import com.appsmith.external.models.ActionExecutionResult; import com.appsmith.external.models.DatasourceConfiguration; import com.appsmith.external.plugins.PluginExecutor; import com.appsmith.server.constants.FieldName; @@ -44,7 +43,7 @@ public class DatasourceServiceTest { class TestPluginExecutor implements PluginExecutor { @Override - public Mono execute(Object connection, DatasourceConfiguration datasourceConfiguration, ActionConfiguration actionConfiguration) { + public Mono execute(Object connection, DatasourceConfiguration datasourceConfiguration, ActionConfiguration actionConfiguration) { System.out.println("In the execute"); return null; }