better error handling and error formatting

context.go

@@ -5,42 +5,10 @@ package v8go
 import "C"
 import (
 	"fmt"
-	"io"
 	"runtime"
 	"unsafe"
 )
 
-type jsErr struct {
-	msg      string
-	location string
-	stack    string
-}
-
-func (e *jsErr) Error() string {
-	return e.msg
-}
-
-func (e *jsErr) Format(s fmt.State, verb rune) {
-	switch verb {
-	case 'v':
-		if s.Flag('+') {
-			io.WriteString(s, e.msg)
-			if e.location != "" {
-				fmt.Fprintf(s, ": %s", e.location)
-			}
-			if e.stack != "" {
-				fmt.Fprintf(s, "\n%s", e.stack)
-			}
-			return
-		}
-		fallthrough
-	case 's':
-		io.WriteString(s, e.msg)
-	case 'q':
-		fmt.Fprintf(s, "%q", e.msg)
-	}
-}
-
 // Context is a global root execution environment that allows separate,
 // unrelated, JavaScript applications to run in a single instance of V8.
 type Context struct {
@@ -79,7 +47,8 @@ func (c *Context) Isolate() (*Isolate, error) {
 }
 
 // RunScript executes the source JavaScript; origin or filename provides a
-// reference for the script and used in the exception stack trace.
+// reference for the script and used in the stack trace if there is an error.
+// error will be of type `JSError` of not nil.
 func (c *Context) RunScript(source string, origin string) (*Value, error) {
 	cSource := C.CString(source)
 	cOrigin := C.CString(origin)
@@ -106,8 +75,13 @@ func getValue(rtn C.RtnValue) *Value {
 }
 
 func getError(rtn C.RtnValue) error {
-	if rtn.error == nil {
+	if rtn.error.msg == nil {
 		return nil
 	}
-	return &jsErr{msg: C.GoString(rtn.error), location: "blah line 8", stack: "bad things happen"}
+	err := &JSError{
+		Message:    C.GoString(rtn.error.msg),
+		Location:   C.GoString(rtn.error.location),
+		StackTrace: C.GoString(rtn.error.stack),
+	}
+	return err
 }

context_test.go

@@ -29,8 +29,32 @@ func TestContextExec(t *testing.T) {
 	}
 }
 
-func TestBadScript(t *testing.T) {
+func TestJSExceptions(t *testing.T) {
+	t.Parallel()
+
+	tests := [...]struct {
+		name   string
+		source string
+		origin string
+		err    string
+	}{
+		{"SyntaxError", "bad js syntax", "syntax.js", "SyntaxError: Unexpected identifier"},
+		{"ReferenceError", "add()", "add.js", "ReferenceError: add is not defined"},
+	}
+
 	ctx, _ := v8go.NewContext(nil)
-	_, err := ctx.RunScript("bad script", "bad.js")
-	t.Errorf("error: %+v", err)
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			_, err := ctx.RunScript(tt.source, tt.origin)
+			if err == nil {
+				t.Error("error expected but got <nil>")
+				return
+			}
+			if err.Error() != tt.err {
+				t.Errorf("expected %q, got %q", tt.err, err.Error())
+			}
+		})
+	}
 }

errors.go

@@ -0,0 +1,36 @@
+package v8go
+
+import (
+	"fmt"
+	"io"
+)
+
+// JSError is an error that is returned if there is are any
+// JavaScript exceptions handled in the context. When used with the fmt
+// verb `%+v`, will output the JavaScript stack trace, if available.
+type JSError struct {
+	Message    string
+	Location   string
+	StackTrace string
+}
+
+func (e *JSError) Error() string {
+	return e.Message
+}
+
+func (e *JSError) Format(s fmt.State, verb rune) {
+	switch verb {
+	case 'v':
+		if s.Flag('+') {
+			if e.StackTrace != "" {
+				io.WriteString(s, e.StackTrace)
+				return
+			}
+		}
+		fallthrough
+	case 's':
+		io.WriteString(s, e.Message)
+	case 'q':
+		fmt.Fprintf(s, "%q", e.Message)
+	}
+}

errors_test.go

@@ -0,0 +1,86 @@
+package v8go_test
+
+import (
+	"fmt"
+	"testing"
+
+	"rogchap.com/v8go"
+)
+
+func TestFormatting(t *testing.T) {
+	t.Parallel()
+	tests := [...]struct {
+		name            string
+		err             error
+		defaultVerb     string
+		defaultVerbFlag string
+		stringVerb      string
+		quoteVerb       string
+	}{
+		{"WithStack", &v8go.JSError{Message: "msg", StackTrace: "stack"}, "msg", "stack", "msg", `"msg"`},
+		{"WithoutStack", &v8go.JSError{Message: "msg"}, "msg", "msg", "msg", `"msg"`},
+	}
+
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			if s := fmt.Sprintf("%v", tt.err); s != tt.defaultVerb {
+				t.Errorf("incorrect format for %%v: %s", s)
+			}
+			if s := fmt.Sprintf("%+v", tt.err); s != tt.defaultVerbFlag {
+				t.Errorf("incorrect format for %%+v: %s", s)
+			}
+			if s := fmt.Sprintf("%s", tt.err); s != tt.stringVerb {
+				t.Errorf("incorrect format for %%s: %s", s)
+			}
+			if s := fmt.Sprintf("%q", tt.err); s != tt.quoteVerb {
+				t.Errorf("incorrect format for %%q: %s", s)
+			}
+		})
+	}
+}
+
+func TestJSErrorOutput(t *testing.T) {
+	t.Parallel()
+	ctx, _ := v8go.NewContext(nil)
+
+	math := `
+	function add(a, b) {
+		return a + b;
+	}
+
+	function addMore(a, b) {
+		return add(a, c);
+	}`
+
+	main := `
+	let a = add(3, 5);
+	let b = addMore(a, 6);
+	b;
+	`
+
+	ctx.RunScript(math, "math.js")
+	_, err := ctx.RunScript(main, "main.js")
+	if err == nil {
+		t.Error("expected error but got <nil>")
+		return
+	}
+	e, ok := err.(*v8go.JSError)
+	if !ok {
+		t.Errorf("expected error of type JSError, got %T", err)
+	}
+	if e.Message != "ReferenceError: c is not defined" {
+		t.Errorf("unexpected error message: %q", e.Message)
+	}
+	if e.Location != "math.js:7:17" {
+		t.Errorf("unexpected error location: %q", e.Location)
+	}
+	expectedStack := `ReferenceError: c is not defined
+    at addMore (math.js:7:17)
+    at main.js:3:10`
+
+	if e.StackTrace != expectedStack {
+		t.Errorf("unexpected error stack trace: %q", e.StackTrace)
+	}
+}

v8go.cc

@@ -26,32 +26,57 @@ typedef struct {
 
 const char* CString(String::Utf8Value& value) {
   if (value.length() == 0) {
-    return "empty exception";
+    return "empty";
   }
   return *value;
 }
 
-const char* ExceptionString(TryCatch& try_catch, Isolate* iso, Local<Context> ctx) {
+const char* CopyString(std::string str) {
+  char* data = static_cast<char*>(malloc(str.length()));
+  sprintf(data, "%s", str.c_str());
+  return data;
+}
+
+const char* CopyString(String::Utf8Value& value) {
+  if (value.length() == 0) {
+    return "";
+  }
+  return CopyString(*value);
+}
+
+RtnError ExceptionError(TryCatch& try_catch, Isolate* iso, Local<Context> ctx) {
   Locker locker(iso);
   Isolate::Scope isolate_scope(iso);
   HandleScope handle_scope(iso);
 
-  std::stringstream sb;
+  RtnError rtn = {nullptr, nullptr, nullptr};
+
   String::Utf8Value exception(iso, try_catch.Exception());
-  sb << CString(exception);
+  rtn.msg = CopyString(exception);
 
   Local<Message> msg = try_catch.Message();
   if (!msg.IsEmpty()) {
     String::Utf8Value origin(iso, msg->GetScriptOrigin().ResourceName());
-    sb << std::endl
-       << " at " << CString(origin);
+    std::ostringstream sb;
+    sb << *origin;
+    Maybe<int> line = try_catch.Message()->GetLineNumber(ctx);
+    if (line.IsJust()) {
+      sb << ":" << line.ToChecked();
+    }
+    Maybe<int> start = try_catch.Message()->GetStartColumn(ctx);
+    if (start.IsJust()) {
+      sb << ":" << start.ToChecked() + 1; // + 1 to match output from stack trace
+    }
+    rtn.location = CopyString(sb.str());
   }
-
-
-  std::string str = sb.str();
-  char* data = static_cast<char*>(malloc(str.length()));
-  sprintf(data, "%s", str.c_str());
-  return data;
+
+  MaybeLocal<Value> mstack = try_catch.StackTrace(ctx);
+  if (!mstack.IsEmpty()) {
+    String::Utf8Value stack(iso, mstack.ToLocalChecked());
+    rtn.stack = CopyString(stack);
+  }
+
+  return rtn;
 }
 
 extern "C"
@@ -120,13 +145,13 @@ RtnValue RunScript(ContextPtr ctx_ptr, const char* source, const char* origin) {
     MaybeLocal<Script> script = Script::Compile(local_ctx, src, &script_origin);
     if (script.IsEmpty()) {
       assert(try_catch.HasCaught());
-      rtn.error = ExceptionString(try_catch, iso, local_ctx);
+      rtn.error = ExceptionError(try_catch, iso, local_ctx);
       return rtn;
     } 
     MaybeLocal<v8::Value> result = script.ToLocalChecked()->Run(local_ctx);
     if (result.IsEmpty()) {
       assert(try_catch.HasCaught());
-      rtn.error = "some error in result";
+      rtn.error = ExceptionError(try_catch, iso, local_ctx);
       return rtn;
     }
     m_value* val = new m_value;

v8go.h

@@ -8,9 +8,15 @@ typedef void* IsolatePtr;
 typedef void* ContextPtr;
 typedef void* ValuePtr;
 
+typedef struct {
+  const char* msg;
+  const char* location;
+  const char* stack;
+} RtnError;
+
 typedef struct {
   ValuePtr value;
-  const char* error;
+  RtnError error;
 } RtnValue;
 
 extern void Init();