Skip to content

Commit

Permalink
Addressing review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
snej committed Jun 20, 2023
1 parent 30840f2 commit fa73ec2
Show file tree
Hide file tree
Showing 8 changed files with 16 additions and 22 deletions.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ module github.com/couchbase/sg-bucket
go 1.19

require (
github.com/pkg/errors v0.9.1
github.com/robertkrimen/otto v0.0.0-20211024170158-b87d35c0b86f
github.com/snej/v8go v1.7.3
github.com/stretchr/testify v1.7.1
Expand All @@ -16,6 +15,7 @@ require (
github.com/golang/snappy v0.0.4 // indirect
github.com/google/uuid v1.3.0 // indirect
github.com/opentracing/opentracing-go v1.2.0 // indirect
github.com/pkg/errors v0.9.1 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
golang.org/x/net v0.0.0-20220425223048-2871e0cb64e4 // indirect
golang.org/x/sync v0.0.0-20210220032951-036812b2e83c // indirect
Expand Down
2 changes: 1 addition & 1 deletion js/otto_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ package js
import (
"context"
"encoding/json"
"errors"
"fmt"
"strconv"

"github.com/pkg/errors"
"github.com/robertkrimen/otto"

_ "github.com/robertkrimen/otto/underscore"
Expand Down
4 changes: 2 additions & 2 deletions js/otto_vm.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,10 @@ func (vm *ottoVM) getRunner(service *Service) (Runner, error) {
return nil, fmt.Errorf("the js.VM has been closed")
}
if vm.curRunner != nil {
panic("illegal access to v8VM: already has a v8Runner")
panic("illegal access to ottoVM: already has a ottoRunner")
}
if !vm.services.hasService(service) {
return nil, fmt.Errorf("unknown js.Service instance passed to VM")
return nil, fmt.Errorf("unknown js.Service instance passed to VM: %v", service)
}
if service.v8Init != nil {
return nil, fmt.Errorf("js.Service has custom initialization not supported by Otto")
Expand Down
13 changes: 8 additions & 5 deletions js/v8_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@ package js
import (
"context"
"encoding/json"
"errors"
"fmt"
"strings"
"time"

"github.com/pkg/errors"
v8 "github.com/snej/v8go" // Docs: https://pkg.go.dev/github.com/snej/v8go
)

Expand All @@ -32,22 +32,25 @@ type V8Runner struct {
Client any // You can put whatever you want here, to point back to your state
}

func newV8Runner(vm *v8VM, template V8Template, id serviceID) (*V8Runner, error) {
func newV8Runner(vm *v8VM, template V8Template, id serviceID) (runner *V8Runner, err error) {
// Create a V8 Context and run the setup script in it:
ctx := v8.NewContext(vm.iso, template.Global())
defer func() {
if err != nil {
ctx.Close()
}
}()
if _, err := vm.setupScript.Run(ctx); err != nil {
return nil, errors.Wrap(err, "Unexpected error in JavaScript initialization code")
return nil, fmt.Errorf("Unexpected error in JavaScript initialization code: %w", err)
}

// Now run the service's script, which returns the service's main function:
result, err := template.Script().Run(ctx)
if err != nil {
ctx.Close()
return nil, fmt.Errorf("JavaScript error initializing %s: %w", template.Name(), err)
}
mainFn, err := result.AsFunction()
if err != nil {
ctx.Close()
return nil, fmt.Errorf("%s's script did not return a function: %w", template.Name(), err)
}

Expand Down
3 changes: 1 addition & 2 deletions js/v8_template.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ package js
import (
"fmt"

"github.com/pkg/errors"
v8 "github.com/snej/v8go"
)

Expand Down Expand Up @@ -114,7 +113,7 @@ func (t *V8BasicTemplate) NewCallback(callback TemplateCallback) *v8.FunctionTem
if v8Result, newValErr := runner.NewValue(result); err == nil {
return v8Result
} else {
err = errors.Wrap(newValErr, "Could not convert a callback's result to JavaScript")
err = fmt.Errorf("Could not convert a callback's result to JavaScript: %w", newValErr)
}
}
return v8Throw(vm.iso, err)
Expand Down
8 changes: 1 addition & 7 deletions js/v8_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,13 +114,7 @@ func newJSONString(ctx *v8.Context, val any) (*v8.Value, error) {
// Returns an error back to a V8 caller.
// Calls v8.Isolate.ThrowException, with the Go error's string as the message.
func v8Throw(i *v8.Isolate, err error) *v8.Value {
var errStr string
// if httpErr, ok := err.(*base.HTTPError); ok {
// errStr = fmt.Sprintf("[%d] %s", httpErr.Status, httpErr.Message)
// } else {
errStr = err.Error()
// }
return i.ThrowException(newString(i, errStr))
return i.ThrowException(newString(i, err.Error()))
}

// Simple utility to wrap a function that returns a value and an error; returns just the value, panicking if there was an error.
Expand Down
5 changes: 2 additions & 3 deletions js/v8_vm.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"sync"
"time"

"github.com/pkg/errors"
v8 "github.com/snej/v8go"
)

Expand Down Expand Up @@ -123,7 +122,7 @@ func (vm *v8VM) release() {
func (vm *v8VM) getTemplate(service *Service) (V8Template, error) {
var tmpl V8Template
if !vm.services.hasService(service) {
return nil, fmt.Errorf("unknown js.Service instance passed to VM")
return nil, fmt.Errorf("unknown js.Service instance passed to VM: %v", service)
}
if int(service.id) < len(vm.templates) {
tmpl = vm.templates[service.id]
Expand All @@ -134,7 +133,7 @@ func (vm *v8VM) getTemplate(service *Service) (V8Template, error) {
var err error
vm.setupScript, err = vm.iso.CompileUnboundScript(kSetupLoggingJS+kUnderscoreJS, "setupScript.js", v8.CompileOptions{})
if err != nil {
return nil, errors.Wrapf(err, "Couldn't compile setup script")
return nil, fmt.Errorf("Couldn't compile setup script: %w", err)
}
}

Expand Down
1 change: 0 additions & 1 deletion js/vmpool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ func TestPoolsSequentially(t *testing.T) {

func TestPoolsConcurrently(t *testing.T) {
maxProcs := runtime.GOMAXPROCS(0)
log.Printf("FYI, GOMAXPROCS = %d", maxProcs)
if !assert.GreaterOrEqual(t, maxProcs, 2, "Not enough OS threads available") {
return
}
Expand Down

0 comments on commit fa73ec2

Please sign in to comment.