diff --git a/app/server/src/main/java/com/appsmith/server/dtos/ResponseDto.java b/app/server/src/main/java/com/appsmith/server/dtos/ResponseDto.java index f4c12b1f0d..6699234b32 100644 --- a/app/server/src/main/java/com/appsmith/server/dtos/ResponseDto.java +++ b/app/server/src/main/java/com/appsmith/server/dtos/ResponseDto.java @@ -18,4 +18,13 @@ public class ResponseDto 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; + } + } diff --git a/app/server/src/main/java/com/appsmith/server/exceptions/AppsmithError.java b/app/server/src/main/java/com/appsmith/server/exceptions/AppsmithError.java new file mode 100644 index 0000000000..0dab1465f1 --- /dev/null +++ b/app/server/src/main/java/com/appsmith/server/exceptions/AppsmithError.java @@ -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); + } + +} diff --git a/app/server/src/main/java/com/appsmith/server/exceptions/AppsmithException.java b/app/server/src/main/java/com/appsmith/server/exceptions/AppsmithException.java index 28787d8086..876fd11f7e 100644 --- a/app/server/src/main/java/com/appsmith/server/exceptions/AppsmithException.java +++ b/app/server/src/main/java/com/appsmith/server/exceptions/AppsmithException.java @@ -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(); + } + } diff --git a/app/server/src/main/java/com/appsmith/server/exceptions/ErrorDTO.java b/app/server/src/main/java/com/appsmith/server/exceptions/ErrorDTO.java new file mode 100644 index 0000000000..3399703740 --- /dev/null +++ b/app/server/src/main/java/com/appsmith/server/exceptions/ErrorDTO.java @@ -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; +} diff --git a/app/server/src/main/java/com/appsmith/server/exceptions/GlobalExceptionHandler.java b/app/server/src/main/java/com/appsmith/server/exceptions/GlobalExceptionHandler.java new file mode 100644 index 0000000000..113e1db21f --- /dev/null +++ b/app/server/src/main/java/com/appsmith/server/exceptions/GlobalExceptionHandler.java @@ -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 object before + * sending it to the client. + */ +@ControllerAdvice +public class GlobalExceptionHandler { + + /** + * This function only catches the AppsmithException type and formats it into ResponseEntity 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> + */ + @ExceptionHandler + @ResponseBody + public Mono> 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> + */ + @ExceptionHandler + @ResponseBody + public Mono> 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)); + } + +} diff --git a/app/server/src/main/java/com/appsmith/server/filters/MDCFilter.java b/app/server/src/main/java/com/appsmith/server/filters/MDCFilter.java index 5cbacee57a..a233c8c602 100644 --- a/app/server/src/main/java/com/appsmith/server/filters/MDCFilter.java +++ b/app/server/src/main/java/com/appsmith/server/filters/MDCFilter.java @@ -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 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.>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; + } } diff --git a/app/server/src/main/java/com/appsmith/server/filters/RequestIdFilter.java b/app/server/src/main/java/com/appsmith/server/filters/RequestIdFilter.java deleted file mode 100644 index 903bab3b13..0000000000 --- a/app/server/src/main/java/com/appsmith/server/filters/RequestIdFilter.java +++ /dev/null @@ -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 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)); - } -} diff --git a/app/server/src/main/java/com/appsmith/server/services/BaseService.java b/app/server/src/main/java/com/appsmith/server/services/BaseService.java index 1132ff3c85..3ec6e0dcb9 100644 --- a/app/server/src/main/java/com/appsmith/server/services/BaseService.java +++ b/app/server/src/main/java/com/appsmith/server/services/BaseService.java @@ -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 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 diff --git a/app/server/src/main/java/com/appsmith/server/services/PluginService.java b/app/server/src/main/java/com/appsmith/server/services/PluginService.java index cb9a43b053..9153dbeb4e 100644 --- a/app/server/src/main/java/com/appsmith/server/services/PluginService.java +++ b/app/server/src/main/java/com/appsmith/server/services/PluginService.java @@ -19,8 +19,6 @@ public interface PluginService extends CrudService { */ PluginExecutor getPluginExecutor(PluginType pluginType, String className); - public Mono create(Plugin plugin) throws AppsmithException; - public Mono installPlugin(PluginTenantDTO plugin); public Mono uninstallPlugin(PluginTenantDTO plugin); diff --git a/app/server/src/main/java/com/appsmith/server/services/PluginServiceImpl.java b/app/server/src/main/java/com/appsmith/server/services/PluginServiceImpl.java index 1b5fd8dbf4..33248736a5 100644 --- a/app/server/src/main/java/com/appsmith/server/services/PluginServiceImpl.java +++ b/app/server/src/main/java/com/appsmith/server/services/PluginServiceImpl.java @@ -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 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); diff --git a/app/server/src/main/java/com/appsmith/server/services/QueryServiceImpl.java b/app/server/src/main/java/com/appsmith/server/services/QueryServiceImpl.java index d1b0421cfe..c7241db460 100644 --- a/app/server/src/main/java/com/appsmith/server/services/QueryServiceImpl.java +++ b/app/server/src/main/java/com/appsmith/server/services/QueryServiceImpl.java @@ -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 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 pluginExecutorMono = queryMono.map(queryObj -> diff --git a/app/server/src/main/java/com/appsmith/server/services/ResourceServiceImpl.java b/app/server/src/main/java/com/appsmith/server/services/ResourceServiceImpl.java index 418a93fd8f..fa3dfec22b 100644 --- a/app/server/src/main/java/com/appsmith/server/services/ResourceServiceImpl.java +++ b/app/server/src/main/java/com/appsmith/server/services/ResourceServiceImpl.java @@ -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 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 tenantMono = tenantService.findById(tenantId);