Merge branch 'hotfix/log-execute-action' into 'release'

Added log for execute action

See merge request theappsmith/internal-tools-server!159

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);
+    }
+
+}

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();
+    }
+
+}

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<ActionExecutionResult> execute(Object connection, DatasourceConfiguration datasourceConfiguration, ActionConfiguration actionConfiguration);
+    Mono<Object> execute(Object connection, DatasourceConfiguration datasourceConfiguration, ActionConfiguration actionConfiguration);
 
     /**
      * This function is responsible for creating the connection to the data source and returning the connection variable

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<ActionExecutionResult> execute(Object connection,
+        public Mono<Object> execute(Object connection,
-                                                   DatasourceConfiguration datasourceConfiguration,
+                                    DatasourceConfiguration datasourceConfiguration,
-                                                   ActionConfiguration actionConfiguration) {
+                                    ActionConfiguration actionConfiguration) {
 
             ActionExecutionResult result = new ActionExecutionResult();
             MongoClient mongoClient = (MongoClient) connection;

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<ActionExecutionResult> execute(Object connection,
+        public Mono<Object> execute(Object connection,
-                                                   DatasourceConfiguration datasourceConfiguration,
+                                    DatasourceConfiguration datasourceConfiguration,
-                                                   ActionConfiguration actionConfiguration) {
+                                    ActionConfiguration actionConfiguration) {
 
             Connection conn = (Connection) connection;
             Assert.notNull(conn);

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<ActionExecutionResult> execute(Object connection,
+        public Mono<Object> execute(Object connection,
-                                                   DatasourceConfiguration datasourceConfiguration,
+                                    DatasourceConfiguration datasourceConfiguration,
-                                                   ActionConfiguration actionConfiguration) {
+                                    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<ClientResponse> 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;

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<ResponseDTO<ErrorDTO>> 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

app/server/appsmith-server/src/main/java/com/appsmith/server/services/ActionServiceImpl.java

@@ -370,7 +370,11 @@ public class ActionServiceImpl extends BaseService<ActionRepository, Action, Str
                         actionConfiguration = action.getActionConfiguration();
                     }
                     Integer timeoutDuration = actionConfiguration.getTimeoutInMillisecond();
-                    log.debug("Got the timeoutDuration to be: {} ms for action: {}", timeoutDuration, action.getName());
+
+                    log.debug("Execute Action called in Page {}, for action id : {}  action name : {}, {}, {}",
+                            action.getPageId(), action.getId(), action.getName(), datasourceConfiguration,
+                            actionConfiguration);
+
                     return datasourceContextService
                             .getDatasourceContext(datasource)
                             //Now that we have the context (connection details, execute the action
@@ -381,7 +385,8 @@ public class ActionServiceImpl extends BaseService<ActionRepository, Action, Str
                             .timeout(Duration.ofMillis(timeoutDuration));
                 }))
                 .flatMap(obj -> obj)
-                .flatMap(result -> {
+                .flatMap(res -> {
+                    ActionExecutionResult result = (ActionExecutionResult) res;
                     Mono<ActionExecutionResult> 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

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<ActionExecutionResult> execute(Object connection, DatasourceConfiguration datasourceConfiguration, ActionConfiguration actionConfiguration) {
+        public Mono<Object> execute(Object connection, DatasourceConfiguration datasourceConfiguration, ActionConfiguration actionConfiguration) {
             System.out.println("In the execute");
             return null;
         }