added log warning for async marked JSVM handlers and resolve when possible the returned Promise as fallback

CHANGELOG.md

@@ -11,10 +11,13 @@
 
 - Added `$os.stat(file)` JSVM helper ([#6407](https://github.com/pocketbase/pocketbase/discussions/6407)).
 
+- Added log warning for `async` marked JSVM handlers and resolve when possible the returned `Promise` as fallback ([#6476](https://github.com/pocketbase/pocketbase/issues/6476)).
+
 - Added `store.Store.SetFunc(key, func(old T) new T)` to set/update a store value with the return result of the callback in a concurrent safe manner.
 
 - Added `subscription.Message.WriteSSE(w, id)` for writing an SSE formatted message into the provided writer interface (_used mostly to assist with the unit testing_).
 
+
 - Bumped the default request read and write timeouts to 5mins (_old 3mins_) to accommodate slower internet connections and larger file uploads/downloads.
     _If you want to change them you can modify the `OnServe` hook's `ServeEvent.ReadTimeout/WriteTimeout` fields as shown in [#6550](https://github.com/pocketbase/pocketbase/discussions/6550#discussioncomment-12364515)._
 

plugins/jsvm/binds.go

@@ -83,12 +83,10 @@ func hooksBinds(app core.App, loader *goja.Runtime, executors *vmsPool) {
 					res, err := executor.RunProgram(pr)
 					executor.Set("__args", goja.Undefined())
 
-					// (legacy) check for returned Go error value
-					if res != nil {
-						if resErr, ok := res.Export().(error); ok {
+					// check for returned Go error value
+					if resErr := checkGojaValueForError(app, res); resErr != nil {
 						return resErr
 					}
-					}
 
 					return normalizeException(err)
 				})
@@ -199,11 +197,9 @@ func wrapHandlerFunc(executors *vmsPool, handler goja.Value) (func(*core.Request
 				res, err := executor.RunProgram(pr)
 				executor.Set("__args", goja.Undefined())
 
-				// (legacy) check for returned Go error value
-				if res != nil {
-					if v, ok := res.Export().(error); ok {
-						return v
-					}
+				// check for returned Go error value
+				if resErr := checkGojaValueForError(e.App, res); resErr != nil {
+					return resErr
 				}
 
 				return normalizeException(err)
@@ -256,11 +252,9 @@ func wrapMiddlewares(executors *vmsPool, rawMiddlewares ...goja.Value) ([]*hook.
 						res, err := executor.RunProgram(pr)
 						executor.Set("__args", goja.Undefined())
 
-						// (legacy) check for returned Go error value
-						if res != nil {
-							if v, ok := res.Export().(error); ok {
-								return v
-							}
+						// check for returned Go error value
+						if resErr := checkGojaValueForError(e.App, res); resErr != nil {
+							return resErr
 						}
 
 						return normalizeException(err)
@@ -278,11 +272,9 @@ func wrapMiddlewares(executors *vmsPool, rawMiddlewares ...goja.Value) ([]*hook.
 						res, err := executor.RunProgram(pr)
 						executor.Set("__args", goja.Undefined())
 
-						// (legacy) check for returned Go error value
-						if res != nil {
-							if v, ok := res.Export().(error); ok {
-								return v
-							}
+						// check for returned Go error value
+						if resErr := checkGojaValueForError(e.App, res); resErr != nil {
+							return resErr
 						}
 
 						return normalizeException(err)
@@ -920,6 +912,29 @@ func httpClientBinds(vm *goja.Runtime) {
 
 // -------------------------------------------------------------------
 
+// checkGojaValueForError resolves the provided goja.Value and tries
+// to extract its underlying error value (if any).
+func checkGojaValueForError(app core.App, value goja.Value) error {
+	if value == nil {
+		return nil
+	}
+
+	exported := value.Export()
+	switch v := exported.(type) {
+	case error:
+		return v
+	case *goja.Promise:
+		// Promise as return result is not officially supported but try to
+		// resolve any thrown exception to avoid silently ignoring it
+		app.Logger().Warn("the handler must a non-async function and not return a Promise")
+		if promiseErr, ok := v.Result().Export().(error); ok {
+			return normalizeException(promiseErr)
+		}
+	}
+
+	return nil
+}
+
 // normalizeException checks if the provided error is a goja.Exception
 // and attempts to return its underlying Go error.
 //

plugins/jsvm/binds_test.go

@@ -83,7 +83,7 @@ func TestBaseBindsReaderToString(t *testing.T) {
 	}
 }
 
-func TestBaseBindsToString(t *testing.T) {
+func TestBaseBindsToStringAndToBytes(t *testing.T) {
 	vm := goja.New()
 	baseBinds(vm)
 	vm.Set("scenarios", []struct {
@@ -1597,13 +1597,14 @@ func TestRouterBinds(t *testing.T) {
 	defer app.Cleanup()
 
 	result := &struct {
-		AddCount  int
-		WithCount int
+		RouteMiddlewareCalls  int
+		GlobalMiddlewareCalls int
 	}{}
 
 	vmFactory := func() *goja.Runtime {
 		vm := goja.New()
 		baseBinds(vm)
+		apisBinds(vm)
 		vm.Set("$app", app)
 		vm.Set("result", result)
 		return vm
@@ -1616,14 +1617,20 @@ func TestRouterBinds(t *testing.T) {
 
 	_, err := vm.RunString(`
 		routerAdd("GET", "/test", (e) => {
-			result.addCount++;
+			result.routeMiddlewareCalls++;
 		}, (e) => {
-			result.addCount++;
+			result.routeMiddlewareCalls++;
 			return e.next();
 		})
 
+		// Promise is not technically supported as return result
+		// but we try to resolve it at least for thrown errors
+		routerAdd("GET", "/error", async (e) => {
+			throw new ApiError(456, 'test', null)
+		})
+
 		routerUse((e) => {
-			result.withCount++;
+			result.globalMiddlewareCalls++;
 
 			return e.next();
 		})
@@ -1644,21 +1651,44 @@ func TestRouterBinds(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	rec := httptest.NewRecorder()
-	req := httptest.NewRequest("GET", "/test", nil)
-
 	mux, err := serveEvent.Router.BuildMux()
 	if err != nil {
 		t.Fatalf("Failed to build router mux: %v", err)
 	}
-	mux.ServeHTTP(rec, req)
 
-	if result.AddCount != 2 {
-		t.Fatalf("Expected AddCount %d, got %d", 2, result.AddCount)
+	scenarios := []struct {
+		method                        string
+		path                          string
+		expectedRouteMiddlewareCalls  int
+		expectedGlobalMiddlewareCalls int
+		expectedCode                  int
+	}{
+		{"GET", "/test", 2, 1, 200},
+		{"GET", "/error", 0, 1, 456},
 	}
 
-	if result.WithCount != 1 {
-		t.Fatalf("Expected WithCount %d, got %d", 1, result.WithCount)
+	for _, s := range scenarios {
+		t.Run(s.method+" "+s.path, func(t *testing.T) {
+			// reset
+			result.RouteMiddlewareCalls = 0
+			result.GlobalMiddlewareCalls = 0
+
+			rec := httptest.NewRecorder()
+			req := httptest.NewRequest(s.method, s.path, nil)
+			mux.ServeHTTP(rec, req)
+
+			if result.RouteMiddlewareCalls != s.expectedRouteMiddlewareCalls {
+				t.Fatalf("Expected RouteMiddlewareCalls %d, got %d", s.expectedRouteMiddlewareCalls, result.RouteMiddlewareCalls)
+			}
+
+			if result.GlobalMiddlewareCalls != s.expectedGlobalMiddlewareCalls {
+				t.Fatalf("Expected GlobalMiddlewareCalls %d, got %d", s.expectedGlobalMiddlewareCalls, result.GlobalMiddlewareCalls)
+			}
+
+			if rec.Code != s.expectedCode {
+				t.Fatalf("Expected status code %d, got %d", s.expectedCode, rec.Code)
+			}
+		})
 	}
 }