Protei/PromucFlow_constructor
2
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

Merge branch 'feature/http-error-handling' into 'master'

Browse Source 
Standardizing Http error handling

Fixes #9

Sending standardized HTTP error codes to the client application. This structure ensures that we can define the http error code, app error code and (in future) localized string when we want to.

Now when we want to send any error message to the client, we should add it to the enum `AppsmithError` and throw an `AppsmithException` by passing this enum value to the constructor. In the future, we can also localize these error messages by defining the template message in a `messages_en.properties` file.

See merge request theappsmith/internal-tools-server!8
This commit is contained in:
Arpit Mohan 2019-08-28 09:35:06 +00:00
commit 2acb44382c
12 changed files with 166 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
app/server/src/main/java/com/appsmith/server/dtos/ResponseDto.java
View File

@ -18,4 +18,13 @@ public class ResponseDto<T> implements Serializable {
private T data;
private String message;
private boolean success = true;
public ResponseDto(int status, T data, String message) {
this.status = status;
this.data = data;
this.message = message;
}
}

29
app/server/src/main/java/com/appsmith/server/exceptions/AppsmithError.java Normal file
View File

@ -0,0 +1,29 @@
package com.appsmith.server.exceptions;
import lombok.Getter;
import java.text.MessageFormat;
@Getter
public enum AppsmithError {
NO_RESOURCE_FOUND(404, 1000, "Unable to find {0} with id {1}"),
INVALID_PARAMETER(400, 4000, "Invalid parameter {0} provided in the input"),
INTERNAL_SERVER_ERROR(500, 5000, "Internal server error while processing request");
private Integer httpErrorCode;
private Integer appErrorCode;
private String message;
private AppsmithError(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);
}
}

27
app/server/src/main/java/com/appsmith/server/exceptions/AppsmithException.java
View File

@ -1,8 +1,35 @@
package com.appsmith.server.exceptions;
import lombok.Getter;
import lombok.Setter;
@Getter
@Setter
public class AppsmithException extends Exception {
private AppsmithError error;
private Object[] args;
public AppsmithException(String msg) {
super(msg);
}
public AppsmithException(AppsmithError 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();
}
}

19
app/server/src/main/java/com/appsmith/server/exceptions/ErrorDTO.java Normal file
View File

@ -0,0 +1,19 @@
package com.appsmith.server.exceptions;
import lombok.AllArgsConstructor;
import lombok.Builder;
import lombok.Getter;
import lombok.Setter;
import java.io.Serializable;
@Getter
@Setter
@Builder
@AllArgsConstructor
public class ErrorDTO implements Serializable {
private int errorCode;
private String errorMessage;
}

49
app/server/src/main/java/com/appsmith/server/exceptions/GlobalExceptionHandler.java Normal file
View File

@ -0,0 +1,49 @@
package com.appsmith.server.exceptions;
import com.appsmith.server.dtos.ResponseDto;
import org.springframework.http.HttpStatus;
import org.springframework.web.bind.annotation.ControllerAdvice;
import org.springframework.web.bind.annotation.ExceptionHandler;
import org.springframework.web.bind.annotation.ResponseBody;
import org.springframework.web.server.ServerWebExchange;
import reactor.core.publisher.Mono;
/**
* This class catches all the Exceptions and formats them into a proper ResponseDTO<ErrorDTO> object before
* sending it to the client.
*/
@ControllerAdvice
public class GlobalExceptionHandler {
/**
* This function only catches the AppsmithException type and formats it into ResponseEntity<ErrorDTO> object
* Ideally, we should only be throwing AppsmithException from our code. This ensures that we can standardize
* and set proper error messages and codes.
* @param e AppsmithException that will be caught by the function
* @param exchange ServerWebExchange contract in order to extract the response and set the http status code
* @return Mono<ResponseDto<ErrorDTO>>
*/
@ExceptionHandler
@ResponseBody
public Mono<ResponseDto<ErrorDTO>> catchAppsmithException(AppsmithException e, ServerWebExchange exchange) {
exchange.getResponse().setStatusCode(HttpStatus.resolve(e.getHttpStatus()));
return Mono.just(new ResponseDto<>(e.getHttpStatus(), new ErrorDTO(e.getAppErrorCode(), e.getMessage()), null, false));
}
/**
* 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
*
* @param e Exception that will be caught by the function
* @param exchange ServerWebExchange contract in order to extract the response and set the http status code
* @return Mono<ResponseDto<ErrorDTO>>
*/
@ExceptionHandler
@ResponseBody
public Mono<ResponseDto<ErrorDTO>> catchException(Exception e, ServerWebExchange exchange) {
exchange.getResponse().setStatusCode(HttpStatus.INTERNAL_SERVER_ERROR);
return Mono.just(new ResponseDto<>(HttpStatus.INTERNAL_SERVER_ERROR.value(), new ErrorDTO(AppsmithError.INTERNAL_SERVER_ERROR.getHttpErrorCode(),
AppsmithError.INTERNAL_SERVER_ERROR.getMessage()), null, false));
}
}

25
app/server/src/main/java/com/appsmith/server/filters/MDCFilter.java
View File

@ -1,6 +1,7 @@
package com.appsmith.server.filters;
import com.appsmith.server.helpers.LogHelper;
import lombok.extern.slf4j.Slf4j;
import org.slf4j.MDC;
import org.springframework.http.HttpHeaders;
import org.springframework.http.server.reactive.ServerHttpRequest;
@ -13,6 +14,7 @@ import reactor.core.publisher.Mono;
import reactor.util.context.Context;
import java.util.Map;
import java.util.UUID;
import static java.util.stream.Collectors.toMap;
@ -21,9 +23,12 @@ import static java.util.stream.Collectors.toMap;
* These MDC parameters are also set in the response object before being sent to the user.
*/
@Component
@Slf4j
public class MDCFilter implements WebFilter {
private static final String MDC_HEADER_PREFIX = "X-MDC-";
private static final String REQUEST_ID_HEADER = "X-REQUEST-ID";
private static final String REQUEST_ID_LOG = "requestId";
@Override
public Mono<Void> filter(ServerWebExchange exchange, WebFilterChain chain) {
@ -43,6 +48,8 @@ public class MDCFilter implements WebFilter {
.filter(x -> x.getKey().startsWith(MDC_HEADER_PREFIX))
.collect(toMap(v -> v.getKey().substring((MDC_HEADER_PREFIX.length())), Map.Entry::getValue));
contextMap.put(REQUEST_ID_LOG, getOrCreateRequestId(request));
// Set the MDC context here for regular non-reactive logs
MDC.setContextMap(contextMap);
@ -59,8 +66,24 @@ public class MDCFilter implements WebFilter {
final HttpHeaders httpHeaders = response.getHeaders();
// Add all the request MDC keys to the response object
ctx.<Map<String, String>>get(LogHelper.CONTEXT_MAP)
.forEach((key, value) -> httpHeaders.add(MDC_HEADER_PREFIX + key, value));
.forEach((key, value) -> {
if(!key.contains(REQUEST_ID_LOG)) {
httpHeaders.add(MDC_HEADER_PREFIX + key, value);
} else {
httpHeaders.add(REQUEST_ID_HEADER, value);
}
});
}).then();
}
private String getOrCreateRequestId(final ServerHttpRequest request) {
if (!request.getHeaders().containsKey(REQUEST_ID_HEADER)) {
request.mutate().header(REQUEST_ID_HEADER, UUID.randomUUID().toString()).build();
}
String header = request.getHeaders().get(REQUEST_ID_HEADER).get(0);
return header;
}
}

34
app/server/src/main/java/com/appsmith/server/filters/RequestIdFilter.java
View File

@ -1,34 +0,0 @@
package com.appsmith.server.filters;
import com.appsmith.server.helpers.LogHelper;
import lombok.extern.slf4j.Slf4j;
import org.springframework.stereotype.Component;
import org.springframework.web.server.ServerWebExchange;
import org.springframework.web.server.WebFilter;
import org.springframework.web.server.WebFilterChain;
import reactor.core.publisher.Mono;
import java.util.UUID;
/**
* This class specifically parses the requestId key from the headers and sets it in the logger MDC
*/
@Slf4j
@Component
public class RequestIdFilter implements WebFilter {
private static final String REQUEST_ID_HEADER = "X-REQUEST-ID";
private static final String REQUEST_ID_LOG = "requestId";
@Override
public Mono<Void> filter(ServerWebExchange exchange, WebFilterChain chain) {
if (!exchange.getRequest().getHeaders().containsKey(REQUEST_ID_HEADER)) {
exchange.getRequest().mutate().header(REQUEST_ID_HEADER, UUID.randomUUID().toString()).build();
}
String header = exchange.getRequest().getHeaders().get(REQUEST_ID_HEADER).get(0);
log.debug("Setting the requestId header to {}", header);
return chain.filter(exchange).subscriberContext(LogHelper.putLogContext(REQUEST_ID_LOG, header));
}
}

4
app/server/src/main/java/com/appsmith/server/services/BaseService.java
View File

@ -1,8 +1,10 @@
package com.appsmith.server.services;
import com.appsmith.server.domains.BaseDomain;
import com.appsmith.server.exceptions.AppsmithError;
import com.appsmith.server.exceptions.AppsmithException;
import com.appsmith.server.repositories.BaseRepository;
import com.mongodb.BasicDBObject;
import com.mongodb.DBObject;
import org.springframework.data.mongodb.core.ReactiveMongoTemplate;
@ -63,7 +65,7 @@ public abstract class BaseService<R extends BaseRepository, T extends BaseDomain
@Override
public Mono<T> getById(ID id) {
return repository.findById(id)
.switchIfEmpty(Mono.error(new AppsmithException("Unable to find resource with id: " + id.toString())));
.switchIfEmpty(Mono.error(new AppsmithException(AppsmithError.NO_RESOURCE_FOUND, "resource", id)));
}
@Override

2
app/server/src/main/java/com/appsmith/server/services/PluginService.java
View File

@ -19,8 +19,6 @@ public interface PluginService extends CrudService<Plugin, String> {
*/
PluginExecutor getPluginExecutor(PluginType pluginType, String className);
public Mono<Plugin> create(Plugin plugin) throws AppsmithException;
public Mono<Tenant> installPlugin(PluginTenantDTO plugin);
public Mono<Tenant> uninstallPlugin(PluginTenantDTO plugin);

3
app/server/src/main/java/com/appsmith/server/services/PluginServiceImpl.java
View File

@ -7,6 +7,7 @@ import com.appsmith.server.domains.Tenant;
import com.appsmith.server.domains.TenantPlugin;
import com.appsmith.server.dtos.PluginTenantDTO;
import com.appsmith.server.dtos.TenantPluginStatus;
import com.appsmith.server.exceptions.AppsmithError;
import com.appsmith.server.exceptions.AppsmithException;
import com.appsmith.server.repositories.PluginRepository;
import com.appsmith.server.repositories.UserRepository;
@ -71,7 +72,7 @@ public class PluginServiceImpl extends BaseService<PluginRepository, Plugin, Str
@Override
public Mono<Plugin> create(Plugin plugin) throws AppsmithException {
if (plugin.getId() != null) {
throw new AppsmithException("During create plugin, Id is not null. Can't create new plugin.");
throw new AppsmithException(AppsmithError.INVALID_PARAMETER, "id");
}
plugin.setDeleted(false);
return pluginRepository.save(plugin);

3
app/server/src/main/java/com/appsmith/server/services/QueryServiceImpl.java
View File

@ -2,6 +2,7 @@ package com.appsmith.server.services;
import com.appsmith.server.domains.Query;
import com.appsmith.server.dtos.CommandQueryParams;
import com.appsmith.server.exceptions.AppsmithError;
import com.appsmith.server.exceptions.AppsmithException;
import com.appsmith.server.repositories.QueryRepository;
import lombok.extern.slf4j.Slf4j;
@ -36,7 +37,7 @@ public class QueryServiceImpl extends BaseService<QueryRepository, Query, String
// 1. Fetch the query from the DB to get the type
Mono<Query> queryMono = repository.findByName(name)
.switchIfEmpty(Mono.defer(() -> Mono.error(new AppsmithException("Unable to find query by id: " + name))));
.switchIfEmpty(Mono.defer(() -> Mono.error(new AppsmithException(AppsmithError.NO_RESOURCE_FOUND, "query", name))));
// 2. Instantiate the implementation class based on the query type
Mono<PluginExecutor> pluginExecutorMono = queryMono.map(queryObj ->

3
app/server/src/main/java/com/appsmith/server/services/ResourceServiceImpl.java
View File

@ -4,6 +4,7 @@ import com.appsmith.server.domains.Plugin;
import com.appsmith.server.domains.Resource;
import com.appsmith.server.domains.Tenant;
import com.appsmith.server.domains.TenantPlugin;
import com.appsmith.server.exceptions.AppsmithError;
import com.appsmith.server.exceptions.AppsmithException;
import com.appsmith.server.repositories.ResourceRepository;
import lombok.extern.slf4j.Slf4j;
@ -41,7 +42,7 @@ public class ResourceServiceImpl extends BaseService<ResourceRepository, Resourc
@Override
public Mono<Resource> create(@NotNull Resource resource) throws AppsmithException {
if (resource.getId() != null) {
throw new AppsmithException("During create resource, Id is not null. Can't create new resource.");
throw new AppsmithException(AppsmithError.INVALID_PARAMETER, "id");
}
Mono<Tenant> tenantMono = tenantService.findById(tenantId);