Skip to content

Commit

Permalink
Closes #818 Send 404 instead of 500 for NoResourceFoundException (#821)
Browse files Browse the repository at this point in the history
  • Loading branch information
Ivan-Shaml committed Jan 12, 2024
1 parent 4cdbc1a commit c293989
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
package com.faforever.api.error;

import jakarta.servlet.ServletException;

import lombok.extern.slf4j.Slf4j;
import org.apache.catalina.connector.ClientAbortException;
import org.springframework.http.HttpStatus;
import org.springframework.security.access.AccessDeniedException;
import org.springframework.security.authentication.InsufficientAuthenticationException;
import org.springframework.web.HttpRequestMethodNotSupportedException;
import org.springframework.web.bind.MissingServletRequestParameterException;
import org.springframework.web.bind.annotation.ControllerAdvice;
import org.springframework.web.bind.annotation.ExceptionHandler;
Expand All @@ -15,6 +18,9 @@

import jakarta.validation.ConstraintViolationException;
import jakarta.validation.ValidationException;

import org.springframework.web.servlet.resource.NoResourceFoundException;

import java.text.MessageFormat;
import java.util.Arrays;
import java.util.concurrent.CompletionException;
Expand Down Expand Up @@ -68,6 +74,20 @@ public ErrorResponse processNotFoundException(NotFoundApiException ex) {
return createResponseFromApiException(ex, HttpStatus.NOT_FOUND);
}

@ExceptionHandler({HttpRequestMethodNotSupportedException.class, NoResourceFoundException.class})
@ResponseStatus(HttpStatus.NOT_FOUND)
@ResponseBody
public ErrorResponse processResourceNotFoundException(ServletException ex) {
log.debug("Resource could not be found", ex);
ErrorResponse response = new ErrorResponse();
response.addError(new ErrorResult(
String.valueOf(HttpStatus.NOT_FOUND.value()),
HttpStatus.NOT_FOUND.getReasonPhrase(),
ex.getMessage()
));
return response;
}

@ExceptionHandler(ApiException.class)
@ResponseStatus(HttpStatus.UNPROCESSABLE_ENTITY)
@ResponseBody
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,15 @@

import com.faforever.api.data.domain.Clan;
import com.faforever.api.data.domain.Player;

import jakarta.servlet.ServletException;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import org.springframework.http.HttpMethod;
import org.springframework.http.HttpStatus;

import jakarta.validation.ConstraintViolation;
Expand All @@ -12,10 +19,16 @@
import jakarta.validation.ValidationException;
import jakarta.validation.Validator;
import jakarta.validation.ValidatorFactory;

import org.springframework.web.HttpRequestMethodNotSupportedException;
import org.springframework.web.servlet.resource.NoResourceFoundException;

import java.text.MessageFormat;
import java.util.Set;
import java.util.stream.Stream;

import static org.junit.Assert.assertEquals;
import static org.junit.jupiter.api.Assertions.assertAll;

public class GlobalControllerExceptionHandlerTest {
private static final String COMMON_MESSAGE = "Error";
Expand Down Expand Up @@ -64,6 +77,20 @@ public void testProgrammingError() {
assertEquals(COMMON_MESSAGE, errorResult.getDetail());
}

@ParameterizedTest
@MethodSource(value = "servletExceptionSource")
public void testProcessResourceNotFoundException(final ServletException ex) {
ErrorResponse response = instance.processResourceNotFoundException(ex);

assertEquals(1, response.getErrors().size());
final ErrorResult errorResult = response.getErrors().get(0);

assertAll(
() -> assertEquals(HttpStatus.NOT_FOUND.getReasonPhrase(), errorResult.getTitle()),
() -> assertEquals(ex.getMessage(), errorResult.getDetail())
);
}

@Test
public void testValidationException() {
final ValidationException ex = new ValidationException(COMMON_MESSAGE);
Expand Down Expand Up @@ -102,4 +129,11 @@ public void testConstraintViolationException() {
assertEquals(ErrorCode.VALIDATION_FAILED.getTitle(), errorResult.getTitle());
assertEquals(String.valueOf(ErrorCode.VALIDATION_FAILED.getCode()), errorResult.getAppCode());
}

public static Stream<Arguments> servletExceptionSource() {
return Stream.of(
Arguments.of(new HttpRequestMethodNotSupportedException(HttpMethod.DELETE.name())),
Arguments.of(new NoResourceFoundException(HttpMethod.POST, "test/path"))
);
}
}

0 comments on commit c293989

Please sign in to comment.