From 0d0b99f4c0719c79850901bf4d8fcd0de4c8045b Mon Sep 17 00:00:00 2001 From: hns Date: Wed, 21 Sep 2005 10:11:10 +0000 Subject: [PATCH] * Fix exception handling: - Always print source file name and line number - Only print stack trace once - Slways print stack trace - Always print stack trace for original exception - Also log full error stack trace for exceptions caught in macros * Implement HopObject.__proto__ and JavaObject.__proto__ containing prototype object * Fix constructor property in HopObject protos to be set to the actual constructor * Implement JavaObject.__javaObject__ to contain the original java object in an unscripted wrapper * Make sure JS functions in script-extended java objects actually override java methods * Use unscripted wrapper rather than HopObject prototype if the prototype for java class is not defined --- .../framework/core/RequestEvaluator.java | 9 ++- src/helma/framework/core/Skin.java | 15 ++-- src/helma/scripting/ScriptingException.java | 79 +++---------------- src/helma/scripting/rhino/HopObject.java | 38 +++++---- src/helma/scripting/rhino/JavaObject.java | 24 +++++- src/helma/scripting/rhino/RhinoCore.java | 34 ++------ src/helma/scripting/rhino/RhinoEngine.java | 28 ++++--- 7 files changed, 96 insertions(+), 131 deletions(-) diff --git a/src/helma/framework/core/RequestEvaluator.java b/src/helma/framework/core/RequestEvaluator.java index a8e0560e..56e7be56 100644 --- a/src/helma/framework/core/RequestEvaluator.java +++ b/src/helma/framework/core/RequestEvaluator.java @@ -505,6 +505,7 @@ public final class RequestEvaluator implements Runnable { done = true; } } catch (Throwable x) { + String txname = localrtx.getTransactionName(); abortTransaction(); // If the transactor thread has been killed by the invoker thread we don't have to @@ -518,7 +519,6 @@ public final class RequestEvaluator implements Runnable { // check if we tried to process the error already if (error == null) { app.errorCount += 1; - app.logError("Error in " + Thread.currentThread(), x); // set done to false so that the error will be processed done = false; @@ -531,6 +531,13 @@ public final class RequestEvaluator implements Runnable { if (error == null) { error = "Unspecified error"; } + + if (x instanceof ScriptingException) { + x = ((ScriptingException) x).getWrappedException(); + } + + app.logError(txname + ": " + error, x); + } else { // error in error action. use traditional minimal error message res.writeErrorReport(app.getName(), error); diff --git a/src/helma/framework/core/Skin.java b/src/helma/framework/core/Skin.java index c33b3917..dda8983b 100644 --- a/src/helma/framework/core/Skin.java +++ b/src/helma/framework/core/Skin.java @@ -20,6 +20,7 @@ import helma.framework.*; import helma.framework.repository.Resource; import helma.objectmodel.ConcurrencyException; import helma.util.*; +import helma.scripting.ScriptingException; import java.util.*; import java.io.UnsupportedEncodingException; @@ -573,16 +574,18 @@ public final class Skin { } catch (TimeoutException timeout) { throw timeout; } catch (Exception x) { - // x.printStackTrace(); String msg = x.getMessage(); - if ((msg == null) || (msg.length() < 10)) { msg = x.toString(); } - - msg = "[Macro error in " + fullName + ": " + msg + "]"; - reval.getResponse().write(" " + msg + " "); - app.logEvent(msg); + msg = new StringBuffer("Macro error in ").append(fullName) + .append(": ").append(x.getMessage()).toString(); + reval.getResponse().write(" [" + msg + "] "); + // for ScriptingExceptions get the original exception + if (x instanceof ScriptingException) { + x = ((ScriptingException) x).getWrappedException(); + } + app.logError(msg, x); } } diff --git a/src/helma/scripting/ScriptingException.java b/src/helma/scripting/ScriptingException.java index 0e9f6f28..5af73019 100644 --- a/src/helma/scripting/ScriptingException.java +++ b/src/helma/scripting/ScriptingException.java @@ -23,83 +23,24 @@ import java.io.PrintWriter; * The base class for exceptions thrown by Helma scripting package */ public class ScriptingException extends Exception { + + // original exception that caused this ScriptingException to be thrown. Exception wrapped; /** - * Construct a ScriptingException given an error message + * Construct a ScriptingException given an error message and wrapped exception. */ - public ScriptingException(String msg) { + public ScriptingException(String msg, Exception wrapped) { super(msg); - wrapped = null; + this.wrapped = wrapped; } /** - * Construct a ScriptingException given an error message + * Get the original exception that caused this exception to be thrown. + * + * @return the wrapped exception */ - public ScriptingException(Exception w) { - wrapped = w; - } - - /** - * - * - * @return ... - */ - public String toString() { - if (wrapped == null) { - return super.toString(); - } else { - return wrapped.toString(); - } - } - - /** - * - * - * @return ... - */ - public String getMessage() { - if (wrapped == null) { - return super.getMessage(); - } else { - return wrapped.getMessage(); - } - } - - /** - * - */ - public void printStackTrace() { - if (wrapped == null) { - super.printStackTrace(); - } else { - wrapped.printStackTrace(); - } - } - - /** - * - * - * @param stream ... - */ - public void printStackTrace(PrintStream stream) { - if (wrapped == null) { - super.printStackTrace(stream); - } else { - wrapped.printStackTrace(stream); - } - } - - /** - * - * - * @param writer ... - */ - public void printStackTrace(PrintWriter writer) { - if (wrapped == null) { - super.printStackTrace(writer); - } else { - wrapped.printStackTrace(writer); - } + public Exception getWrappedException() { + return wrapped; } } diff --git a/src/helma/scripting/rhino/HopObject.java b/src/helma/scripting/rhino/HopObject.java index 1b8ca65c..42c41208 100644 --- a/src/helma/scripting/rhino/HopObject.java +++ b/src/helma/scripting/rhino/HopObject.java @@ -149,7 +149,7 @@ public class HopObject extends ScriptableObject implements Wrapper, PropertyReco hobj.init(core, node); if (proto != null) { - engine.invoke(hobj, "constructor", args, ScriptingEngine.ARGS_WRAP_NONE); + engine.invoke(hobj, "__constructor__", args, ScriptingEngine.ARGS_WRAP_NONE); } return hobj; @@ -710,22 +710,25 @@ public class HopObject extends ScriptableObject implements Wrapper, PropertyReco } /** + * Set a property in this HopObject * - * - * @param name ... - * @param start ... + * @param name property name + * @param start * @param value ... */ public void put(String name, Scriptable start, Object value) { - // register property for PropertyRecorder interface - if (isRecording) { - changedProperties.add(name); - } - if (node == null) { + // redirect the scripted constructor to __constructor__, + // constructor is set to the native constructor method. + if ("constructor".equals(name) && value instanceof NativeFunction) { + name = "__constructor__"; + } + // register property for PropertyRecorder interface + if (isRecording) { + changedProperties.add(name); + } super.put(name, start, value); } else { - checkNode(); if ("subnodeRelation".equals(name)) { @@ -769,12 +772,11 @@ public class HopObject extends ScriptableObject implements Wrapper, PropertyReco } /** + * Check if a property is set in this HopObject * - * - * @param name ... - * @param start ... - * - * @return ... + * @param name the property name + * @param start the object in which the lookup began + * @return true if the property was found */ public boolean has(String name, Scriptable start) { if (node != null) { @@ -895,8 +897,12 @@ public class HopObject extends ScriptableObject implements Wrapper, PropertyReco return node.getID(); } + if ("__proto__".equals(name)) { + return getPrototype(); // prototype object + } + if ("__prototype__".equals(name) || "_prototype".equals(name)) { - return node.getPrototype(); + return node.getPrototype(); // prototype name } if ("__parent__".equals(name) || "_parent".equals(name)) { diff --git a/src/helma/scripting/rhino/JavaObject.java b/src/helma/scripting/rhino/JavaObject.java index b0e5be1a..de7f3078 100644 --- a/src/helma/scripting/rhino/JavaObject.java +++ b/src/helma/scripting/rhino/JavaObject.java @@ -32,6 +32,7 @@ public class JavaObject extends NativeJavaObject { RhinoCore core; String protoName; + NativeJavaObject unscriptedJavaObj; static HashMap overload; @@ -57,6 +58,7 @@ public class JavaObject extends NativeJavaObject { this.protoName = protoName; this.core = core; staticType = obj.getClass(); + unscriptedJavaObj = new NativeJavaObject(scope, obj, staticType); setPrototype(prototype); initMembers(); } @@ -181,10 +183,30 @@ public class JavaObject extends NativeJavaObject { return new FunctionObject(name, (Method) obj, this); } - if ("_prototype".equals(name)) { + // we really are not supposed to walk down the prototype chain in get(), + // but we break the rule in order to be able to override java methods, + // which are looked up by super.get() below + Scriptable proto = getPrototype(); + while (proto != null) { + obj = proto.get(name, start); + if (obj != NOT_FOUND) { + return obj; + } + proto = proto.getPrototype(); + } + + if ("_prototype".equals(name) || "__prototype__".equals(name)) { return protoName; } + if ("__proto__".equals(name)) { + return getPrototype(); + } + + if ("__javaObject__".equals(name)) { + return unscriptedJavaObj; + } + return super.get(name, start); } diff --git a/src/helma/scripting/rhino/RhinoCore.java b/src/helma/scripting/rhino/RhinoCore.java index 122a5430..85630570 100644 --- a/src/helma/scripting/rhino/RhinoCore.java +++ b/src/helma/scripting/rhino/RhinoCore.java @@ -218,7 +218,8 @@ public final class RhinoCore implements ScopeProvider { // the actual (scripted) constructor on it. if (!"global".equals(lowerCaseName)) { try { - installConstructor(name, op); + FunctionObject fo = new FunctionObject(name, HopObject.hopObjCtor, global); + fo.addAsConstructor(global, op); } catch (Exception ignore) { System.err.println("Error adding ctor for " + name + ": " + ignore); ignore.printStackTrace(); @@ -285,29 +286,6 @@ public final class RhinoCore implements ScopeProvider { } } - /** - * This is a version of org.mozilla.javascript.FunctionObject.addAsConstructor() - * that does not set the constructor property in the prototype. This is because - * we want our own scripted constructor function to be visible, if it is defined. - * - * @param name the name of the constructor - * @param op the object prototype - */ - private void installConstructor(String name, Scriptable op) { - FunctionObject fo = new FunctionObject(name, HopObject.hopObjCtor, global); - - ScriptRuntime.setFunctionProtoAndParent(global, fo); - fo.setImmunePrototypeProperty(op); - - // add static getById() function - fo.defineProperty("getById", new GetById(name), GetById.ATTRIBUTES); - op.setParentScope(fo); - - ScriptableObject.defineProperty(global, name, fo, ScriptableObject.DONTENUM); - - fo.setParentScope(global); - } - /** * This method is called before an execution context is entered to let the * engine know it should update its prototype information. The update policy @@ -619,12 +597,12 @@ public final class RhinoCore implements ScopeProvider { Scriptable op = getPrototype(prototypeName); if (op == null) { - prototypeName = "HopObject"; - op = getPrototype("HopObject"); + // no prototype found, return an unscripted wrapper + w = new NativeJavaObject(global, e, e.getClass()); + } else { + w = new JavaObject(global, e, prototypeName, op, this); } - w = new JavaObject(global, e, prototypeName, op, this); - wrappercache.put(e, w); } diff --git a/src/helma/scripting/rhino/RhinoEngine.java b/src/helma/scripting/rhino/RhinoEngine.java index afc4f052..e1d4d8ff 100644 --- a/src/helma/scripting/rhino/RhinoEngine.java +++ b/src/helma/scripting/rhino/RhinoEngine.java @@ -314,9 +314,12 @@ public class RhinoEngine implements ScriptingEngine { // create and throw a ScriptingException with the right message String msg; if (x instanceof JavaScriptException) { - msg = ((JavaScriptException) x).getValue().toString(); + // created by javascript throw statement + msg = ((JavaScriptException) x).getMessage(); } else if (x instanceof WrappedException) { - Throwable wrapped = ((WrappedException) x).getWrappedException(); + // wrapped java excepiton + WrappedException wx = (WrappedException) x; + Throwable wrapped = wx.getWrappedException(); // if this is a wrapped concurrencyException, rethrow it. if (wrapped instanceof ConcurrencyException) { throw (ConcurrencyException) wrapped; @@ -325,17 +328,22 @@ public class RhinoEngine implements ScriptingEngine { if (wrapped instanceof RedirectException) { throw (RedirectException) wrapped; } - msg = wrapped.toString(); - } else { + // we need to build our own message here, default implementation is broken + StringBuffer b = new StringBuffer(wrapped.toString()); + b.append(" (").append(wx.getSourceName()).append("#") + .append(wx.getLineNumber()).append(")"); + msg = b.toString(); + // replace wrapper with original exception + if (wrapped instanceof Exception) { + x = (Exception) wrapped; + } + } else if (x instanceof EcmaError) { msg = x.toString(); + } else { + msg = x.getMessage(); } - if (app.debug()) { - System.err.println("Error in Script: " + msg); - x.printStackTrace(); - } - - throw new ScriptingException(msg); + throw new ScriptingException(msg, x); } }