From 71a70bac9d2623df65a21b8c7f10735d2661cbe9 Mon Sep 17 00:00:00 2001
From: Gani Georgiev <gani.georgiev@gmail.com>
Date: Tue, 18 Jul 2023 12:33:18 +0300
Subject: [PATCH] updated jsvm errors handling

---
 apis/base.go          | 17 +++++++----------
 apis/base_test.go     |  1 -
 plugins/jsvm/binds.go | 35 ++++++++++++++++++++++++-----------
 plugins/jsvm/jsvm.go  | 34 ++++++++++++++++++++++++++++++++++
 4 files changed, 65 insertions(+), 22 deletions(-)

diff --git a/apis/base.go b/apis/base.go
index e9438bc3..fc3b3a4e 100644
--- a/apis/base.go
+++ b/apis/base.go
@@ -2,6 +2,7 @@
 package apis
 
 import (
+	"database/sql"
 	"errors"
 	"fmt"
 	"io/fs"
@@ -11,7 +12,6 @@ import (
 	"path/filepath"
 	"strings"
 
-	"github.com/dop251/goja"
 	"github.com/labstack/echo/v5"
 	"github.com/labstack/echo/v5/middleware"
 	"github.com/pocketbase/pocketbase/core"
@@ -59,14 +59,6 @@ func InitApi(app core.App) (*echo.Echo, error) {
 			return
 		}
 
-		// manually extract the goja exception error value for
-		// consistency when throwing or returning errors
-		if jsException, ok := err.(*goja.Exception); ok {
-			if wrapped, ok := jsException.Value().Export().(error); ok {
-				err = wrapped
-			}
-		}
-
 		var apiErr *ApiError
 
 		switch v := err.(type) {
@@ -85,7 +77,12 @@ func InitApi(app core.App) (*echo.Echo, error) {
 			if err != nil && app.IsDebug() {
 				log.Println(err)
 			}
-			apiErr = NewBadRequestError("", err)
+
+			if err != nil && errors.Is(err, sql.ErrNoRows) {
+				apiErr = NewNotFoundError("", err)
+			} else {
+				apiErr = NewBadRequestError("", err)
+			}
 		}
 
 		event := new(core.ApiErrorEvent)
diff --git a/apis/base_test.go b/apis/base_test.go
index 2dfbd2b2..c93e8a9c 100644
--- a/apis/base_test.go
+++ b/apis/base_test.go
@@ -214,7 +214,6 @@ func TestRemoveTrailingSlashMiddleware(t *testing.T) {
 }
 
 func TestEagerRequestInfoCache(t *testing.T) {
-
 	scenarios := []tests.ApiScenario{
 		{
 			Name:   "custom non-api group route",
diff --git a/plugins/jsvm/binds.go b/plugins/jsvm/binds.go
index 08c5b070..4d00a0ce 100644
--- a/plugins/jsvm/binds.go
+++ b/plugins/jsvm/binds.go
@@ -77,15 +77,12 @@ func hooksBinds(app core.App, loader *goja.Runtime, executors *vmsPool) {
 
 					// check for returned error or false
 					if res != nil {
-						exported := res.Export()
-						if exported != nil {
-							switch v := exported.(type) {
-							case error:
-								return v
-							case bool:
-								if !v {
-									return hook.StopPropagation
-								}
+						switch v := res.Export().(type) {
+						case error:
+							return v
+						case bool:
+							if !v {
+								return hook.StopPropagation
 							}
 						}
 					}
@@ -156,8 +153,16 @@ func routerBinds(app core.App, loader *goja.Runtime, executors *vmsPool) {
 			e.Router.Add(strings.ToUpper(method), path, func(c echo.Context) error {
 				return executors.run(func(executor *goja.Runtime) error {
 					executor.Set("__args", []any{c})
-					_, err := executor.RunProgram(pr)
+					res, err := executor.RunProgram(pr)
 					executor.Set("__args", goja.Undefined())
+
+					// check for returned error
+					if res != nil {
+						if v, ok := res.Export().(error); ok {
+							return v
+						}
+					}
+
 					return err
 				})
 			}, wrappedMiddlewares...)
@@ -207,9 +212,17 @@ func wrapMiddlewares(executors *vmsPool, rawMiddlewares ...goja.Value) ([]echo.M
 					return executors.run(func(executor *goja.Runtime) error {
 						executor.Set("__args", []any{next})
 						executor.Set("__args2", []any{c})
-						_, err := executor.RunProgram(pr)
+						res, err := executor.RunProgram(pr)
 						executor.Set("__args", goja.Undefined())
 						executor.Set("__args2", goja.Undefined())
+
+						// check for returned error
+						if res != nil {
+							if v, ok := res.Export().(error); ok {
+								return v
+							}
+						}
+
 						return err
 					})
 				}
diff --git a/plugins/jsvm/jsvm.go b/plugins/jsvm/jsvm.go
index d965ef67..42159e6a 100644
--- a/plugins/jsvm/jsvm.go
+++ b/plugins/jsvm/jsvm.go
@@ -22,6 +22,7 @@ import (
 	"github.com/dop251/goja_nodejs/require"
 	"github.com/fatih/color"
 	"github.com/fsnotify/fsnotify"
+	"github.com/labstack/echo/v5"
 	"github.com/pocketbase/dbx"
 	"github.com/pocketbase/pocketbase/core"
 	m "github.com/pocketbase/pocketbase/migrations"
@@ -191,6 +192,11 @@ func (p *plugin) registerHooks() error {
 		return nil
 	}
 
+	p.app.OnBeforeServe().Add(func(e *core.ServeEvent) error {
+		e.Router.HTTPErrorHandler = p.normalizeServeExceptions(e.Router.HTTPErrorHandler)
+		return nil
+	})
+
 	// this is safe to be shared across multiple vms
 	registry := new(require.Registry)
 
@@ -236,6 +242,34 @@ func (p *plugin) registerHooks() error {
 	return nil
 }
 
+// normalizeExceptions wraps the provided error handler and returns a new one
+// with extracted goja exception error value for consistency when throwing or returning errors.
+func (p *plugin) normalizeServeExceptions(oldErrorHandler echo.HTTPErrorHandler) echo.HTTPErrorHandler {
+	return func(c echo.Context, err error) {
+		defer func() {
+			oldErrorHandler(c, err)
+		}()
+
+		if err == nil || c.Response().Committed {
+			return // no error or already committed
+		}
+
+		jsException, ok := err.(*goja.Exception)
+		if !ok {
+			return // no exception
+		}
+
+		switch v := jsException.Value().Export().(type) {
+		case error:
+			err = v
+		case map[string]any: // goja.GoError
+			if vErr, ok := v["value"].(error); ok {
+				err = vErr
+			}
+		}
+	}
+}
+
 // watchHooks initializes a hooks file watcher that will restart the
 // application (*if possible) in case of a change in the hooks directory.
 //