Fix disabled opcode endpoint not returning 404 (#9046)

* Add error message to web3 access logs * Change `GenericExceptionAdvice` to extend `ResponseEntityExceptionHandler` to handle a lot more built-in error scenarios * Change error handling to always return exception message for non-5xx errors * Change web3 message to status reason phrase and move prior message to detail * Remove accidentally committed debug code in build.gradle.kts * Remove redundant exception handlers now handled by super class --------- Signed-off-by: Steven Sheehy <[email protected]>
hashgraph · Aug 19, 2024 · 795dc49 · 795dc49
1 parent 0a568e3
commit 795dc49
Show file tree

Hide file tree

Showing 10 changed files with 174 additions and 149 deletions.
diff --git a/hedera-mirror-web3/build.gradle.kts b/hedera-mirror-web3/build.gradle.kts
@@ -160,7 +160,6 @@ tasks.compileTestJava {
     options.compilerArgs.add("--enable-preview")
     options.compilerArgs.removeIf { it == "-Werror" }
     dependsOn(compileHistoricalSolidityContracts)
-    doFirst { file("args").writeText(options.compilerArgs.toString()) }
 }
 
 tasks.openApiGenerate { mustRunAfter(tasks.named("resolveSolidity")) }

diff --git a/hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/config/LoggingFilter.java b/hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/config/LoggingFilter.java
@@ -16,6 +16,8 @@
 
 package com.hedera.mirror.web3.config;
 
+import static org.springframework.web.util.WebUtils.ERROR_EXCEPTION_ATTRIBUTE;
+
 import com.hedera.mirror.web3.Web3Properties;
 import jakarta.inject.Named;
 import jakarta.servlet.FilterChain;
@@ -24,6 +26,7 @@
 import lombok.CustomLog;
 import lombok.RequiredArgsConstructor;
 import org.apache.commons.lang3.StringUtils;
+import org.springframework.http.HttpStatus;
 import org.springframework.web.filter.OncePerRequestFilter;
 import org.springframework.web.util.ContentCachingRequestWrapper;
 import org.springframework.web.util.WebUtils;
@@ -36,7 +39,8 @@ class LoggingFilter extends OncePerRequestFilter {
     @SuppressWarnings("java:S1075")
     private static final String ACTUATOR_PATH = "/actuator/";
 
-    private static final String LOG_FORMAT = "{} {} {} in {} ms: {} - {}";
+    private static final String LOG_FORMAT = "{} {} {} in {} ms: {} {} - {}";
+    private static final String SUCCESS = "Success";
 
     private final Web3Properties web3Properties;
 
@@ -58,7 +62,7 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse
         }
     }
 
-    private void logRequest(HttpServletRequest request, HttpServletResponse response, long startTime, Exception cause) {
+    private void logRequest(HttpServletRequest request, HttpServletResponse response, long startTime, Exception e) {
         var uri = request.getRequestURI();
         boolean actuator = StringUtils.startsWith(uri, ACTUATOR_PATH);
 
@@ -68,19 +72,21 @@ private void logRequest(HttpServletRequest request, HttpServletResponse response
 
         long elapsed = System.currentTimeMillis() - startTime;
         var content = getContent(request);
-        var message = cause != null ? cause.getMessage() : response.getStatus();
-        var params = new Object[] {request.getRemoteAddr(), request.getMethod(), uri, elapsed, message, content};
+        var message = getMessage(request, e);
+        int status = response.getStatus();
+        var params =
+                new Object[] {request.getRemoteAddr(), request.getMethod(), uri, elapsed, status, message, content};
 
         if (actuator) {
             log.debug(LOG_FORMAT, params);
-        } else if (cause != null) {
+        } else if (status != HttpStatus.OK.value()) {
             log.warn(LOG_FORMAT, params);
         } else {
             log.info(LOG_FORMAT, params);
         }
     }
 
-    protected String getContent(HttpServletRequest request) {
+    private String getContent(HttpServletRequest request) {
         var wrapper = WebUtils.getNativeRequest(request, ContentCachingRequestWrapper.class);
 
         if (wrapper != null) {
@@ -89,4 +95,16 @@ protected String getContent(HttpServletRequest request) {
 
         return "";
     }
+
+    private String getMessage(HttpServletRequest request, Exception e) {
+        if (e != null) {
+            return e.getMessage();
+        }
+
+        if (request.getAttribute(ERROR_EXCEPTION_ATTRIBUTE) instanceof Exception ex) {
+            return ex.getMessage();
+        }
+
+        return SUCCESS;
+    }
 }
diff --git a/...-mirror-web3/src/main/java/com/hedera/mirror/web3/controller/GenericControllerAdvice.java b/...-mirror-web3/src/main/java/com/hedera/mirror/web3/controller/GenericControllerAdvice.java
@@ -16,21 +16,19 @@
 
 package com.hedera.mirror.web3.controller;
 
-import static com.hedera.mirror.web3.controller.ValidationErrorParser.extractValidationError;
 import static org.springframework.http.HttpStatus.BAD_REQUEST;
 import static org.springframework.http.HttpStatus.INTERNAL_SERVER_ERROR;
 import static org.springframework.http.HttpStatus.NOT_FOUND;
-import static org.springframework.http.HttpStatus.NOT_IMPLEMENTED;
 import static org.springframework.http.HttpStatus.SERVICE_UNAVAILABLE;
 import static org.springframework.http.HttpStatus.TOO_MANY_REQUESTS;
-import static org.springframework.http.HttpStatus.UNSUPPORTED_MEDIA_TYPE;
+import static org.springframework.web.context.request.RequestAttributes.SCOPE_REQUEST;
 
 import com.hedera.mirror.web3.exception.EntityNotFoundException;
 import com.hedera.mirror.web3.exception.InvalidInputException;
 import com.hedera.mirror.web3.exception.MirrorEvmTransactionException;
 import com.hedera.mirror.web3.exception.RateLimitException;
 import com.hedera.mirror.web3.viewmodel.GenericErrorResponse;
-import java.util.Optional;
+import com.hedera.mirror.web3.viewmodel.GenericErrorResponse.ErrorMessage;
 import lombok.CustomLog;
 import org.apache.commons.lang3.StringUtils;
 import org.jetbrains.annotations.NotNull;
@@ -39,20 +37,28 @@
 import org.springframework.core.Ordered;
 import org.springframework.core.annotation.Order;
 import org.springframework.dao.QueryTimeoutException;
+import org.springframework.http.HttpHeaders;
+import org.springframework.http.HttpStatus;
+import org.springframework.http.HttpStatusCode;
 import org.springframework.http.ResponseEntity;
 import org.springframework.http.converter.HttpMessageConversionException;
-import org.springframework.validation.BindException;
-import org.springframework.web.HttpMediaTypeNotSupportedException;
+import org.springframework.lang.Nullable;
+import org.springframework.validation.FieldError;
+import org.springframework.validation.ObjectError;
+import org.springframework.web.ErrorResponse;
+import org.springframework.web.bind.MethodArgumentNotValidException;
 import org.springframework.web.bind.annotation.ControllerAdvice;
 import org.springframework.web.bind.annotation.ExceptionHandler;
-import org.springframework.web.server.ServerWebInputException;
+import org.springframework.web.context.request.WebRequest;
 import org.springframework.web.servlet.config.annotation.CorsRegistry;
 import org.springframework.web.servlet.config.annotation.WebMvcConfigurer;
+import org.springframework.web.servlet.mvc.method.annotation.ResponseEntityExceptionHandler;
+import org.springframework.web.util.WebUtils;
 
 @ControllerAdvice
 @CustomLog
 @Order(Ordered.HIGHEST_PRECEDENCE)
-class GenericControllerAdvice {
+class GenericControllerAdvice extends ResponseEntityExceptionHandler {
 
     @Bean
     @SuppressWarnings("java:S5122") // Make sure that enabling CORS is safe here.
@@ -65,90 +71,76 @@ public void addCorsMappings(@NotNull CorsRegistry registry) {
         };
     }
 
-    /**
-     * Temporary handler, intended for dealing with forthcoming features that are not yet available, such as the absence
-     * of a precompile for gas estimation.
-     **/
     @ExceptionHandler
-    private ResponseEntity<?> unsupportedOpResponse(final UnsupportedOperationException e) {
-        return new ResponseEntity<>(new GenericErrorResponse(e.getMessage()), NOT_IMPLEMENTED);
-    }
-
-    @ExceptionHandler
-    private ResponseEntity<?> rateLimitError(final RateLimitException e) {
-        return new ResponseEntity<>(new GenericErrorResponse(e.getMessage()), TOO_MANY_REQUESTS);
+    private ResponseEntity<?> defaultExceptionHandler(final Exception e, WebRequest request) {
+        log.error("Generic error: ", e);
+        var headers = e instanceof ErrorResponse er ? er.getHeaders() : null;
+        return handleExceptionInternal(e, null, headers, INTERNAL_SERVER_ERROR, request);
     }
 
-    @ExceptionHandler
-    private ResponseEntity<?> validationError(final BindException e) {
-        final var errors = extractValidationError(e);
-        log.warn("Validation error: {}", errors);
-        return new ResponseEntity<>(new GenericErrorResponse(errors), BAD_REQUEST);
+    @ExceptionHandler({HttpMessageConversionException.class, IllegalArgumentException.class, InvalidInputException.class
+    })
+    private ResponseEntity<?> badRequest(final Exception e, final WebRequest request) {
+        return handleExceptionInternal(e, null, null, BAD_REQUEST, request);
     }
 
     @ExceptionHandler
-    private ResponseEntity<?> invalidInputError(final InvalidInputException e) {
-        log.warn("Input validation error: {}", e.getMessage());
-        return new ResponseEntity<>(new GenericErrorResponse(e.getMessage()), BAD_REQUEST);
+    private ResponseEntity<?> mirrorEvmTransactionError(final MirrorEvmTransactionException e, WebRequest request) {
+        log.warn("Mirror EVM transaction error: {}, detail: {}, data: {}", e.getMessage(), e.getDetail(), e.getData());
+        request.setAttribute(WebUtils.ERROR_EXCEPTION_ATTRIBUTE, e, SCOPE_REQUEST);
+        return new ResponseEntity<>(new GenericErrorResponse(e.getMessage(), e.getDetail(), e.getData()), BAD_REQUEST);
     }
 
     @ExceptionHandler
-    private ResponseEntity<?> illegalArgumentError(final IllegalArgumentException e) {
-        log.warn("Invalid argument error: {}", e.getMessage());
-        return new ResponseEntity<>(new GenericErrorResponse(e.getMessage()), BAD_REQUEST);
+    private ResponseEntity<?> notFoundException(final EntityNotFoundException e, final WebRequest request) {
+        return handleExceptionInternal(e, null, null, NOT_FOUND, request);
     }
 
     @ExceptionHandler
-    private ResponseEntity<?> typeMismatchError(final TypeMismatchException e) {
-        final var message = Optional.ofNullable(e.getRootCause()).orElse(e).getMessage();
-        log.warn("Type mismatch error for expected type {}: {}", e.getRequiredType(), message);
-        return new ResponseEntity<>(new GenericErrorResponse(message), BAD_REQUEST);
+    private ResponseEntity<?> queryTimeoutException(final QueryTimeoutException e, WebRequest request) {
+        return handleExceptionInternal(e, null, null, SERVICE_UNAVAILABLE, request);
     }
 
     @ExceptionHandler
-    private ResponseEntity<?> mirrorEvmTransactionError(final MirrorEvmTransactionException e) {
-        log.warn("Mirror EVM transaction error: {}, detail: {}, data: {}", e.getMessage(), e.getDetail(), e.getData());
-        return new ResponseEntity<>(new GenericErrorResponse(e.getMessage(), e.getDetail(), e.getData()), BAD_REQUEST);
+    private ResponseEntity<?> rateLimitException(final RateLimitException e, final WebRequest request) {
+        return handleExceptionInternal(e, null, null, TOO_MANY_REQUESTS, request);
     }
 
-    @ExceptionHandler
-    private ResponseEntity<?> httpMessageConversionError(final HttpMessageConversionException e) {
-        log.warn("Transaction body parsing error: {}", e.getMessage());
-        return new ResponseEntity<>(
-                new GenericErrorResponse("Unable to parse JSON", e.getMessage(), StringUtils.EMPTY), BAD_REQUEST);
+    @Nullable
+    @Override
+    protected ResponseEntity<Object> handleMethodArgumentNotValid(
+            MethodArgumentNotValidException ex, HttpHeaders headers, HttpStatusCode status, WebRequest request) {
+        var messages = ex.getAllErrors().stream().map(this::formatErrorMessage).toList();
+        request.setAttribute(WebUtils.ERROR_EXCEPTION_ATTRIBUTE, ex, SCOPE_REQUEST);
+        return new ResponseEntity<>(new GenericErrorResponse(messages), headers, status);
     }
 
-    @ExceptionHandler
-    private ResponseEntity<?> serverWebInputError(final ServerWebInputException e) {
-        log.warn("Transaction body parsing error: {}, reason: {}", e.getMessage(), e.getReason());
-        return new ResponseEntity<>(
-                new GenericErrorResponse(e.getReason(), e.getMessage(), StringUtils.EMPTY), BAD_REQUEST);
+    @Nullable
+    @Override
+    protected ResponseEntity<Object> handleTypeMismatch(
+            TypeMismatchException ex, HttpHeaders headers, HttpStatusCode status, WebRequest request) {
+        final var cause = ex.getRootCause() instanceof Exception rc ? rc : ex;
+        return handleExceptionInternal(cause, null, headers, status, request);
     }
 
-    @ExceptionHandler
-    private ResponseEntity<?> notFoundError(final EntityNotFoundException e) {
-        log.warn("Not found: {}", e.getMessage());
-        return new ResponseEntity<>(new GenericErrorResponse(e.getMessage()), NOT_FOUND);
+    @Nullable
+    @Override
+    protected ResponseEntity<Object> handleExceptionInternal(
+            Exception ex, @Nullable Object body, HttpHeaders headers, HttpStatusCode statusCode, WebRequest request) {
+        var message = statusCode instanceof HttpStatus hs ? hs.getReasonPhrase() : statusCode.toString();
+        var detail = !statusCode.is5xxServerError() ? ex.getMessage() : StringUtils.EMPTY; // Don't leak server errors
+        var genericErrorResponse = new GenericErrorResponse(message, detail, StringUtils.EMPTY);
+        request.setAttribute(WebUtils.ERROR_EXCEPTION_ATTRIBUTE, ex, SCOPE_REQUEST);
+        return new ResponseEntity<>(genericErrorResponse, headers, statusCode);
     }
 
-    @ExceptionHandler
-    private ResponseEntity<?> unsupportedMediaTypeError(final HttpMediaTypeNotSupportedException e) {
-        log.warn("Unsupported media type error: {}", e.getMessage());
-        return new ResponseEntity<>(
-                new GenericErrorResponse(UNSUPPORTED_MEDIA_TYPE.getReasonPhrase(), e.getMessage(), StringUtils.EMPTY),
-                UNSUPPORTED_MEDIA_TYPE);
-    }
+    private ErrorMessage formatErrorMessage(ObjectError error) {
+        var detail = error.getDefaultMessage();
 
-    @ExceptionHandler
-    private ResponseEntity<?> queryTimeoutError(final QueryTimeoutException ignored) {
-        return new ResponseEntity<>(
-                new GenericErrorResponse(SERVICE_UNAVAILABLE.getReasonPhrase()), SERVICE_UNAVAILABLE);
-    }
+        if (error instanceof FieldError fieldError) {
+            detail = fieldError.getField() + " field " + fieldError.getDefaultMessage();
+        }
 
-    @ExceptionHandler
-    private ResponseEntity<?> genericError(final Exception e) {
-        log.error("Generic error: ", e);
-        return new ResponseEntity<>(
-                new GenericErrorResponse(INTERNAL_SERVER_ERROR.getReasonPhrase()), INTERNAL_SERVER_ERROR);
+        return new ErrorMessage(BAD_REQUEST.getReasonPhrase(), detail, StringUtils.EMPTY);
     }
 }
diff --git a/...ra-mirror-web3/src/main/java/com/hedera/mirror/web3/controller/ValidationErrorParser.java b/...ra-mirror-web3/src/main/java/com/hedera/mirror/web3/controller/ValidationErrorParser.java
diff --git a/...-mirror-web3/src/main/java/com/hedera/mirror/web3/evm/store/DatabaseBackedStateFrame.java b/...-mirror-web3/src/main/java/com/hedera/mirror/web3/evm/store/DatabaseBackedStateFrame.java
@@ -25,8 +25,10 @@
 import java.util.stream.Collectors;
 import lombok.NonNull;
 
-/** A CachingStateFrame that answers reads by getting entities from some other source - a database! - and
- * disallows all local updates/deletes. */
+/**
+ * A CachingStateFrame that answers reads by getting entities from some other source - a database! - and disallows all
+ * local updates/deletes.
+ */
 public class DatabaseBackedStateFrame<K> extends CachingStateFrame<K> {
 
     @NonNull
@@ -62,23 +64,23 @@ public void setValue(
             @NonNull final UpdatableReferenceCache<K> cache,
             @NonNull final K key,
             @NonNull final Object value) {
-        throw new UnsupportedOperationException("Cannot add/update a value in a database-backed StateFrame");
+        throw new UnsupportedOperationException("Cannot set a value in a read only database");
     }
 
     @Override
     public void deleteValue(
             @NonNull final Class<?> klass, @NonNull final UpdatableReferenceCache<K> cache, @NonNull final K key) {
-        throw new UnsupportedOperationException("Cannot delete a value in a database-backed StateFrame");
+        throw new UnsupportedOperationException("Cannot delete a value in a read only database");
     }
 
     @Override
     public void updatesFromDownstream(@NonNull final CachingStateFrame<K> childFrame) {
-        throw new UnsupportedOperationException("Cannot commit to a database-backed StateFrame (oddly enough)");
+        throw new UnsupportedOperationException("Cannot commit to a read only database");
     }
 
     @Override
     public void commit() {
-        throw new UnsupportedOperationException("Cannot commit to a database-backed StateFrame (oddly enough)");
+        throw new UnsupportedOperationException("Cannot commit to a read only database");
     }
 
     /** Signals that a type error occurred with the _value_ type */

diff --git a/hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/viewmodel/GenericErrorResponse.java b/hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/viewmodel/GenericErrorResponse.java
@@ -29,19 +29,23 @@
 @Value
 @NoArgsConstructor
 public class GenericErrorResponse {
-    List<ErrorMessage> messages = new ArrayList<>();
+    private final List<ErrorMessage> messages = new ArrayList<>();
 
     public GenericErrorResponse(String message) {
-        messages.add(new ErrorMessage(message, StringUtils.EMPTY, StringUtils.EMPTY));
+        this(message, StringUtils.EMPTY, StringUtils.EMPTY);
+    }
+
+    public GenericErrorResponse(String message, String detail) {
+        this(message, detail, StringUtils.EMPTY);
     }
 
     public GenericErrorResponse(String message, String detailedMessage, String data) {
         final var errorMessage = new ErrorMessage(message, detailedMessage, data);
         messages.add(errorMessage);
     }
 
-    public GenericErrorResponse(List<String> errorMessages) {
-        errorMessages.forEach(m -> messages.add(new ErrorMessage(m, StringUtils.EMPTY, StringUtils.EMPTY)));
+    public GenericErrorResponse(List<ErrorMessage> errorMessages) {
+        this.messages.addAll(errorMessages);
     }
 
     @Value