From 25f5bc1d3052021617ea4c4a12cc5f6de66f3c42 Mon Sep 17 00:00:00 2001 From: Keith Wall Date: Tue, 19 Aug 2014 15:32:26 +0000 Subject: QPID-6016: [Java Broker] Web UI error handling Address review comments from Alex Rudyy git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@1618889 13f79535-47bb-0310-9956-ffa450edef68 --- .../plugin/servlet/rest/RestServlet.java | 39 ++++++++++++++++------ .../src/main/java/resources/index.html | 2 +- .../resources/js/qpid/common/grid/GridUpdater.js | 2 +- .../src/main/java/resources/js/qpid/common/util.js | 27 ++++----------- 4 files changed, 37 insertions(+), 33 deletions(-) (limited to 'qpid/java') diff --git a/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/RestServlet.java b/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/RestServlet.java index 171c0d1e1c..dc1f5bba46 100644 --- a/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/RestServlet.java +++ b/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/RestServlet.java @@ -17,7 +17,6 @@ package org.apache.qpid.server.management.plugin.servlet.rest; import java.io.IOException; -import java.io.PrintWriter; import java.io.Writer; import java.security.AccessControlException; import java.util.ArrayList; @@ -43,6 +42,7 @@ import org.apache.log4j.Logger; import org.codehaus.jackson.map.ObjectMapper; import org.codehaus.jackson.map.SerializationConfig; +import org.apache.qpid.server.configuration.IllegalConfigurationException; import org.apache.qpid.server.model.Broker; import org.apache.qpid.server.model.ConfiguredObject; import org.apache.qpid.server.util.urlstreamhandler.data.Handler; @@ -407,7 +407,7 @@ public class RestServlet extends AbstractServlet } catch (RuntimeException e) { - setResponseStatus(response, e); + setResponseStatus(request, response, e); } return; } @@ -423,7 +423,7 @@ public class RestServlet extends AbstractServlet Collection[] objects = new Collection[_hierarchy.length]; if (_hierarchy.length == 1) { - createOrUpdate(providedObject, _hierarchy[0], getBroker(), null, response); + createOrUpdate(providedObject, _hierarchy[0], getBroker(), null, request, response); } else { @@ -486,13 +486,14 @@ public class RestServlet extends AbstractServlet ConfiguredObject theParent = parents.remove(0); ConfiguredObject[] otherParents = parents.toArray(new ConfiguredObject[parents.size()]); - createOrUpdate(providedObject, objClass, theParent, otherParents, response); + createOrUpdate(providedObject, objClass, theParent, otherParents, request, response); } } private void createOrUpdate(Map providedObject, Class objClass, - ConfiguredObject theParent, ConfiguredObject[] otherParents, HttpServletResponse response) throws IOException + ConfiguredObject theParent, ConfiguredObject[] otherParents, HttpServletRequest request, + HttpServletResponse response) throws IOException { try { @@ -513,7 +514,7 @@ public class RestServlet extends AbstractServlet } catch (RuntimeException e) { - setResponseStatus(response, e); + setResponseStatus(request, response, e); } } @@ -552,24 +553,40 @@ public class RestServlet extends AbstractServlet return true; } - private void setResponseStatus(HttpServletResponse response, RuntimeException e) throws IOException + private void setResponseStatus(HttpServletRequest request, HttpServletResponse response, RuntimeException e) throws IOException { if (e instanceof AccessControlException) { if (LOGGER.isDebugEnabled()) { - LOGGER.debug("Caught security exception, sending " + HttpServletResponse.SC_FORBIDDEN, e); + LOGGER.debug("AccessControlException, sending " + HttpServletResponse.SC_FORBIDDEN, e); } response.setStatus(HttpServletResponse.SC_FORBIDDEN); } else { - LOGGER.warn("Caught exception", e); + if (e instanceof IllegalConfigurationException || e instanceof IllegalArgumentException) + { + if (LOGGER.isDebugEnabled()) + { + LOGGER.debug(e.getClass().getSimpleName() + " processing request : " + e.getMessage()); + } + else if (LOGGER.isTraceEnabled()) + { + LOGGER.trace(e.getClass().getSimpleName() + " processing request", e); + } + } + else + { + LOGGER.warn("Unexpected exception processing request ", e); + } + response.setStatus(HttpServletResponse.SC_CONFLICT); response.setContentType("application/json"); response.setCharacterEncoding("UTF-8"); - PrintWriter out = response.getWriter(); + + Writer out = getOutputWriter(request, response); ObjectMapper mapper = new ObjectMapper(); mapper.configure(SerializationConfig.Feature.INDENT_OUTPUT, true); mapper.writeValue(out, Collections.singletonMap("errorMessage", e.getMessage())); @@ -596,7 +613,7 @@ public class RestServlet extends AbstractServlet } catch(RuntimeException e) { - setResponseStatus(response, e); + setResponseStatus(request, response, e); } } diff --git a/qpid/java/broker-plugins/management-http/src/main/java/resources/index.html b/qpid/java/broker-plugins/management-http/src/main/java/resources/index.html index 3c0cb9a5f6..11f229237c 100644 --- a/qpid/java/broker-plugins/management-http/src/main/java/resources/index.html +++ b/qpid/java/broker-plugins/management-http/src/main/java/resources/index.html @@ -125,7 +125,7 @@
- +
diff --git a/qpid/java/broker-plugins/management-http/src/main/java/resources/js/qpid/common/grid/GridUpdater.js b/qpid/java/broker-plugins/management-http/src/main/java/resources/js/qpid/common/grid/GridUpdater.js index 8d58b6dec0..e5d96d44e7 100644 --- a/qpid/java/broker-plugins/management-http/src/main/java/resources/js/qpid/common/grid/GridUpdater.js +++ b/qpid/java/broker-plugins/management-http/src/main/java/resources/js/qpid/common/grid/GridUpdater.js @@ -160,7 +160,7 @@ define(["dojo/_base/xhr", if (this.serviceUrl) { var requestUrl = lang.isFunction(this.serviceUrl) ? this.serviceUrl() : this.serviceUrl; - xhr.get({url: requestUrl, sync: true, handleAs: "json"}).then(processData, util.errorHandler); + xhr.get({url: requestUrl, sync: true, handleAs: "json"}).then(processData, util.xhrErrorHandler); } else { diff --git a/qpid/java/broker-plugins/management-http/src/main/java/resources/js/qpid/common/util.js b/qpid/java/broker-plugins/management-http/src/main/java/resources/js/qpid/common/util.js index 441d6776e7..cb0cc792e8 100644 --- a/qpid/java/broker-plugins/management-http/src/main/java/resources/js/qpid/common/util.js +++ b/qpid/java/broker-plugins/management-http/src/main/java/resources/js/qpid/common/util.js @@ -345,15 +345,18 @@ define(["dojo/_base/xhr", var hasMessage = error.hasOwnProperty("message"); var message; - if (error.status == 401) + if (error.status == 0) { - message = hasMessage ? error.message : "Authentication required"; + message = "Unable to contact the Broker"; + } + else if (error.status == 401) + { + message = "Authentication required"; userMustReauth = true; } else if (error.status == 403) { - message = hasMessage ? error.message : "Forbidden"; - userMustReauth = true; + message = "Access Forbidden"; } else { @@ -398,22 +401,6 @@ define(["dojo/_base/xhr", } }; - util.errorHandler = function errorHandler(error) - { - if(error.status == 401) - { - alert("Authentication Failed"); - } - else if(error.status == 403) - { - alert("Access Denied"); - } - else - { - alert(error); - } - } - util.sendRequest = function (url, method, attributes, sync) { var success = false; -- cgit v1.2.1