Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support FunctionCallback returning an error, and native Exceptions. #195

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
109 changes: 109 additions & 0 deletions exception.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
// Copyright 2021 Roger Chapman and the v8go contributors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

package v8go

import (
// #include <stdlib.h>
// #include "v8go.h"
"C"

"fmt"
"unsafe"
)

// NewRangeError creates a RangeError.
func NewRangeError(iso *Isolate, msg string) *Exception {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is already the JSError type which is used to return exceptions from v8go functions. It seems like there should be consistency between the exception value returned by other v8go functions and these functions to construct errors. That should make it easier to propagate exceptions, even without adding a new feature to return errors from function callbacks.

Currently, JSError is converting the underlying JS error object to a string and storing it in the Message field. Instead, it should store the underlying JS error object, so that the error can be propagated. That Message field could be deprecated in favour of a method so that the conversion can be done on-demand in the future. It looks like the stack trace can similarly be obtained on-demand using v8::Exception::GetStackTrace.

The JSError.Location field is derived from the a v8::Message on v8::TryCatch, which may be present without a stack trace for compilation errors. v8::Exception::CreateMessage can be used to obtained a v8::Message from an exception value, but we don't want to automatically create it in a getter method. Since it is a public field, it will need to continue populating on creation for the current code paths, but it could similarly be deprecated in favour of a method to obtain this on-demand for new code paths. This on-demand code path could at least obtain the when the error has a stack trace, even if we don't get it to initially work on this new code path errors with a location but no stack trace.

It seems like this refactoring and the addition of these functions to create exceptions could be split out into a separate PR.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've opened this as an issue (#274) for greater visibility on this issue with JSError

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't remember exactly why I created a new type for it, but it was probably to avoid having to refactor JSError (avoiding merge conflicts). If there's consensus that's better, that sounds good to me. Thanks for reviewing.

return newExceptionError(iso, C.ERROR_RANGE, msg)
}

// NewReferenceError creates a ReferenceError.
func NewReferenceError(iso *Isolate, msg string) *Exception {
return newExceptionError(iso, C.ERROR_REFERENCE, msg)
}

// NewSyntaxError creates a SyntaxError.
func NewSyntaxError(iso *Isolate, msg string) *Exception {
return newExceptionError(iso, C.ERROR_SYNTAX, msg)
}

// NewTypeError creates a TypeError.
func NewTypeError(iso *Isolate, msg string) *Exception {
return newExceptionError(iso, C.ERROR_TYPE, msg)
}

// NewWasmCompileError creates a WasmCompileError.
func NewWasmCompileError(iso *Isolate, msg string) *Exception {
return newExceptionError(iso, C.ERROR_WASM_COMPILE, msg)
}

// NewWasmLinkError creates a WasmLinkError.
func NewWasmLinkError(iso *Isolate, msg string) *Exception {
return newExceptionError(iso, C.ERROR_WASM_LINK, msg)
}

// NewWasmRuntimeError creates a WasmRuntimeError.
func NewWasmRuntimeError(iso *Isolate, msg string) *Exception {
return newExceptionError(iso, C.ERROR_WASM_RUNTIME, msg)
}

// NewError creates an Error, which is the common thing to throw from
// user code.
func NewError(iso *Isolate, msg string) *Exception {
return newExceptionError(iso, C.ERROR_GENERIC, msg)
}

func newExceptionError(iso *Isolate, typ C.ErrorTypeIndex, msg string) *Exception {
cmsg := C.CString(msg)
defer C.free(unsafe.Pointer(cmsg))
eptr := C.NewValueError(iso.ptr, typ, cmsg)
if eptr == nil {
panic(fmt.Errorf("invalid error type index: %d", typ))
}
return &Exception{&Value{ptr: eptr}}
}

// An Exception is a JavaScript exception.
type Exception struct {
*Value
}

// value implements Valuer.
func (e *Exception) value() *Value {
return e.Value
}

// Error implements error.
func (e *Exception) Error() string {
return e.String()
}

// As provides support for errors.As.
func (e *Exception) As(target interface{}) bool {
ep, ok := target.(**Exception)
if !ok {
return false
}
*ep = e
return true
}

// Is provides support for errors.Is.
func (e *Exception) Is(err error) bool {
eerr, ok := err.(*Exception)
if !ok {
return false
}
return eerr.String() == e.String()
}

// String implements fmt.Stringer.
func (e *Exception) String() string {
if e.Value == nil {
return "<nil>"
}
s := C.ExceptionGetMessageString(e.ptr)
defer C.free(unsafe.Pointer(s))
return C.GoString(s)
}
79 changes: 79 additions & 0 deletions exception_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
// Copyright 2021 Roger Chapman and the v8go contributors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

package v8go_test

import (
"errors"
"strings"
"testing"

v8 "rogchap.com/v8go"
)

func TestNewError(t *testing.T) {
t.Parallel()

tsts := []struct {
New func(*v8.Isolate, string) *v8.Exception
WantType string
}{
{v8.NewRangeError, "RangeError"},
{v8.NewReferenceError, "ReferenceError"},
{v8.NewSyntaxError, "SyntaxError"},
{v8.NewTypeError, "TypeError"},
{v8.NewWasmCompileError, "CompileError"},
{v8.NewWasmLinkError, "LinkError"},
{v8.NewWasmRuntimeError, "RuntimeError"},
{v8.NewError, "Error"},
}
for _, tst := range tsts {
t.Run(tst.WantType, func(t *testing.T) {
iso := v8.NewIsolate()
defer iso.Dispose()

got := tst.New(iso, "amessage")
if !got.IsNativeError() {
t.Error("IsNativeError returned false, want true")
}
if got := got.Error(); !strings.Contains(got, " "+tst.WantType+":") {
t.Errorf("Error(): got %q, want containing %q", got, tst.WantType)
}
})
}
}

func TestExceptionAs(t *testing.T) {
iso := v8.NewIsolate()
defer iso.Dispose()

want := v8.NewRangeError(iso, "faked error")

var got *v8.Exception
if !want.As(&got) {
t.Fatalf("As failed")
}

if got != want {
t.Errorf("As: got %#v, want %#v", got, want)
}
}

func TestExceptionIs(t *testing.T) {
iso := v8.NewIsolate()
defer iso.Dispose()

t.Run("ok", func(t *testing.T) {
ex := v8.NewRangeError(iso, "faked error")
if !ex.Is(v8.NewRangeError(iso, "faked error")) {
t.Fatalf("Is: got false, want true")
}
})

t.Run("notok", func(t *testing.T) {
if (&v8.Exception{}).Is(errors.New("other error")) {
t.Fatalf("Is: got true, want false")
}
})
}
4 changes: 2 additions & 2 deletions export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@
package v8go

// RegisterCallback is exported for testing only.
func (i *Isolate) RegisterCallback(cb FunctionCallback) int {
func (i *Isolate) RegisterCallback(cb FunctionCallbackWithError) int {
return i.registerCallback(cb)
}

// GetCallback is exported for testing only.
func (i *Isolate) GetCallback(ref int) FunctionCallback {
func (i *Isolate) GetCallback(ref int) FunctionCallbackWithError {
return i.getCallback(ref)
}

Expand Down
47 changes: 42 additions & 5 deletions function_template.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,26 @@ import (
// FunctionCallback is a callback that is executed in Go when a function is executed in JS.
type FunctionCallback func(info *FunctionCallbackInfo) *Value

// FunctionCallbackWithError is a callback that is executed in Go when
// a function is executed in JS. If a ValueError is returned, its
// value will be thrown as an exception in V8, otherwise Error() is
// invoked, and the string is thrown.
type FunctionCallbackWithError func(info *FunctionCallbackInfo) (*Value, error)

// FunctionCallbackInfo is the argument that is passed to a FunctionCallback.
type FunctionCallbackInfo struct {
ctx *Context
args []*Value
this *Object
}

// A ValueError can be returned from a FunctionCallbackWithError, and
// its value will be thrown as an exception in V8.
type ValueError interface {
error
Valuer
}

// Context is the current context that the callback is being executed in.
func (i *FunctionCallbackInfo) Context() *Context {
return i.ctx
Expand All @@ -44,8 +57,21 @@ type FunctionTemplate struct {
*template
}

// NewFunctionTemplate creates a FunctionTemplate for a given callback.
// NewFunctionTemplate creates a FunctionTemplate for a given
// callback. Prefer using NewFunctionTemplateWithError.
func NewFunctionTemplate(iso *Isolate, callback FunctionCallback) *FunctionTemplate {
if callback == nil {
panic("nil FunctionCallback argument not supported")
}
return NewFunctionTemplateWithError(iso, func(info *FunctionCallbackInfo) (*Value, error) {
return callback(info), nil
})
}

// NewFunctionTemplateWithError creates a FunctionTemplate for a given
// callback. If the callback returns an error, it will be thrown as a
// JS error.
func NewFunctionTemplateWithError(iso *Isolate, callback FunctionCallbackWithError) *FunctionTemplate {
if iso == nil {
panic("nil Isolate argument not supported")
}
Expand Down Expand Up @@ -77,7 +103,7 @@ func (tmpl *FunctionTemplate) GetFunction(ctx *Context) *Function {
// Note that ideally `thisAndArgs` would be split into two separate arguments, but they were combined
// to workaround an ERROR_COMMITMENT_LIMIT error on windows that was detected in CI.
//export goFunctionCallback
func goFunctionCallback(ctxref int, cbref int, thisAndArgs *C.ValuePtr, argsCount int) C.ValuePtr {
func goFunctionCallback(ctxref int, cbref int, thisAndArgs *C.ValuePtr, argsCount int) (rval C.ValuePtr, rerr C.ValuePtr) {
ctx := getContext(ctxref)

this := *thisAndArgs
Expand All @@ -94,8 +120,19 @@ func goFunctionCallback(ctxref int, cbref int, thisAndArgs *C.ValuePtr, argsCoun
}

callbackFunc := ctx.iso.getCallback(cbref)
if val := callbackFunc(info); val != nil {
return val.ptr
val, err := callbackFunc(info)
if err != nil {
if verr, ok := err.(ValueError); ok {
return nil, verr.value().ptr
}
errv, err := NewValue(ctx.iso, err.Error())
if err != nil {
panic(err)
}
return nil, errv.ptr
}
if val == nil {
return nil, nil
}
return nil
return val.ptr, nil
}
100 changes: 76 additions & 24 deletions function_template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,32 +55,84 @@ func TestFunctionTemplate_panic_on_nil_callback(t *testing.T) {
func TestFunctionTemplateGetFunction(t *testing.T) {
t.Parallel()

iso := v8.NewIsolate()
defer iso.Dispose()
ctx := v8.NewContext(iso)
defer ctx.Close()
t.Run("can_call", func(t *testing.T) {
t.Parallel()

iso := v8.NewIsolate()
defer iso.Dispose()
ctx := v8.NewContext(iso)
defer ctx.Close()

var args *v8.FunctionCallbackInfo
tmpl := v8.NewFunctionTemplate(iso, func(info *v8.FunctionCallbackInfo) *v8.Value {
args = info
reply, _ := v8.NewValue(iso, "hello")
return reply
})
fn := tmpl.GetFunction(ctx)
ten, err := v8.NewValue(iso, int32(10))
if err != nil {
t.Fatal(err)
}
ret, err := fn.Call(v8.Undefined(iso), ten)
if err != nil {
t.Fatal(err)
}
if len(args.Args()) != 1 || args.Args()[0].String() != "10" {
t.Fatalf("expected args [10], got: %+v", args.Args())
}
if !ret.IsString() || ret.String() != "hello" {
t.Fatalf("expected return value of 'hello', was: %v", ret)
}
})

t.Run("can_throw_string", func(t *testing.T) {
t.Parallel()

iso := v8.NewIsolate()
defer iso.Dispose()

tmpl := v8.NewFunctionTemplateWithError(iso, func(info *v8.FunctionCallbackInfo) (*v8.Value, error) {
return nil, fmt.Errorf("fake error")
})
global := v8.NewObjectTemplate(iso)
global.Set("foo", tmpl)

ctx := v8.NewContext(iso, global)
defer ctx.Close()

ret, err := ctx.RunScript("(() => { try { foo(); return null; } catch (e) { return e; } })()", "")
if err != nil {
t.Fatal(err)
}
if !ret.IsString() || ret.String() != "fake error" {
t.Fatalf("expected return value of 'hello', was: %v", ret)
}
})

t.Run("can_throw_exception", func(t *testing.T) {
t.Parallel()

iso := v8.NewIsolate()
defer iso.Dispose()

tmpl := v8.NewFunctionTemplateWithError(iso, func(info *v8.FunctionCallbackInfo) (*v8.Value, error) {
return nil, v8.NewError(iso, "fake error")
})
global := v8.NewObjectTemplate(iso)
global.Set("foo", tmpl)

ctx := v8.NewContext(iso, global)
defer ctx.Close()

var args *v8.FunctionCallbackInfo
tmpl := v8.NewFunctionTemplate(iso, func(info *v8.FunctionCallbackInfo) *v8.Value {
args = info
reply, _ := v8.NewValue(iso, "hello")
return reply
ret, err := ctx.RunScript("(() => { try { foo(); return null; } catch (e) { return e; } })()", "")
if err != nil {
t.Fatal(err)
}
if !ret.IsNativeError() || !strings.Contains(ret.String(), "fake error") {
t.Fatalf("expected return value of Error('hello'), was: %v", ret)
}
})
fn := tmpl.GetFunction(ctx)
ten, err := v8.NewValue(iso, int32(10))
if err != nil {
t.Fatal(err)
}
ret, err := fn.Call(v8.Undefined(iso), ten)
if err != nil {
t.Fatal(err)
}
if len(args.Args()) != 1 || args.Args()[0].String() != "10" {
t.Fatalf("expected args [10], got: %+v", args.Args())
}
if !ret.IsString() || ret.String() != "hello" {
t.Fatalf("expected return value of 'hello', was: %v", ret)
}
}

func TestFunctionCallbackInfoThis(t *testing.T) {
Expand Down
Loading