SQOOP-629: Provide better exception handling during server-client communication (Jarek Jarcec Cecho)
Project: http://git-wip-us.apache.org/repos/asf/sqoop/repo Commit: http://git-wip-us.apache.org/repos/asf/sqoop/commit/5807db8c Tree: http://git-wip-us.apache.org/repos/asf/sqoop/tree/5807db8c Diff: http://git-wip-us.apache.org/repos/asf/sqoop/diff/5807db8c Branch: refs/heads/sqoop2 Commit: 5807db8c3ecfc7de30e529710b768308d7196e55 Parents: a46b25f Author: Bilung Lee <[email protected]> Authored: Wed Oct 17 09:38:15 2012 -0700 Committer: Bilung Lee <[email protected]> Committed: Wed Oct 17 09:38:15 2012 -0700 ---------------------------------------------------------------------- .../org/apache/sqoop/client/core/ClientError.java | 2 +- .../org/apache/sqoop/client/request/Request.java | 6 +- .../apache/sqoop/client/shell/DeleteCommand.java | 4 +- .../apache/sqoop/client/shell/SqoopCommand.java | 2 +- .../org/apache/sqoop/client/shell/SqoopShell.java | 7 + .../sqoop/client/utils/ThrowableDisplayer.java | 97 ++++++++++++ .../java/org/apache/sqoop/json/ExceptionInfo.java | 73 --------- .../java/org/apache/sqoop/json/ThrowableBean.java | 116 +++++++++++++++ .../org/apache/sqoop/json/TestThrowableBean.java | 49 ++++++ .../apache/sqoop/connector/ConnectorManager.java | 4 + .../sqoop/handler/ConnectorRequestHandler.java | 6 + .../apache/sqoop/server/SqoopProtocolServlet.java | 7 +- .../apache/sqoop/server/common/ServerError.java | 3 + 13 files changed, 293 insertions(+), 83 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/sqoop/blob/5807db8c/client/src/main/java/org/apache/sqoop/client/core/ClientError.java ---------------------------------------------------------------------- diff --git a/client/src/main/java/org/apache/sqoop/client/core/ClientError.java b/client/src/main/java/org/apache/sqoop/client/core/ClientError.java index 1cf42e4..fd3b97d 100644 --- a/client/src/main/java/org/apache/sqoop/client/core/ClientError.java +++ b/client/src/main/java/org/apache/sqoop/client/core/ClientError.java @@ -39,7 +39,7 @@ public enum ClientError implements ErrorCode { /** We're not able to get user input */ CLIENT_0005("Can't get user input"), - /** There occured exception on server side **/ + /** There occurred exception on server side **/ CLIENT_0006("Server has returned exception"), ; http://git-wip-us.apache.org/repos/asf/sqoop/blob/5807db8c/client/src/main/java/org/apache/sqoop/client/request/Request.java ---------------------------------------------------------------------- diff --git a/client/src/main/java/org/apache/sqoop/client/request/Request.java b/client/src/main/java/org/apache/sqoop/client/request/Request.java index 5e381c9..b243dfd 100644 --- a/client/src/main/java/org/apache/sqoop/client/request/Request.java +++ b/client/src/main/java/org/apache/sqoop/client/request/Request.java @@ -28,7 +28,7 @@ import com.sun.jersey.api.client.filter.ClientFilter; import org.apache.sqoop.client.core.ClientError; import org.apache.sqoop.common.SqoopException; import org.apache.sqoop.common.SqoopProtocolConstants; -import org.apache.sqoop.json.ExceptionInfo; +import org.apache.sqoop.json.ThrowableBean; import org.json.simple.JSONObject; import org.json.simple.JSONValue; @@ -91,13 +91,13 @@ public class Request if(resp.getHeaders().containsKey( SqoopProtocolConstants.HEADER_SQOOP_INTERNAL_ERROR_CODE)) { - ExceptionInfo ex = new ExceptionInfo(); + ThrowableBean ex = new ThrowableBean(); String responseText = resp.getEntity(String.class); JSONObject json = (JSONObject) JSONValue.parse(responseText); ex.restore(json); - throw new SqoopException(ClientError.CLIENT_0006, ex.toString()); + throw new SqoopException(ClientError.CLIENT_0006, ex.getThrowable()); } } http://git-wip-us.apache.org/repos/asf/sqoop/blob/5807db8c/client/src/main/java/org/apache/sqoop/client/shell/DeleteCommand.java ---------------------------------------------------------------------- diff --git a/client/src/main/java/org/apache/sqoop/client/shell/DeleteCommand.java b/client/src/main/java/org/apache/sqoop/client/shell/DeleteCommand.java index 1ea773b..bb09bf3 100644 --- a/client/src/main/java/org/apache/sqoop/client/shell/DeleteCommand.java +++ b/client/src/main/java/org/apache/sqoop/client/shell/DeleteCommand.java @@ -37,6 +37,7 @@ public class DeleteCommand extends SqoopCommand { "Delete", "info"); } + @Override @SuppressWarnings("unchecked") public Object execute(List args) { if (args.size() == 0) { @@ -59,5 +60,6 @@ public class DeleteCommand extends SqoopCommand { } else { String msg = "Usage: delete " + getUsage(); throw new SqoopException(ClientError.CLIENT_0002, msg); - } } + } + } } http://git-wip-us.apache.org/repos/asf/sqoop/blob/5807db8c/client/src/main/java/org/apache/sqoop/client/shell/SqoopCommand.java ---------------------------------------------------------------------- diff --git a/client/src/main/java/org/apache/sqoop/client/shell/SqoopCommand.java b/client/src/main/java/org/apache/sqoop/client/shell/SqoopCommand.java index c610fd4..9ae693e 100644 --- a/client/src/main/java/org/apache/sqoop/client/shell/SqoopCommand.java +++ b/client/src/main/java/org/apache/sqoop/client/shell/SqoopCommand.java @@ -137,4 +137,4 @@ public abstract class SqoopCommand extends ComplexCommandSupport } Collections.copy(arg, temp); } -} \ No newline at end of file +} http://git-wip-us.apache.org/repos/asf/sqoop/blob/5807db8c/client/src/main/java/org/apache/sqoop/client/shell/SqoopShell.java ---------------------------------------------------------------------- diff --git a/client/src/main/java/org/apache/sqoop/client/shell/SqoopShell.java b/client/src/main/java/org/apache/sqoop/client/shell/SqoopShell.java index d2b56ba..c9ac848 100644 --- a/client/src/main/java/org/apache/sqoop/client/shell/SqoopShell.java +++ b/client/src/main/java/org/apache/sqoop/client/shell/SqoopShell.java @@ -23,6 +23,8 @@ import java.io.FileReader; import java.util.HashSet; import java.util.Iterator; +import org.apache.sqoop.client.utils.ThrowableDisplayer; +import org.codehaus.groovy.runtime.MethodClosure; import org.codehaus.groovy.tools.shell.Command; import org.codehaus.groovy.tools.shell.CommandRegistry; import org.codehaus.groovy.tools.shell.Groovysh; @@ -45,6 +47,11 @@ public final class SqoopShell System.setProperty("groovysh.prompt", "sqoop"); Groovysh shell = new Groovysh(); + // Install our error hook (exception handling) + shell.setErrorHook( + new MethodClosure(ThrowableDisplayer.class, "errorHook")); + ThrowableDisplayer.setIo(shell.getIo()); + CommandRegistry registry = shell.getRegistry(); @SuppressWarnings("unchecked") Iterator<Command> iterator = registry.iterator(); http://git-wip-us.apache.org/repos/asf/sqoop/blob/5807db8c/client/src/main/java/org/apache/sqoop/client/utils/ThrowableDisplayer.java ---------------------------------------------------------------------- diff --git a/client/src/main/java/org/apache/sqoop/client/utils/ThrowableDisplayer.java b/client/src/main/java/org/apache/sqoop/client/utils/ThrowableDisplayer.java new file mode 100644 index 0000000..8d6e9b4 --- /dev/null +++ b/client/src/main/java/org/apache/sqoop/client/utils/ThrowableDisplayer.java @@ -0,0 +1,97 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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. + */ +package org.apache.sqoop.client.utils; + +import org.apache.sqoop.client.core.ClientError; +import org.apache.sqoop.common.SqoopException; +import org.codehaus.groovy.tools.shell.IO; + +/** + * Pretty printing of Throwable objects + */ +public class ThrowableDisplayer { + + /** + * Associated shell IO object. + * + * This objects needs to be set explicitly as some of the methods are called + * by Groovy shell without ability to pass additional arguments. + */ + private static IO io; + + public static void setIo(IO ioObject) { + io = ioObject; + } + + /** + * Error hook installed to Groovy shell. + * + * Will display exception that appeared during executing command. In most + * cases we will simply delegate the call to printing throwable method, + * however in case that we've received ClientError.CLIENT_0006 (server + * exception), we will unwrap server issue and view only that as local + * context shouldn't make any difference. + * + * @param t Throwable to be displayed + */ + public static void errorHook(Throwable t) { + io.out.println("@|red Exception has occurred during processing command |@"); + + // If this is server exception from server + if(t instanceof SqoopException + && ((SqoopException)t).getErrorCode() == ClientError.CLIENT_0006) { + io.out.print("@|red Server has returned exception: |@"); + printThrowable(io, t.getCause()); + } else { + printThrowable(io, t); + } + } + + /** + * Pretty print Throwable instance including stack trace and causes. + * + * @param io IO object to use for generating output + * @param t Throwable to display + */ + protected static void printThrowable(IO io, Throwable t) { + io.out.print("@|red Exception: |@"); + io.out.print(t.getClass().getName()); + io.out.print(" @|red Message: |@"); + io.out.print(t.getMessage()); + io.out.println(); + + io.out.println("Stack trace:"); + for(StackTraceElement e : t.getStackTrace()) { + io.out.print("\t @|bold at |@ "); + io.out.print(e.getClassName()); + io.out.print(" (@|bold " + e.getFileName() + ":" + + e.getLineNumber() + ") |@ "); + io.out.println(); + } + + Throwable cause = t.getCause(); + if(cause != null) { + io.out.print("Caused by: "); + printThrowable(io, cause); + } + } + + private ThrowableDisplayer() { + // Instantiation is prohibited + } +} http://git-wip-us.apache.org/repos/asf/sqoop/blob/5807db8c/common/src/main/java/org/apache/sqoop/json/ExceptionInfo.java ---------------------------------------------------------------------- diff --git a/common/src/main/java/org/apache/sqoop/json/ExceptionInfo.java b/common/src/main/java/org/apache/sqoop/json/ExceptionInfo.java deleted file mode 100644 index 68de4b1..0000000 --- a/common/src/main/java/org/apache/sqoop/json/ExceptionInfo.java +++ /dev/null @@ -1,73 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you 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. - */ -package org.apache.sqoop.json; - -import java.io.PrintWriter; -import java.io.StringWriter; - -import org.json.simple.JSONObject; - -public class ExceptionInfo implements JsonBean { - - public static final String ERROR_CODE = "error-code"; - public static final String ERROR_MESSAGE = "error-message"; - public static final String STACK_TRACE = "stack-trace"; - - private String errorCode; - private String errorMessage; - private String stackTrace; - - // For "extract" - public ExceptionInfo(String code, String message, Exception ex) { - errorCode = code; - errorMessage = message; - - StringWriter writer = new StringWriter(); - ex.printStackTrace(new PrintWriter(writer)); - writer.flush(); - - stackTrace = writer.getBuffer().toString(); - } - - // For "restore" - public ExceptionInfo() { - } - - @SuppressWarnings("unchecked") - @Override - public JSONObject extract() { - JSONObject result = new JSONObject(); - result.put(ERROR_CODE, errorCode); - result.put(ERROR_MESSAGE, errorMessage); - result.put(STACK_TRACE, stackTrace); - - return result; - } - - @Override - public void restore(JSONObject jsonObject) { - errorCode = (String) jsonObject.get(ERROR_CODE); - errorMessage = (String) jsonObject.get(ERROR_MESSAGE); - stackTrace = (String) jsonObject.get(STACK_TRACE); - } - - @Override - public String toString() { - return errorMessage; - } -} http://git-wip-us.apache.org/repos/asf/sqoop/blob/5807db8c/common/src/main/java/org/apache/sqoop/json/ThrowableBean.java ---------------------------------------------------------------------- diff --git a/common/src/main/java/org/apache/sqoop/json/ThrowableBean.java b/common/src/main/java/org/apache/sqoop/json/ThrowableBean.java new file mode 100644 index 0000000..91914e8 --- /dev/null +++ b/common/src/main/java/org/apache/sqoop/json/ThrowableBean.java @@ -0,0 +1,116 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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. + */ +package org.apache.sqoop.json; + +import org.json.simple.JSONArray; +import org.json.simple.JSONObject; + +import java.util.LinkedList; +import java.util.List; + +/** + * Transfer throwable. + * + * TODO(jarcec): After SQOOP-627 will get committed, change the throwable + * creation to same class as was on the server instead of Throwable. + */ +public class ThrowableBean implements JsonBean { + + public static final String MESSAGE = "message"; + public static final String STACK_TRACE = "stack-trace"; + public static final String CLASS = "class"; + public static final String METHOD = "method"; + public static final String FILE = "file"; + public static final String LINE = "line"; + public static final String CAUSE = "cause"; + + private Throwable throwable; + + // For "extract" + public ThrowableBean(Throwable ex) { + throwable = ex; + } + + // For "restore" + public ThrowableBean() { + } + + public Throwable getThrowable() { + return throwable; + } + + @Override + @SuppressWarnings("unchecked") + public JSONObject extract() { + JSONObject result = new JSONObject(); + + result.put(MESSAGE, throwable.getMessage()); + result.put(CLASS, throwable.getClass().getName()); + + JSONArray st = new JSONArray(); + for(StackTraceElement element : throwable.getStackTrace()) { + JSONObject obj = new JSONObject(); + + obj.put(CLASS, element.getClassName()); + obj.put(METHOD, element.getMethodName()); + obj.put(FILE, element.getFileName()); + obj.put(LINE, element.getLineNumber()); + + st.add(obj); + } + + result.put(STACK_TRACE, st); + + Throwable cause = throwable.getCause(); + if(cause != null) { + ThrowableBean causeBean = new ThrowableBean(cause); + result.put(CAUSE, causeBean.extract()); + } + + return result; + } + + @Override + public void restore(JSONObject jsonObject) { + throwable = new Throwable((String) jsonObject.get(MESSAGE)); + + List<StackTraceElement> st = new LinkedList<StackTraceElement>(); + for(Object object : (JSONArray)jsonObject.get(STACK_TRACE)) { + JSONObject json = (JSONObject)object; + StackTraceElement element = new StackTraceElement( + (String)json.get(CLASS), + (String)json.get(METHOD), + (String)json.get(FILE), + ((Long)json.get(LINE)).intValue() + ); + st.add(element); + } + + throwable.setStackTrace(st.toArray(new StackTraceElement[]{})); + + Object cause = jsonObject.get(CAUSE); + if(cause != null) { + JSONObject causeJson = (JSONObject)cause; + + ThrowableBean causeBean = new ThrowableBean(); + causeBean.restore(causeJson); + + throwable.initCause(causeBean.getThrowable()); + } + } +} http://git-wip-us.apache.org/repos/asf/sqoop/blob/5807db8c/common/src/test/java/org/apache/sqoop/json/TestThrowableBean.java ---------------------------------------------------------------------- diff --git a/common/src/test/java/org/apache/sqoop/json/TestThrowableBean.java b/common/src/test/java/org/apache/sqoop/json/TestThrowableBean.java new file mode 100644 index 0000000..19a0a27 --- /dev/null +++ b/common/src/test/java/org/apache/sqoop/json/TestThrowableBean.java @@ -0,0 +1,49 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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. + */ +package org.apache.sqoop.json; + +import junit.framework.TestCase; +import org.json.simple.JSONObject; +import org.json.simple.JSONValue; + +/** + * + */ +public class TestThrowableBean extends TestCase { + public void testSerialization() { + Throwable ex = new RuntimeException("A"); + ex.initCause(new Exception("B")); + + // Serialize it to JSON object + ThrowableBean bean = new ThrowableBean(ex); + JSONObject json = bean.extract(); + + // "Move" it across network in text form + String string = json.toJSONString(); + + // Retrieved transferred object + JSONObject retrievedJson = (JSONObject) JSONValue.parse(string); + ThrowableBean retrievedBean = new ThrowableBean(); + retrievedBean.restore(retrievedJson); + Throwable retrieved = retrievedBean.getThrowable(); + + assertEquals("A", retrieved.getMessage()); + assertEquals("B", retrieved.getCause().getMessage()); + assertNull(retrieved.getCause().getCause()); + } +} http://git-wip-us.apache.org/repos/asf/sqoop/blob/5807db8c/core/src/main/java/org/apache/sqoop/connector/ConnectorManager.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/sqoop/connector/ConnectorManager.java b/core/src/main/java/org/apache/sqoop/connector/ConnectorManager.java index e3bf0e1..f7228d3 100644 --- a/core/src/main/java/org/apache/sqoop/connector/ConnectorManager.java +++ b/core/src/main/java/org/apache/sqoop/connector/ConnectorManager.java @@ -77,6 +77,10 @@ public final class ConnectorManager { public static MConnector getConnectorMetadata(long connectorId) { ConnectorHandler handler = handlerMap.get(nameMap.get(connectorId)); + if(handler == null) { + return null; + } + return handler.getMetadata(); } http://git-wip-us.apache.org/repos/asf/sqoop/blob/5807db8c/server/src/main/java/org/apache/sqoop/handler/ConnectorRequestHandler.java ---------------------------------------------------------------------- diff --git a/server/src/main/java/org/apache/sqoop/handler/ConnectorRequestHandler.java b/server/src/main/java/org/apache/sqoop/handler/ConnectorRequestHandler.java index 81044ce..fda91fd 100644 --- a/server/src/main/java/org/apache/sqoop/handler/ConnectorRequestHandler.java +++ b/server/src/main/java/org/apache/sqoop/handler/ConnectorRequestHandler.java @@ -25,6 +25,7 @@ import org.apache.sqoop.json.ConnectorBean; import org.apache.sqoop.model.MConnector; import org.apache.sqoop.server.RequestContext; import org.apache.sqoop.server.RequestHandler; +import org.apache.sqoop.server.common.ServerError; import java.util.LinkedList; import java.util.List; @@ -58,6 +59,11 @@ public class ConnectorRequestHandler implements RequestHandler { } else { Long id = Long.parseLong(cid); + // Check that user is not asking for non existing connector id + if(!ConnectorManager.getConnectoIds().contains(id)) { + throw new SqoopException(ServerError.SERVER_0004, "Invalid id " + id); + } + connectors = new LinkedList<MConnector>(); bundles = new LinkedList<ResourceBundle>(); http://git-wip-us.apache.org/repos/asf/sqoop/blob/5807db8c/server/src/main/java/org/apache/sqoop/server/SqoopProtocolServlet.java ---------------------------------------------------------------------- diff --git a/server/src/main/java/org/apache/sqoop/server/SqoopProtocolServlet.java b/server/src/main/java/org/apache/sqoop/server/SqoopProtocolServlet.java index ece3d93..dc0764e 100644 --- a/server/src/main/java/org/apache/sqoop/server/SqoopProtocolServlet.java +++ b/server/src/main/java/org/apache/sqoop/server/SqoopProtocolServlet.java @@ -26,7 +26,7 @@ import javax.servlet.http.HttpServletResponse; import org.apache.log4j.Logger; import org.apache.sqoop.common.ErrorCode; -import org.apache.sqoop.json.ExceptionInfo; +import org.apache.sqoop.json.ThrowableBean; import org.apache.sqoop.common.SqoopException; import org.apache.sqoop.common.SqoopProtocolConstants; import org.apache.sqoop.common.SqoopResponseCode; @@ -136,11 +136,10 @@ public class SqoopProtocolServlet extends HttpServlet { SqoopProtocolConstants.HEADER_SQOOP_INTERNAL_ERROR_MESSAGE, ex.getMessage()); - ExceptionInfo exceptionInfo = new ExceptionInfo(ec.getCode(), - ex.getMessage(), ex); + ThrowableBean throwableBean = new ThrowableBean(ex); response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); - response.getWriter().write(exceptionInfo.extract().toJSONString()); + response.getWriter().write(throwableBean.extract().toJSONString()); } else { response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); } http://git-wip-us.apache.org/repos/asf/sqoop/blob/5807db8c/server/src/main/java/org/apache/sqoop/server/common/ServerError.java ---------------------------------------------------------------------- diff --git a/server/src/main/java/org/apache/sqoop/server/common/ServerError.java b/server/src/main/java/org/apache/sqoop/server/common/ServerError.java index 0a97d2d..02d9174 100644 --- a/server/src/main/java/org/apache/sqoop/server/common/ServerError.java +++ b/server/src/main/java/org/apache/sqoop/server/common/ServerError.java @@ -33,6 +33,9 @@ public enum ServerError implements ErrorCode { /** We've received invalid HTTP request */ SERVER_0003("Invalid HTTP request"), + /** Invalid argument in HTTP request */ + SERVER_0004("Invalid argument in HTTP request"), + ; private final String message;
