From: Lott, Christopher (cl778h) Date: Tue, 13 Aug 2019 13:06:13 +0000 (-0400) Subject: Revise controller error handling X-Git-Tag: R2~40^2 X-Git-Url: https://gerrit.o-ran-sc.org/r/gitweb?a=commitdiff_plain;h=c7a3ec6561b3b2ce1facbec092305977c3eba665;p=portal%2Fric-dashboard.git Revise controller error handling Change-Id: Icb4a546172c86e82ae35e8f97b72857cce62077b Signed-off-by: Lott, Christopher (cl778h) --- diff --git a/docs/release-notes.rst b/docs/release-notes.rst index 8b516960..d70516ef 100644 --- a/docs/release-notes.rst +++ b/docs/release-notes.rst @@ -38,6 +38,7 @@ Version 1.2.0, 15 Aug 2019 * Add controller for page refresh of Angular routes * Extend E2 mock configuration for demo purposes * Add pattern for matching AC/admin application name +* Add custom (plain but not white-label) error page Version 1.0.5, 5 July 2019 -------------------------- diff --git a/webapp-backend/src/main/java/org/oransc/ric/portal/dashboard/config/WebSecurityConfiguration.java b/webapp-backend/src/main/java/org/oransc/ric/portal/dashboard/config/WebSecurityConfiguration.java index 3477ab0a..4e243564 100644 --- a/webapp-backend/src/main/java/org/oransc/ric/portal/dashboard/config/WebSecurityConfiguration.java +++ b/webapp-backend/src/main/java/org/oransc/ric/portal/dashboard/config/WebSecurityConfiguration.java @@ -32,6 +32,7 @@ import org.oransc.ric.portal.dashboard.controller.AdminController; import org.oransc.ric.portal.dashboard.controller.AnrXappController; import org.oransc.ric.portal.dashboard.controller.AppManagerController; import org.oransc.ric.portal.dashboard.controller.E2ManagerController; +import org.oransc.ric.portal.dashboard.controller.SimpleErrorController; import org.oransc.ric.portal.dashboard.portalapi.DashboardUserManager; import org.oransc.ric.portal.dashboard.portalapi.PortalAuthManager; import org.oransc.ric.portal.dashboard.portalapi.PortalAuthenticationFilter; @@ -102,6 +103,7 @@ public class WebSecurityConfiguration extends WebSecurityConfigurerAdapter { AppManagerController.CONTROLLER_PATH + "/" + AppManagerController.VERSION_METHOD, // E2ManagerController.CONTROLLER_PATH + "/" + E2ManagerController.HEALTH_METHOD, // E2ManagerController.CONTROLLER_PATH + "/" + E2ManagerController.VERSION_METHOD, // + SimpleErrorController.ERROR_PATH, // DashboardConstants.LOGIN_PAGE // }; diff --git a/webapp-backend/src/main/java/org/oransc/ric/portal/dashboard/controller/AcXappController.java b/webapp-backend/src/main/java/org/oransc/ric/portal/dashboard/controller/AcXappController.java index 655b47aa..570bd418 100644 --- a/webapp-backend/src/main/java/org/oransc/ric/portal/dashboard/controller/AcXappController.java +++ b/webapp-backend/src/main/java/org/oransc/ric/portal/dashboard/controller/AcXappController.java @@ -45,10 +45,12 @@ import io.swagger.annotations.ApiOperation; import io.swagger.annotations.ApiParam; /** - * * Proxies calls from the front end to the AC xApp via the A1 Mediator API. - * All methods answer 502 on failure:
HTTP server received an - * invalid response from a server it consulted when acting as a proxy or - * gateway.
+ * Proxies calls from the front end to the AC xApp via the A1 Mediator API. + * + * If a method throws RestClientResponseException, it is handled by + * {@link CustomResponseEntityExceptionHandler#handleProxyMethodException(Exception, org.springframework.web.context.request.WebRequest)} + * which returns status 502. All other exceptions are handled by Spring which + * returns status 500. */ @RestController @RequestMapping(value = AcXappController.CONTROLLER_PATH, produces = MediaType.APPLICATION_JSON_VALUE) diff --git a/webapp-backend/src/main/java/org/oransc/ric/portal/dashboard/controller/AnrXappController.java b/webapp-backend/src/main/java/org/oransc/ric/portal/dashboard/controller/AnrXappController.java index a05c25e8..ae6845bb 100644 --- a/webapp-backend/src/main/java/org/oransc/ric/portal/dashboard/controller/AnrXappController.java +++ b/webapp-backend/src/main/java/org/oransc/ric/portal/dashboard/controller/AnrXappController.java @@ -50,8 +50,13 @@ import org.springframework.web.bind.annotation.RestController; import io.swagger.annotations.ApiOperation; /** - * Provides methods to contact the ANR xApp which manages a Neighbor Cell - * Relation Table (NCRT). + * Proxies calls from the front end to the ANR xApp, which manages a Neighbor + * Cell Relation Table (NCRT). + * + * If a method throws RestClientResponseException, it is handled by + * {@link CustomResponseEntityExceptionHandler#handleProxyMethodException(Exception, org.springframework.web.context.request.WebRequest)} + * which returns status 502. All other exceptions are handled by Spring which + * returns status 500. */ @Configuration @RestController diff --git a/webapp-backend/src/main/java/org/oransc/ric/portal/dashboard/controller/AppManagerController.java b/webapp-backend/src/main/java/org/oransc/ric/portal/dashboard/controller/AppManagerController.java index 365a5803..3f2e8ac0 100644 --- a/webapp-backend/src/main/java/org/oransc/ric/portal/dashboard/controller/AppManagerController.java +++ b/webapp-backend/src/main/java/org/oransc/ric/portal/dashboard/controller/AppManagerController.java @@ -56,9 +56,12 @@ import org.springframework.web.bind.annotation.RestController; import io.swagger.annotations.ApiOperation; /** - * Proxies calls from the front end to the App Manager API. All methods answer - * 502 on failure:
HTTP server received an invalid response from a - * server it consulted when acting as a proxy or gateway.
+ * Proxies calls from the front end to the App Manager API. + * + * If a method throws RestClientResponseException, it is handled by + * {@link CustomResponseEntityExceptionHandler#handleProxyMethodException(Exception, org.springframework.web.context.request.WebRequest)} + * which returns status 502. All other exceptions are handled by Spring which + * returns status 500. */ @Configuration @RestController diff --git a/webapp-backend/src/main/java/org/oransc/ric/portal/dashboard/controller/CustomResponseEntityExceptionHandler.java b/webapp-backend/src/main/java/org/oransc/ric/portal/dashboard/controller/CustomResponseEntityExceptionHandler.java index 6d8afa7d..2c432d54 100644 --- a/webapp-backend/src/main/java/org/oransc/ric/portal/dashboard/controller/CustomResponseEntityExceptionHandler.java +++ b/webapp-backend/src/main/java/org/oransc/ric/portal/dashboard/controller/CustomResponseEntityExceptionHandler.java @@ -29,17 +29,14 @@ import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.ControllerAdvice; import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.client.HttpStatusCodeException; +import org.springframework.web.client.RestClientResponseException; import org.springframework.web.context.request.WebRequest; import org.springframework.web.servlet.mvc.method.annotation.ResponseEntityExceptionHandler; /** - * Catches Http status code exceptions and builds a response with code 502 and - * some details wrapped in an ErrorTransport object. This factors out try-catch + * Catches certain exceptions. This controller advice factors out try-catch * blocks in many controller methods. * - * Why 502? I quote:
HTTP server received an invalid response from a - * server it consulted when acting as a proxy or gateway.
- * * Also see:
* https://www.baeldung.com/exception-handling-for-rest-with-spring * https://www.springboottutorial.com/spring-boot-exception-handling-for-rest-services @@ -50,23 +47,36 @@ public class CustomResponseEntityExceptionHandler extends ResponseEntityExceptio // Superclass has "logger" that is exposed here, so use a different name private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); - /* - * Generates the response when a REST controller method takes an - * HttpStatusCodeException. + /** + * Logs the error and generates a JSON response when a REST controller method + * takes a RestClientResponseException. This is thrown by the Http client when a + * remote method returns a non-2xx code. All the controller methods are proxies + * in that they just forward the request along to a remote system, so if that + * remote system fails, return 502 plus some details about the failure, rather + * than the generic 500 that Spring-Boot will return on an uncaught exception. + * + * Why 502? I quote:
HTTP server received an invalid response from a + * server it consulted when acting as a proxy or gateway.
* - * @param ex The exception + * @param ex + * The exception * - * @param request The original request + * @param request + * The original request * * @return A response entity with status code 502 plus some details in the body. */ - @ExceptionHandler(HttpStatusCodeException.class) - public final ResponseEntity handleHttpStatusCodeException(HttpStatusCodeException ex, - WebRequest request) { - log.warn("handleHttpStatusCodeException: request {}, status code {}", request.getDescription(false), - ex.getStatusCode()); - return new ResponseEntity<>(new ErrorTransport(ex.getRawStatusCode(), ex.getResponseBodyAsString(), ex), - HttpStatus.BAD_GATEWAY); + @ExceptionHandler({ RestClientResponseException.class }) + public final ResponseEntity handleProxyMethodException(Exception ex, WebRequest request) { + // Capture the full stack trace in the log. + log.error("handleProxyMethodException: request {}, exception {}", request.getDescription(false), ex); + if (ex instanceof HttpStatusCodeException) { + HttpStatusCodeException hsce = (HttpStatusCodeException) ex; + return new ResponseEntity<>(new ErrorTransport(hsce.getRawStatusCode(), hsce.getResponseBodyAsString(), + ex.toString(), request.getDescription(false)), HttpStatus.BAD_GATEWAY); + } else { + return new ResponseEntity<>(new ErrorTransport(500, ex), HttpStatus.BAD_GATEWAY); + } } } diff --git a/webapp-backend/src/main/java/org/oransc/ric/portal/dashboard/controller/E2ManagerController.java b/webapp-backend/src/main/java/org/oransc/ric/portal/dashboard/controller/E2ManagerController.java index d06aeff0..a8b64b32 100644 --- a/webapp-backend/src/main/java/org/oransc/ric/portal/dashboard/controller/E2ManagerController.java +++ b/webapp-backend/src/main/java/org/oransc/ric/portal/dashboard/controller/E2ManagerController.java @@ -54,10 +54,12 @@ import org.springframework.web.client.HttpStatusCodeException; import io.swagger.annotations.ApiOperation; /** - * Proxies calls from the front end to the E2 Manager API. All methods answer - * 502 on failure and wrap the remote details:
HTTP server received - * an invalid response from a server it consulted when acting as a proxy or - * gateway.
+ * Proxies calls from the front end to the E2 Manager API. + * + * If a method throws RestClientResponseException, it is handled by + * {@link CustomResponseEntityExceptionHandler#handleProxyMethodException(Exception, org.springframework.web.context.request.WebRequest)} + * which returns status 502. All other exceptions are handled by Spring which + * returns status 500. */ @Configuration @RestController diff --git a/webapp-backend/src/main/java/org/oransc/ric/portal/dashboard/controller/SimpleErrorController.java b/webapp-backend/src/main/java/org/oransc/ric/portal/dashboard/controller/SimpleErrorController.java new file mode 100644 index 00000000..3102e4e1 --- /dev/null +++ b/webapp-backend/src/main/java/org/oransc/ric/portal/dashboard/controller/SimpleErrorController.java @@ -0,0 +1,73 @@ +/*- + * ========================LICENSE_START================================= + * O-RAN-SC + * %% + * Copyright (C) 2019 AT&T Intellectual Property and Nokia + * %% + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * ========================LICENSE_END=================================== + */ + +package org.oransc.ric.portal.dashboard.controller; + +import java.lang.invoke.MethodHandles; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.boot.web.servlet.error.ErrorController; +import org.springframework.http.MediaType; +import org.springframework.stereotype.Controller; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RequestMapping; + +import springfox.documentation.annotations.ApiIgnore; + +/** + * Provides a controller which is invoked on any error within the Spring-managed + * context, including page not found, and redirects the caller to a custom error + * page. The caller is also redirected to this page if a REST controller takes + * an uncaught exception. + * + * If trace is requested via request parameter ("?trace=true") and available, + * adds stack trace information to the standard JSON error response. + * + * Excluded from Swagger API documentation. + * + * https://stackoverflow.com/questions/25356781/spring-boot-remove-whitelabel-error-page + * https://www.baeldung.com/spring-boot-custom-error-page + */ + +@ApiIgnore +@Controller +@RequestMapping(value = SimpleErrorController.ERROR_PATH, produces = MediaType.APPLICATION_JSON_VALUE) +public class SimpleErrorController implements ErrorController { + + private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + + public static final String ERROR_PATH = "/error"; + + @Override + public String getErrorPath() { + logger.warn("getErrorPath"); + return ERROR_PATH; + } + + @GetMapping + public String handleError() { + logger.warn("handleError"); + // Return the name of the page INCLUDING suffix, which I guess is a "view" name. + // Just "error" is not enough, but don't seem to need a ModelAndView object. + return "error.html"; + } + +} \ No newline at end of file diff --git a/webapp-backend/src/main/java/org/oransc/ric/portal/dashboard/model/ErrorTransport.java b/webapp-backend/src/main/java/org/oransc/ric/portal/dashboard/model/ErrorTransport.java index 2d3a5c23..c0157067 100644 --- a/webapp-backend/src/main/java/org/oransc/ric/portal/dashboard/model/ErrorTransport.java +++ b/webapp-backend/src/main/java/org/oransc/ric/portal/dashboard/model/ErrorTransport.java @@ -20,14 +20,19 @@ package org.oransc.ric.portal.dashboard.model; +import java.time.Instant; + /** - * Model for message returned on failure, to be serialized as JSON. + * This mimics the model Spring-Boot uses for a message returned on failure, to + * be serialized as JSON. */ public class ErrorTransport implements IDashboardResponse { + private Instant timestamp; private Integer status; + private String error; private String message; - private String exception; + private String path; /** * Builds an empty object. @@ -37,38 +42,52 @@ public class ErrorTransport implements IDashboardResponse { } /** - * Builds an object with the specified values. + * Convenience constructor for minimal value set. + * + * @param status + * Integer value like 400 + * @param error + * Error message + */ + public ErrorTransport(int status, String error) { + this(status, error, null, null); + } + + /** + * Convenience constructor for populating an error from an exception * - * @param statusCode - * Integer value like 400 - * @param errMsg - * Explanation + * @param status + * Integer value like 400 + * @param throwable + * The caught exception/throwable to convert to String with + * an upper bound on characters */ - public ErrorTransport(int statusCode, String errMsg) { - this(statusCode, errMsg, null); + public ErrorTransport(int status, Throwable throwable) { + this.timestamp = Instant.now(); + this.status = status; + final int enough = 256; + String exString = throwable.toString(); + this.error = exString.length() > enough ? exString.substring(0, enough) : exString; } /** - * Builds an object with the specified status code, message and a String version - * of the exception. + * Builds an object with all fields * - * @param statusCode - * Integer value like 500 - * @param errMsg - * Explanation - * @param exception - * Exception that should be reported; optional and ignored - * if null. + * @param status + * Integer value like 500 + * @param error + * Explanation + * @param message + * Additional explanation + * @param path + * Requested path */ - public ErrorTransport(int statusCode, String errMsg, Exception exception) { - this.status = statusCode; - this.message = errMsg; - if (exception != null) { - final int enough = 512; - String exString = exception.toString(); - String exceptionMsg = exString.length() > enough ? exString.substring(0, enough) : exString; - this.exception = exceptionMsg; - } + public ErrorTransport(int status, String error, String message, String path) { + this.timestamp = Instant.now(); + this.status = status; + this.error = error; + this.message = message; + this.path = path; } public Integer getStatus() { @@ -87,12 +106,28 @@ public class ErrorTransport implements IDashboardResponse { this.message = error; } - public String getException() { - return exception; + public Instant getTimestamp() { + return timestamp; + } + + public void setTimestamp(Instant timestamp) { + this.timestamp = timestamp; + } + + public String getError() { + return error; + } + + public void setError(String error) { + this.error = error; + } + + public String getPath() { + return path; } - public void setException(String exception) { - this.exception = exception; + public void setPath(String path) { + this.path = path; } } diff --git a/webapp-backend/src/main/resources/static/error.html b/webapp-backend/src/main/resources/static/error.html new file mode 100644 index 00000000..ae9d6cc9 --- /dev/null +++ b/webapp-backend/src/main/resources/static/error.html @@ -0,0 +1,36 @@ + + + + + +Static error page + + + +

RIC Dashboard Error

+

The previous request could not be processed.

+Click here to reload the application + + diff --git a/webapp-backend/src/test/java/org/oransc/ric/portal/dashboard/controller/AdminController2.java b/webapp-backend/src/test/java/org/oransc/ric/portal/dashboard/controller/AdminController2.java new file mode 100644 index 00000000..218d5e1b --- /dev/null +++ b/webapp-backend/src/test/java/org/oransc/ric/portal/dashboard/controller/AdminController2.java @@ -0,0 +1,57 @@ +/*- + * ========================LICENSE_START================================= + * O-RAN-SC + * %% + * Copyright (C) 2019 AT&T Intellectual Property and Nokia + * %% + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * ========================LICENSE_END=================================== + */ +package org.oransc.ric.portal.dashboard.controller; + +import java.lang.invoke.MethodHandles; + +import org.oransc.ric.portal.dashboard.DashboardConstants; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.http.MediaType; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RestController; +import org.springframework.web.client.RestClientResponseException; + +/** + * Provides methods that throw exceptions to support testing. + */ +@RestController +@RequestMapping(value = AdminController.CONTROLLER_PATH, produces = MediaType.APPLICATION_JSON_VALUE) +public class AdminController2 { + + private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + + // Publish paths in constants so tests are easy to write + public static final String CONTROLLER_PATH = DashboardConstants.ENDPOINT_PREFIX + "/admin"; + + @GetMapping("throw1") + public void throw1() { + logger.warn("throwing RestClientResponseException"); + throw new RestClientResponseException("foo", 0, "bar", null, null, null); + } + + @GetMapping("throw2") + public void throw2() { + logger.warn("throwing RuntimeException"); + throw new RuntimeException("throw2"); + } + +}