Unverified Commit dd9d45b3 authored by Simon Esposito's avatar Simon Esposito Committed by GitHub
Browse files

Improve JS runtime error handling (#648) 

Handle thrown JS Custom errors if thrown inside an API call and issue a response mapped to that code.
Add optional userID argument to Lua/JS runtime groupUpdate functions to allow permission checks on the caller.
Other JS runtime bug fixes, resolve #647, #646 and #644.
parent d94a7081

CHANGELOG.md

+8 −0
Original line number Diff line number Diff line
@@ -4,10 +4,18 @@ All notable changes to this project are documented below. 
The format is based on [keep a changelog](http://keepachangelog.com) and this project uses [semantic versioning](http://semver.org).



## [Unreleased]

### Added

- Handle thrown JS runtime custom exceptions containing a message and a grpc code to be returned in the server response.



### Changed

- Size limit for status messages increased from 128 to 2048 characters.

- Improve unfiltered group listings responses.

- Improve error when attempting to create a group with the system user.

- Add userId field for permission validation in JS/Lua runtimes group update functions.



### Fixed

- Fix creator id being read from the wrong argument in JS runtime group update function.

- Fix max count being incorrectly validated in group update JS runtime function.



## [3.4.0] - 2021-07-08

### Added

server/runtime_javascript.go

+62 −35
Original line number Diff line number Diff line
@@ -72,26 +72,10 @@ func (r *RuntimeJS) GetCallback(e RuntimeExecutionMode, key string) string { 
	return ""

}



type JsErrorType int



func (e JsErrorType) String() string {

	switch e {

	case JsErrorException:

		return "exception"

	default:

		return ""

	}

}



const (

	JsErrorException JsErrorType = iota

	JsErrorRuntime

)



type jsError struct {

	StackTrace string

	Type       string

	Message    string `json:",omitempty"`

	StackTrace string `json:"stackTrace,omitempty"`

	Type       string `json:"type,omitempty"`

	custom     bool

	error      error

}
@@ -99,18 +83,21 @@ func (e *jsError) Error() string { 
	return e.error.Error()

}



func newJsExceptionError(t JsErrorType, error, st string) *jsError {

	return &jsError{

		StackTrace: st,

		Type:       t.String(),

		error:      errors.New(error),

func newJsUncaughtExceptionError(error error, st string, custom bool) *jsError {

	jsErr := &jsError{

		Type:   "uncaughtExceptionError",

		error:  error,

		custom: custom,

	}

	if !custom {

		jsErr.StackTrace = st

	}

	return jsErr

}



func newJsError(t JsErrorType, err error) *jsError {

func newJsRuntimeError(err error) *jsError {

	return &jsError{

		Message: err.Error(),

		Type:    t.String(),

		Type:  "runtimeError",

		error: err,

	}

}
@@ -240,7 +227,11 @@ func (rp *RuntimeProviderJS) BeforeRt(ctx context.Context, id string, logger *za 
	rp.Put(r)



	if fnErr != nil {

		if jsErr, ok := fnErr.(*jsError); ok {

			if !jsErr.custom {

				logger.Error("Runtime Before function caused an error.", zap.String("id", id), zap.Error(fnErr))

			}

		}

		return nil, fnErr

	}
@@ -301,7 +292,11 @@ func (rp *RuntimeProviderJS) AfterRt(ctx context.Context, id string, logger *zap 
	rp.Put(r)



	if fnErr != nil {

		if jsErr, ok := fnErr.(*jsError); ok {

			if !jsErr.custom {

				logger.Error("Runtime After function caused an error.", zap.String("id", id), zap.Error(fnErr))

			}

		}

		return fnErr

	}
@@ -358,7 +353,11 @@ func (rp *RuntimeProviderJS) BeforeReq(ctx context.Context, id string, logger *z 
	rp.Put(r)



	if fnErr != nil {

		if jsErr, ok := fnErr.(*jsError); ok {

			if !jsErr.custom {

				logger.Error("Runtime Before function caused an error.", zap.String("id", id), zap.Error(err))

			}

		}

		return nil, fnErr, code

	}
@@ -453,7 +452,11 @@ func (rp *RuntimeProviderJS) AfterReq(ctx context.Context, id string, logger *za 
	rp.Put(r)



	if fnErr != nil {

		if jsErr, ok := fnErr.(*jsError); ok {

			if !jsErr.custom {

				logger.Error("JavaScript runtime After function caused an error.", zap.String("id", id), zap.Error(fnErr))

			}

		}

		return fnErr

	}
@@ -487,11 +490,32 @@ func (r *RuntimeJS) invokeFunction(execMode RuntimeExecutionMode, id string, fn 
	retVal, err := fn(goja.Null(), args...)

	if err != nil {

		if exErr, ok := err.(*goja.Exception); ok {

			errMsg := exErr.Error()

			errCode := codes.Internal

			custom := false

			if errMap, ok := exErr.Value().Export().(map[string]interface{}); ok {

				// Custom exception with message and code

				if msg, ok := errMap["message"]; ok {

					if msgStr, ok := msg.(string); ok {

						errMsg = msgStr

						custom = true

					}

				}

				if code, ok := errMap["code"]; ok {

					if codeInt, ok := code.(int64); ok {

						errCode = codes.Code(codeInt)

						custom = true

					}

				}

			}



			if !custom {

				r.logger.Error("JavaScript runtime function raised an uncaught exception", zap.String("mode", execMode.String()), zap.String("id", id), zap.Error(err))

			return nil, newJsExceptionError(JsErrorException, exErr.Error(), exErr.String()), codes.Internal

			}

			return nil, newJsUncaughtExceptionError(errors.New(errMsg), exErr.String(), custom), errCode

		}

		r.logger.Error("JavaScript runtime function caused an error", zap.String("mode", execMode.String()), zap.String("id", id), zap.Error(err))

		return nil, newJsError(JsErrorRuntime, err), codes.Internal

		return nil, newJsRuntimeError(err), codes.Internal

	}

	if retVal == nil || retVal == goja.Undefined() || retVal == goja.Null() {

		return nil, nil, codes.OK
@@ -1613,6 +1637,9 @@ func (rp *RuntimeProviderJS) MatchmakerMatched(ctx context.Context, entries []*M 


	retValue, err, _ := r.InvokeFunction(RuntimeExecutionModeMatchmaker, "matchmakerMatched", fn, jsLogger, nil, "", "", nil, 0, "", "", "", "", r.vm.ToValue(entriesSlice))

	rp.Put(r)

	if err != nil {

		return "", false, fmt.Errorf("Error running runtime Matchmaker Matched hook: %v", err.Error())

	}



	if retValue == nil {

		// No return value or hook decided not to return an authoritative match ID.

server/runtime_javascript_context.go

+1 −1
Original line number Diff line number Diff line
@@ -43,7 +43,7 @@ func NewRuntimeJsContext(r *goja.Runtime, node string, env goja.Value, mode Runt 
	ctxObj := r.NewObject()

	ctxObj.Set(__RUNTIME_JAVASCRIPT_CTX_NODE, node)

	ctxObj.Set(__RUNTIME_JAVASCRIPT_CTX_ENV, env)

	ctxObj.Set(__RUNTIME_JAVASCRIPT_CTX_MODE, mode)

	ctxObj.Set(__RUNTIME_JAVASCRIPT_CTX_MODE, mode.String())

	if queryParams != nil {

		ctxObj.Set(__RUNTIME_JAVASCRIPT_CTX_QUERY_PARAMS, queryParams)

	}

server/runtime_javascript_nakama.go

+28 −19
Original line number Diff line number Diff line
@@ -5172,8 +5172,8 @@ func (n *runtimeJavascriptNakamaModule) groupCreate(r *goja.Runtime) func(goja.F 
		}



		creatorIDString := uuid.Nil.String()

		if f.Argument(3) != goja.Undefined() && f.Argument(3) != goja.Null() {

			creatorIDString = getJsString(r, f.Argument(3))

		if f.Argument(2) != goja.Undefined() && f.Argument(2) != goja.Null() {

			creatorIDString = getJsString(r, f.Argument(2))

		}

		creatorID, err := uuid.FromString(creatorIDString)

		if err != nil {
@@ -5259,15 +5259,24 @@ func (n *runtimeJavascriptNakamaModule) groupUpdate(r *goja.Runtime) func(goja.F 
			panic(r.NewTypeError("expects group ID to be a valid identifier"))

		}



		var name *wrapperspb.StringValue

		userId := uuid.Nil

		if f.Argument(1) != goja.Undefined() && f.Argument(1) != goja.Null() {

			nameStr := getJsString(r, f.Argument(1))

			userIdStr := getJsString(r, f.Argument(1))

			userId, err = uuid.FromString(userIdStr)

			if err != nil {

				panic(r.NewTypeError("expects user ID to be a valid identifier"))

			}

		}



		var name *wrapperspb.StringValue

		if f.Argument(2) != goja.Undefined() && f.Argument(2) != goja.Null() {

			nameStr := getJsString(r, f.Argument(2))

			name = &wrapperspb.StringValue{Value: nameStr}

		}



		creatorID := uuid.Nil

		if f.Argument(2) != goja.Undefined() && f.Argument(2) != goja.Null() {

			creatorIDStr := getJsString(r, f.Argument(2))

		if f.Argument(3) != goja.Undefined() && f.Argument(3) != goja.Null() {

			creatorIDStr := getJsString(r, f.Argument(3))

			creatorID, err = uuid.FromString(creatorIDStr)

			if err != nil {

				panic(r.NewTypeError("expects creator ID to be a valid identifier"))
@@ -5275,30 +5284,30 @@ func (n *runtimeJavascriptNakamaModule) groupUpdate(r *goja.Runtime) func(goja.F 
		}



		var lang *wrapperspb.StringValue

		if f.Argument(3) != goja.Undefined() && f.Argument(3) != goja.Null() {

			langStr := getJsString(r, f.Argument(3))

		if f.Argument(4) != goja.Undefined() && f.Argument(4) != goja.Null() {

			langStr := getJsString(r, f.Argument(4))

			lang = &wrapperspb.StringValue{Value: langStr}

		}



		var desc *wrapperspb.StringValue

		if f.Argument(4) != goja.Undefined() && f.Argument(4) != goja.Null() {

			descStr := getJsString(r, f.Argument(4))

		if f.Argument(5) != goja.Undefined() && f.Argument(5) != goja.Null() {

			descStr := getJsString(r, f.Argument(5))

			desc = &wrapperspb.StringValue{Value: descStr}

		}



		var avatarURL *wrapperspb.StringValue

		if f.Argument(5) != goja.Undefined() && f.Argument(5) != goja.Null() {

			avatarStr := getJsString(r, f.Argument(5))

		if f.Argument(6) != goja.Undefined() && f.Argument(6) != goja.Null() {

			avatarStr := getJsString(r, f.Argument(6))

			avatarURL = &wrapperspb.StringValue{Value: avatarStr}

		}



		var open *wrapperspb.BoolValue

		if f.Argument(6) != goja.Undefined() && f.Argument(6) != goja.Null() {

			open = &wrapperspb.BoolValue{Value: getJsBool(r, f.Argument(6))}

		if f.Argument(7) != goja.Undefined() && f.Argument(7) != goja.Null() {

			open = &wrapperspb.BoolValue{Value: getJsBool(r, f.Argument(7))}

		}



		var metadata *wrapperspb.StringValue

		metadataIn := f.Argument(7)

		metadataIn := f.Argument(8)

		if metadataIn != goja.Undefined() && metadataIn != goja.Null() {

			metadataMap, ok := metadataIn.Export().(map[string]interface{})

			if !ok {
@@ -5312,16 +5321,16 @@ func (n *runtimeJavascriptNakamaModule) groupUpdate(r *goja.Runtime) func(goja.F 
		}



		maxCount := 0

		if f.Argument(8) != goja.Undefined() && f.Argument(8) != goja.Null() {

			maxCountIn := int(getJsInt(r, f.Argument(8)))

			if maxCount > 0 && maxCount <= 100 {

		if f.Argument(9) != goja.Undefined() && f.Argument(9) != goja.Null() {

			maxCountIn := int(getJsInt(r, f.Argument(9)))

			if maxCountIn > 0 && maxCountIn <= 100 {

				maxCount = maxCountIn

			} else {

				panic(r.NewTypeError("max count must be 1-100"))

			}

		}



		if err = UpdateGroup(context.Background(), n.logger, n.db, groupID, uuid.Nil, creatorID, name, lang, desc, avatarURL, metadata, open, maxCount); err != nil {

		if err = UpdateGroup(context.Background(), n.logger, n.db, groupID, userId, creatorID, name, lang, desc, avatarURL, metadata, open, maxCount); err != nil {

			panic(r.NewGoError(fmt.Errorf("error while trying to update group: %v", err.Error())))

		}

server/runtime_lua_nakama.go

+15 −9
Original line number Diff line number Diff line
@@ -6764,13 +6764,19 @@ func (n *RuntimeLuaNakamaModule) groupUpdate(l *lua.LState) int { 
		return 0

	}



	nameStr := l.OptString(2, "")

	userID, err := uuid.FromString(l.OptString(2, uuid.Nil.String()))

	if err != nil {

		l.ArgError(1, "expects user ID to be a valid identifier")

		return 0

	}



	nameStr := l.OptString(3, "")

	var name *wrapperspb.StringValue

	if nameStr != "" {

		name = &wrapperspb.StringValue{Value: nameStr}

	}



	creatorIDStr := l.OptString(3, "")

	creatorIDStr := l.OptString(4, "")

	creatorID := uuid.Nil

	if creatorIDStr != "" {

		var err error
@@ -6781,31 +6787,31 @@ func (n *RuntimeLuaNakamaModule) groupUpdate(l *lua.LState) int { 
		}

	}



	langStr := l.OptString(4, "")

	langStr := l.OptString(5, "")

	var lang *wrapperspb.StringValue

	if langStr != "" {

		lang = &wrapperspb.StringValue{Value: langStr}

	}



	descStr := l.OptString(5, "")

	descStr := l.OptString(6, "")

	var desc *wrapperspb.StringValue

	if descStr != "" {

		desc = &wrapperspb.StringValue{Value: descStr}

	}



	avatarURLStr := l.OptString(6, "")

	avatarURLStr := l.OptString(7, "")

	var avatarURL *wrapperspb.StringValue

	if avatarURLStr != "" {

		avatarURL = &wrapperspb.StringValue{Value: avatarURLStr}

	}



	openV := l.Get(7)

	openV := l.Get(8)

	var open *wrapperspb.BoolValue

	if openV != lua.LNil {

		open = &wrapperspb.BoolValue{Value: l.OptBool(7, false)}

	}



	metadataTable := l.OptTable(8, nil)

	metadataTable := l.OptTable(9, nil)

	var metadata *wrapperspb.StringValue

	if metadataTable != nil {

		metadataMap := RuntimeLuaConvertLuaTable(metadataTable)
@@ -6817,13 +6823,13 @@ func (n *RuntimeLuaNakamaModule) groupUpdate(l *lua.LState) int { 
		metadata = &wrapperspb.StringValue{Value: string(metadataBytes)}

	}



	maxCountInt := l.OptInt(9, 0)

	maxCountInt := l.OptInt(10, 0)

	maxCount := 0

	if maxCountInt > 0 && maxCountInt <= 100 {

		maxCount = maxCountInt

	}



	if err = UpdateGroup(l.Context(), n.logger, n.db, groupID, uuid.Nil, creatorID, name, lang, desc, avatarURL, metadata, open, maxCount); err != nil {

	if err = UpdateGroup(l.Context(), n.logger, n.db, groupID, userID, creatorID, name, lang, desc, avatarURL, metadata, open, maxCount); err != nil {

		l.RaiseError("error while trying to update group: %v", err.Error())

		return 0

	}