chore: Don't mask 405 as 500 (#32829)

/ok-to-test tags="@tag.Sanity"

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- **New Features**
- Introduced handling for "HTTP method not allowed" errors to improve
user feedback on unsupported actions.

- **Refactor**
- Optimized error handling structure by using annotations and removing
outdated constructors.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Shrikant Sharat Kandula 2024-04-20 13:46:43 +05:30 committed by GitHub
parent efc088e509
commit 37696bb3ec
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 31 additions and 11 deletions

View File

@ -12,6 +12,14 @@ import java.text.MessageFormat;
public enum AppsmithError { public enum AppsmithError {
// Ref syntax for message templates: // Ref syntax for message templates:
// https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/text/MessageFormat.html // https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/text/MessageFormat.html
HTTP_METHOD_NOT_ALLOWED(
405,
AppsmithErrorCode.HTTP_METHOD_NOT_ALLOWED.getCode(),
"HTTP Method not allowed.",
AppsmithErrorAction.DEFAULT,
"Invalid method",
ErrorType.BAD_REQUEST,
null),
INVALID_PARAMETER( INVALID_PARAMETER(
400, 400,
AppsmithErrorCode.INVALID_PARAMETER.getCode(), AppsmithErrorCode.INVALID_PARAMETER.getCode(),

View File

@ -4,6 +4,7 @@ import lombok.Getter;
@Getter @Getter
public enum AppsmithErrorCode { public enum AppsmithErrorCode {
HTTP_METHOD_NOT_ALLOWED("AE-APP-4001", "HTTP method not allowed"),
INVALID_ACTION_COLLECTION("AE-ACC-4038", "Invalid action collection"), INVALID_ACTION_COLLECTION("AE-ACC-4038", "Invalid action collection"),
UNAUTHORIZED_ACCESS("AE-ACL-4003", "Unauthorized access"), UNAUTHORIZED_ACCESS("AE-ACL-4003", "Unauthorized access"),
ACL_NO_RESOURCE_FOUND("AE-ACL-4004", "Acl no resource found"), ACL_NO_RESOURCE_FOUND("AE-ACL-4004", "Acl no resource found"),

View File

@ -17,6 +17,7 @@ import io.micrometer.core.instrument.util.StringUtils;
import io.sentry.Sentry; import io.sentry.Sentry;
import io.sentry.SentryLevel; import io.sentry.SentryLevel;
import io.sentry.protocol.User; import io.sentry.protocol.User;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j; import lombok.extern.slf4j.Slf4j;
import org.eclipse.jgit.api.errors.JGitInternalException; import org.eclipse.jgit.api.errors.JGitInternalException;
import org.eclipse.jgit.errors.LockFailedException; import org.eclipse.jgit.errors.LockFailedException;
@ -28,6 +29,7 @@ import org.springframework.web.bind.annotation.ControllerAdvice;
import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.ExceptionHandler;
import org.springframework.web.bind.annotation.ResponseBody; import org.springframework.web.bind.annotation.ResponseBody;
import org.springframework.web.bind.support.WebExchangeBindException; import org.springframework.web.bind.support.WebExchangeBindException;
import org.springframework.web.server.MethodNotAllowedException;
import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.ServerWebExchange;
import org.springframework.web.server.ServerWebInputException; import org.springframework.web.server.ServerWebInputException;
import reactor.core.publisher.Mono; import reactor.core.publisher.Mono;
@ -44,6 +46,7 @@ import java.util.Map;
* sending it to the client. * sending it to the client.
*/ */
@ControllerAdvice @ControllerAdvice
@RequiredArgsConstructor
@Slf4j @Slf4j
public class GlobalExceptionHandler { public class GlobalExceptionHandler {
@ -55,17 +58,6 @@ public class GlobalExceptionHandler {
private final SessionUserService sessionUserService; private final SessionUserService sessionUserService;
public GlobalExceptionHandler(
RedisUtils redisUtils,
AnalyticsService analyticsService,
GitFileUtils fileUtils,
SessionUserService sessionUserService) {
this.redisUtils = redisUtils;
this.analyticsService = analyticsService;
this.fileUtils = fileUtils;
this.sessionUserService = sessionUserService;
}
private void doLog(Throwable error) { private void doLog(Throwable error) {
if (error instanceof BaseException baseException && baseException.isHideStackTraceInLogs()) { if (error instanceof BaseException baseException && baseException.isHideStackTraceInLogs()) {
log.error(baseException.getClass().getSimpleName() + ": " + baseException.getMessage()); log.error(baseException.getClass().getSimpleName() + ": " + baseException.getMessage());
@ -284,6 +276,25 @@ public class GlobalExceptionHandler {
return getResponseDTOMono(urlPath, response); return getResponseDTOMono(urlPath, response);
} }
@ExceptionHandler
@ResponseBody
public Mono<ResponseDTO<ErrorDTO>> catchMethodNotAllowed(MethodNotAllowedException e, ServerWebExchange exchange) {
AppsmithError appsmithError = AppsmithError.HTTP_METHOD_NOT_ALLOWED;
exchange.getResponse().setStatusCode(HttpStatus.resolve(appsmithError.getHttpErrorCode()));
String urlPath = exchange.getRequest().getPath().toString();
ResponseDTO<ErrorDTO> response = new ResponseDTO<>(
appsmithError.getHttpErrorCode(),
new ErrorDTO(
appsmithError.getAppErrorCode(),
appsmithError.getErrorType(),
appsmithError.getMessage(e.getMessage()),
appsmithError.getTitle()));
return getResponseDTOMono(urlPath, response);
}
/** /**
* 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