Added timeout on the plugin execution.
Next TODO : Make the timeout duration configurable
This commit is contained in:
parent
db5deb520e
commit
0fd6351a76
|
|
@ -25,6 +25,8 @@ public class ActionConfiguration {
|
||||||
* action execution.
|
* action execution.
|
||||||
*/
|
*/
|
||||||
|
|
||||||
|
int timeoutInMillisecond = 10000;
|
||||||
|
|
||||||
// API fields
|
// API fields
|
||||||
String path;
|
String path;
|
||||||
List<Property> headers;
|
List<Property> headers;
|
||||||
|
|
|
||||||
|
|
@ -31,7 +31,8 @@ public enum AppsmithError {
|
||||||
INTERNAL_SERVER_ERROR(500, 5000, "Internal server error while processing request"),
|
INTERNAL_SERVER_ERROR(500, 5000, "Internal server error while processing request"),
|
||||||
REPOSITORY_SAVE_FAILED(500, 5001, "Repository save failed"),
|
REPOSITORY_SAVE_FAILED(500, 5001, "Repository save failed"),
|
||||||
PLUGIN_INSTALLATION_FAILED_DOWNLOAD_ERROR(500, 5002, "Due to error in downloading the plugin from remote repository, plugin installation has failed. Check the jar location and try again"),
|
PLUGIN_INSTALLATION_FAILED_DOWNLOAD_ERROR(500, 5002, "Due to error in downloading the plugin from remote repository, plugin installation has failed. Check the jar location and try again"),
|
||||||
PLUGIN_RUN_FAILED(500, 5003, "Plugin execution failed with error {0}");
|
PLUGIN_RUN_FAILED(500, 5003, "Plugin execution failed with error {0}"),
|
||||||
|
PLUGIN_EXECUTION_TIMEOUT(504, 5040, "Plugin Execution exceeded the maximum allowed time. Please increase the timeout in your action settings or check your backend action endpoint");
|
||||||
|
|
||||||
|
|
||||||
private Integer httpErrorCode;
|
private Integer httpErrorCode;
|
||||||
|
|
|
||||||
|
|
@ -48,7 +48,7 @@ public class GlobalExceptionHandler {
|
||||||
|
|
||||||
@ExceptionHandler
|
@ExceptionHandler
|
||||||
@ResponseBody
|
@ResponseBody
|
||||||
public Mono<ResponseDTO<ErrorDTO>> catchException(org.springframework.dao.DuplicateKeyException e, ServerWebExchange exchange) {
|
public Mono<ResponseDTO<ErrorDTO>> catchDuplicateKeyException(org.springframework.dao.DuplicateKeyException e, ServerWebExchange exchange) {
|
||||||
exchange.getResponse().setStatusCode(HttpStatus.BAD_REQUEST);
|
exchange.getResponse().setStatusCode(HttpStatus.BAD_REQUEST);
|
||||||
log.error("", e);
|
log.error("", e);
|
||||||
rollbar.log(e);
|
rollbar.log(e);
|
||||||
|
|
@ -56,6 +56,16 @@ public class GlobalExceptionHandler {
|
||||||
AppsmithError.DUPLICATE_KEY.getMessage())));
|
AppsmithError.DUPLICATE_KEY.getMessage())));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ExceptionHandler
|
||||||
|
@ResponseBody
|
||||||
|
public Mono<ResponseDTO<ErrorDTO>> catchTimeoutException(java.util.concurrent.TimeoutException e, ServerWebExchange exchange) {
|
||||||
|
exchange.getResponse().setStatusCode(HttpStatus.GATEWAY_TIMEOUT);
|
||||||
|
log.error("", e);
|
||||||
|
rollbar.log(e);
|
||||||
|
return Mono.just(new ResponseDTO<>(HttpStatus.GATEWAY_TIMEOUT.value(), new ErrorDTO(AppsmithError.PLUGIN_EXECUTION_TIMEOUT.getHttpErrorCode(),
|
||||||
|
AppsmithError.PLUGIN_EXECUTION_TIMEOUT.getMessage())));
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* This function catches the generic Exception class and is meant to be a catch all to ensure that we don't leak
|
* 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
|
* any information to the client. Ideally, the function #catchAppsmithException should be used
|
||||||
|
|
|
||||||
|
|
@ -37,6 +37,7 @@ import javax.validation.constraints.NotNull;
|
||||||
import java.io.StringReader;
|
import java.io.StringReader;
|
||||||
import java.io.StringWriter;
|
import java.io.StringWriter;
|
||||||
import java.io.Writer;
|
import java.io.Writer;
|
||||||
|
import java.time.Duration;
|
||||||
import java.util.ArrayList;
|
import java.util.ArrayList;
|
||||||
import java.util.HashSet;
|
import java.util.HashSet;
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
|
|
@ -296,6 +297,7 @@ public class ActionServiceImpl extends BaseService<ActionRepository, Action, Str
|
||||||
@Override
|
@Override
|
||||||
public Mono<ActionExecutionResult> executeAction(ExecuteActionDTO executeActionDTO) {
|
public Mono<ActionExecutionResult> executeAction(ExecuteActionDTO executeActionDTO) {
|
||||||
Action actionFromDto = executeActionDTO.getAction();
|
Action actionFromDto = executeActionDTO.getAction();
|
||||||
|
final int[] timeoutDuration = new int[1];
|
||||||
|
|
||||||
// 1. Validate input parameters which are required for mustache replacements
|
// 1. Validate input parameters which are required for mustache replacements
|
||||||
List<Param> params = executeActionDTO.getParams();
|
List<Param> params = executeActionDTO.getParams();
|
||||||
|
|
@ -365,7 +367,7 @@ public class ActionServiceImpl extends BaseService<ActionRepository, Action, Str
|
||||||
.getParams()
|
.getParams()
|
||||||
.stream()
|
.stream()
|
||||||
.collect(Collectors.toMap(Param::getKey, Param::getValue,
|
.collect(Collectors.toMap(Param::getKey, Param::getValue,
|
||||||
// Incase there's a conflict, we pick the older value
|
// In case of a conflict, we pick the older value
|
||||||
(oldValue, newValue) -> oldValue)
|
(oldValue, newValue) -> oldValue)
|
||||||
);
|
);
|
||||||
datasourceConfiguration = (DatasourceConfiguration) variableSubstitution(datasource.getDatasourceConfiguration(), replaceParamsMap);
|
datasourceConfiguration = (DatasourceConfiguration) variableSubstitution(datasource.getDatasourceConfiguration(), replaceParamsMap);
|
||||||
|
|
@ -374,6 +376,7 @@ public class ActionServiceImpl extends BaseService<ActionRepository, Action, Str
|
||||||
datasourceConfiguration = datasource.getDatasourceConfiguration();
|
datasourceConfiguration = datasource.getDatasourceConfiguration();
|
||||||
actionConfiguration = action.getActionConfiguration();
|
actionConfiguration = action.getActionConfiguration();
|
||||||
}
|
}
|
||||||
|
timeoutDuration[0] = actionConfiguration.getTimeoutInMillisecond();
|
||||||
return datasourceContextService
|
return datasourceContextService
|
||||||
.getDatasourceContext(datasource)
|
.getDatasourceContext(datasource)
|
||||||
//Now that we have the context (connection details, execute the action
|
//Now that we have the context (connection details, execute the action
|
||||||
|
|
@ -382,8 +385,8 @@ public class ActionServiceImpl extends BaseService<ActionRepository, Action, Str
|
||||||
datasourceConfiguration,
|
datasourceConfiguration,
|
||||||
actionConfiguration));
|
actionConfiguration));
|
||||||
}))
|
}))
|
||||||
// .onErrorResume(e -> Mono.error(new AppsmithException(AppsmithError.PLUGIN_RUN_FAILED, e.getMessage())))
|
.flatMap(obj -> obj)
|
||||||
.flatMap(obj -> obj);
|
.timeout(Duration.ofMillis(timeoutDuration[0]));
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue
Block a user