Merge pull request #46 from krakend/content_typed_errors

Support a third optional argument in the custom_error helper for the content-type
krakend · Oct 24, 2024 · c062d75 · c062d75
2 parents 3a2f876 + 4dab457
commit c062d75
Show file tree

Hide file tree

Showing 6 changed files with 178 additions and 29 deletions.
diff --git a/errors.go b/errors.go
@@ -50,6 +50,17 @@ func (e ErrInternalHTTP) Error() string {
 	return e.msg
 }
 
+type ErrInternalHTTPWithContentType struct {
+	ErrInternalHTTP
+	contentType string
+}
+
+func (e ErrInternalHTTPWithContentType) Encoding() string {
+	return e.contentType
+}
+
+const separator = " || "
+
 func ToError(e error) error {
 	if e == nil {
 		return nil
@@ -60,17 +71,29 @@ func ToError(e error) error {
 	}
 
 	originalMsg := e.Error()
-	start := strings.Index(originalMsg, ":")
+	errMsgParts := strings.Split(originalMsg[strings.Index(originalMsg, ":")+2:], separator)
 
-	if l := len(originalMsg); originalMsg[l-1] == ')' && originalMsg[l-5] == '(' {
-		code, err := strconv.Atoi(originalMsg[l-4 : l-1])
-		if err != nil {
-			code = 500
-		}
-		return ErrInternalHTTP{msg: originalMsg[start+2 : l-6], code: code}
+	if len(errMsgParts) == 0 {
+		return e
 	}
+	if len(errMsgParts) == 1 {
+		return ErrInternal(errMsgParts[0])
+	}
+
+	code, err := strconv.Atoi(errMsgParts[1])
+	if err != nil {
+		code = 500
+	}
+	errHTTP := ErrInternalHTTP{msg: errMsgParts[0], code: code}
 
-	return ErrInternal(originalMsg[start+2:])
+	if len(errMsgParts) == 2 {
+		return errHTTP
+	}
+
+	return ErrInternalHTTPWithContentType{
+		ErrInternalHTTP: errHTTP,
+		contentType:     errMsgParts[2],
+	}
 }
 
 func RegisterErrors(b *binder.Binder) {
@@ -80,8 +103,10 @@ func RegisterErrors(b *binder.Binder) {
 			return errNeedsArguments
 		case 1:
 			return errors.New(c.Arg(1).String())
+		case 2:
+			return fmt.Errorf("%s%s%d", c.Arg(1).String(), separator, int(c.Arg(2).Number()))
 		default:
-			return fmt.Errorf("%s (%d)", c.Arg(1).String(), int(c.Arg(2).Number()))
+			return fmt.Errorf("%s%s%d%s%s", c.Arg(1).String(), separator, int(c.Arg(2).Number()), separator, c.Arg(3).String())
 		}
 	})
 }
diff --git a/proxy/proxy_test.go b/proxy/proxy_test.go
@@ -20,21 +20,26 @@ import (
 )
 
 func TestProxyFactory_error(t *testing.T) {
-	testProxyFactoryError(t, `custom_error('expect me')`, "expect me", false, 0)
-	testProxyFactoryPostError(t, `custom_error('expect me')`, "expect me", false, 0)
+	testProxyFactoryError(t, `custom_error('expect me')`, "expect me", "", false, 0)
+	testProxyFactoryPostError(t, `custom_error('expect me')`, "expect me", "", false, 0)
 }
 
 func TestProxyFactory_errorHTTP(t *testing.T) {
-	testProxyFactoryError(t, `custom_error('expect me', 404)`, "expect me", true, 404)
-	testProxyFactoryPostError(t, `custom_error('expect me', 404)`, "expect me", true, 404)
+	testProxyFactoryError(t, `custom_error('expect me', 404)`, "expect me", "", true, 404)
+	testProxyFactoryPostError(t, `custom_error('expect me', 404)`, "expect me", "", true, 404)
 }
 
 func TestProxyFactory_errorHTTPJson(t *testing.T) {
-	testProxyFactoryError(t, `custom_error('{"msg":"expect me"}', 404)`, `{"msg":"expect me"}`, true, 404)
-	testProxyFactoryPostError(t, `custom_error('{"msg":"expect me"}', 404)`, `{"msg":"expect me"}`, true, 404)
+	testProxyFactoryError(t, `custom_error('{"msg":"expect me"}', 404)`, `{"msg":"expect me"}`, "", true, 404)
+	testProxyFactoryPostError(t, `custom_error('{"msg":"expect me"}', 404)`, `{"msg":"expect me"}`, "", true, 404)
 }
 
-func testProxyFactoryError(t *testing.T, code, errMsg string, isHTTP bool, statusCode int) {
+func TestProxyFactory_errorHTTPWithContentType(t *testing.T) {
+	testProxyFactoryError(t, `custom_error('{"msg":"expect me"}', 404, 'application/json')`, `{"msg":"expect me"}`, "application/json", true, 404)
+	testProxyFactoryPostError(t, `custom_error('{"msg":"expect me"}', 404, 'application/json')`, `{"msg":"expect me"}`, "application/json", true, 404)
+}
+
+func testProxyFactoryError(t *testing.T, code, errMsg, contentType string, isHTTP bool, statusCode int) {
 	buff := bytes.NewBuffer(make([]byte, 1024))
 	logger, err := logging.NewLogger("ERROR", buff, "pref")
 	if err != nil {
@@ -99,6 +104,19 @@ func testProxyFactoryError(t *testing.T, code, errMsg string, isHTTP bool, statu
 			t.Errorf("unexpected internal error: %v (%T)", err, err)
 			return
 		}
+	case lua.ErrInternalHTTPWithContentType:
+		if !isHTTP {
+			t.Errorf("unexpected http error: %v (%T)", err, err)
+			return
+		}
+		if sc := err.StatusCode(); sc != statusCode {
+			t.Errorf("unexpected http status code: %d", sc)
+			return
+		}
+		if ct := err.Encoding(); ct != contentType {
+			t.Errorf("unexpected content type: %s", ct)
+			return
+		}
 	default:
 		t.Errorf("unexpected error: %v (%T)", err, err)
 		return
@@ -110,7 +128,7 @@ func testProxyFactoryError(t *testing.T, code, errMsg string, isHTTP bool, statu
 	}
 }
 
-func testProxyFactoryPostError(t *testing.T, code, errMsg string, isHTTP bool, statusCode int) {
+func testProxyFactoryPostError(t *testing.T, code, errMsg, contentType string, isHTTP bool, statusCode int) {
 	buff := bytes.NewBuffer(make([]byte, 1024))
 	logger, err := logging.NewLogger("ERROR", buff, "pref")
 	if err != nil {
@@ -173,6 +191,19 @@ func testProxyFactoryPostError(t *testing.T, code, errMsg string, isHTTP bool, s
 			t.Errorf("unexpected internal error: %v (%T)", err, err)
 			return
 		}
+	case lua.ErrInternalHTTPWithContentType:
+		if !isHTTP {
+			t.Errorf("unexpected http error: %v (%T)", err, err)
+			return
+		}
+		if sc := err.StatusCode(); sc != statusCode {
+			t.Errorf("unexpected http status code: %d", sc)
+			return
+		}
+		if ct := err.Encoding(); ct != contentType {
+			t.Errorf("unexpected content type: %s", ct)
+			return
+		}
 	default:
 		t.Errorf("unexpected error: %v (%T)", err, err)
 		return

diff --git a/router/gin/lua.go b/router/gin/lua.go
@@ -58,6 +58,9 @@ func HandlerFactory(l logging.Logger, next krakendgin.HandlerFactory) krakendgin
 			if err := process(c, &cfg); err != nil {
 				err = lua.ToError(err)
 				if errhttp, ok := err.(errHTTP); ok {
+					if e, ok := err.(errHTTPWithContentType); ok {
+						c.Writer.Header().Add("content-type", e.Encoding())
+					}
 					c.AbortWithError(errhttp.StatusCode(), err)
 					return
 				}
@@ -75,6 +78,11 @@ type errHTTP interface {
 	StatusCode() int
 }
 
+type errHTTPWithContentType interface {
+	errHTTP
+	Encoding() string
+}
+
 func process(c *gin.Context, cfg *lua.Config) error {
 	b := binder.New(binder.Options{
 		SkipOpenLibs:        !cfg.AllowOpenLibs,

diff --git a/router/gin/lua_test.go b/router/gin/lua_test.go
@@ -73,7 +73,7 @@ func TestHandlerFactory(t *testing.T) {
 	engine := gin.New()
 	engine.GET("/some-path/:id", handler)
 
-	req, _ := http.NewRequest("GET", "/some-path/42?id=1", nil)
+	req, _ := http.NewRequest("GET", "/some-path/42?id=1", http.NoBody)
 	req.Host = "domain.tld"
 	req.Header.Set("Accept", "application/json")
 	w := httptest.NewRecorder()
@@ -98,7 +98,7 @@ func TestHandlerFactory_error(t *testing.T) {
 	}
 
 	hf := func(_ *config.EndpointConfig, _ proxy.Proxy) gin.HandlerFunc {
-		return func(c *gin.Context) {
+		return func(_ *gin.Context) {
 			t.Error("the handler shouldn't be executed")
 		}
 	}
@@ -107,7 +107,7 @@ func TestHandlerFactory_error(t *testing.T) {
 	engine := gin.New()
 	engine.GET("/some-path/:id", handler)
 
-	req, _ := http.NewRequest("GET", "/some-path/42?id=1", nil)
+	req, _ := http.NewRequest("GET", "/some-path/42?id=1", http.NoBody)
 	req.Header.Set("Accept", "application/json")
 	w := httptest.NewRecorder()
 
@@ -131,7 +131,40 @@ func TestHandlerFactory_errorHTTP(t *testing.T) {
 	}
 
 	hf := func(_ *config.EndpointConfig, _ proxy.Proxy) gin.HandlerFunc {
-		return func(c *gin.Context) {
+		return func(_ *gin.Context) {
+			t.Error("the handler shouldn't be executed")
+		}
+	}
+	handler := HandlerFactory(logging.NoOp, hf)(cfg, proxy.NoopProxy)
+
+	engine := gin.New()
+	engine.GET("/some-path/:id", handler)
+
+	req, _ := http.NewRequest("GET", "/some-path/42?id=1", http.NoBody)
+	req.Header.Set("Accept", "application/json")
+	w := httptest.NewRecorder()
+
+	engine.ServeHTTP(w, req)
+
+	if w.Code != 999 {
+		t.Errorf("unexpected status code %d", w.Code)
+		return
+	}
+}
+
+func TestHandlerFactory_errorHTTPWithContentType(t *testing.T) {
+	gin.SetMode(gin.TestMode)
+	cfg := &config.EndpointConfig{
+		Endpoint: "/",
+		ExtraConfig: config.ExtraConfig{
+			router.Namespace: map[string]interface{}{
+				"pre": `custom_error('expect me', 999, 'foo/bar')`,
+			},
+		},
+	}
+
+	hf := func(_ *config.EndpointConfig, _ proxy.Proxy) gin.HandlerFunc {
+		return func(_ *gin.Context) {
 			t.Error("the handler shouldn't be executed")
 		}
 	}
@@ -140,7 +173,7 @@ func TestHandlerFactory_errorHTTP(t *testing.T) {
 	engine := gin.New()
 	engine.GET("/some-path/:id", handler)
 
-	req, _ := http.NewRequest("GET", "/some-path/42?id=1", nil)
+	req, _ := http.NewRequest("GET", "/some-path/42?id=1", http.NoBody)
 	req.Header.Set("Accept", "application/json")
 	w := httptest.NewRecorder()
 
@@ -150,4 +183,9 @@ func TestHandlerFactory_errorHTTP(t *testing.T) {
 		t.Errorf("unexpected status code %d", w.Code)
 		return
 	}
+
+	if h := w.Header().Get("content-type"); h != "foo/bar" {
+		t.Errorf("unexpected content-type %s", h)
+		return
+	}
 }
diff --git a/router/mux/lua.go b/router/mux/lua.go
@@ -3,6 +3,7 @@ package mux
 import (
 	"bytes"
 	"errors"
+	"fmt"
 	"io"
 	"net/http"
 	"net/url"
@@ -66,7 +67,12 @@ func HandlerFactory(l logging.Logger, next mux.HandlerFactory, pe mux.ParamExtra
 			if err := process(r, pe, &cfg); err != nil {
 				err = lua.ToError(err)
 				if errhttp, ok := err.(errHTTP); ok {
-					http.Error(w, err.Error(), errhttp.StatusCode())
+					if e, ok := err.(errHTTPWithContentType); ok {
+						fmt.Println(e.Encoding())
+						w.Header().Add("content-type", e.Encoding())
+					}
+					w.WriteHeader(errhttp.StatusCode())
+					w.Write([]byte(err.Error()))
 					return
 				}
 
@@ -84,6 +90,11 @@ type errHTTP interface {
 	StatusCode() int
 }
 
+type errHTTPWithContentType interface {
+	errHTTP
+	Encoding() string
+}
+
 func process(r *http.Request, pe mux.ParamExtractor, cfg *lua.Config) error {
 	b := binder.New(binder.Options{
 		SkipOpenLibs:        !cfg.AllowOpenLibs,

diff --git a/router/mux/lua_test.go b/router/mux/lua_test.go
@@ -33,7 +33,7 @@ func TestHandlerFactory(t *testing.T) {
 	}
 
 	hf := func(_ *config.EndpointConfig, _ proxy.Proxy) http.HandlerFunc {
-		return func(w http.ResponseWriter, r *http.Request) {
+		return func(_ http.ResponseWriter, r *http.Request) {
 			if URL := r.URL.String(); URL != "/some-path/42?extra=foo&id=1&more=true" {
 				t.Errorf("unexpected URL: %s", URL)
 			}
@@ -66,7 +66,7 @@ func TestHandlerFactory(t *testing.T) {
 		return map[string]string{}
 	})(cfg, proxy.NoopProxy)
 
-	req, _ := http.NewRequest("GET", "/some-path/42?id=1", nil)
+	req, _ := http.NewRequest("GET", "/some-path/42?id=1", http.NoBody)
 	req.Header.Set("Accept", "application/json")
 	w := httptest.NewRecorder()
 
@@ -89,15 +89,15 @@ func TestHandlerFactory_error(t *testing.T) {
 	}
 
 	hf := func(_ *config.EndpointConfig, _ proxy.Proxy) http.HandlerFunc {
-		return func(w http.ResponseWriter, r *http.Request) {
+		return func(_ http.ResponseWriter, _ *http.Request) {
 			t.Error("the handler shouldn't be executed")
 		}
 	}
 	handler := HandlerFactory(logging.NoOp, hf, func(_ *http.Request) map[string]string {
 		return map[string]string{}
 	})(cfg, proxy.NoopProxy)
 
-	req, _ := http.NewRequest("GET", "/some-path/42?id=1", nil)
+	req, _ := http.NewRequest("GET", "/some-path/42?id=1", http.NoBody)
 	req.Header.Set("Accept", "application/json")
 	w := httptest.NewRecorder()
 
@@ -120,15 +120,15 @@ func TestHandlerFactory_errorHTTP(t *testing.T) {
 	}
 
 	hf := func(_ *config.EndpointConfig, _ proxy.Proxy) http.HandlerFunc {
-		return func(w http.ResponseWriter, r *http.Request) {
+		return func(_ http.ResponseWriter, _ *http.Request) {
 			t.Error("the handler shouldn't be executed")
 		}
 	}
 	handler := HandlerFactory(logging.NoOp, hf, func(_ *http.Request) map[string]string {
 		return map[string]string{}
 	})(cfg, proxy.NoopProxy)
 
-	req, _ := http.NewRequest("GET", "/some-path/42?id=1", nil)
+	req, _ := http.NewRequest("GET", "/some-path/42?id=1", http.NoBody)
 	req.Header.Set("Accept", "application/json")
 	w := httptest.NewRecorder()
 
@@ -139,3 +139,39 @@ func TestHandlerFactory_errorHTTP(t *testing.T) {
 		return
 	}
 }
+
+func TestHandlerFactory_errorHTTPWithContentType(t *testing.T) {
+	cfg := &config.EndpointConfig{
+		Endpoint: "/",
+		ExtraConfig: config.ExtraConfig{
+			router.Namespace: map[string]interface{}{
+				"pre": `custom_error('expect me', 999, 'foo/bar')`,
+			},
+		},
+	}
+
+	hf := func(_ *config.EndpointConfig, _ proxy.Proxy) http.HandlerFunc {
+		return func(_ http.ResponseWriter, _ *http.Request) {
+			t.Error("the handler shouldn't be executed")
+		}
+	}
+	handler := HandlerFactory(logging.NoOp, hf, func(_ *http.Request) map[string]string {
+		return map[string]string{}
+	})(cfg, proxy.NoopProxy)
+
+	req, _ := http.NewRequest("GET", "/some-path/42?id=1", http.NoBody)
+	req.Header.Set("Accept", "application/json")
+	w := httptest.NewRecorder()
+
+	handler(w, req)
+
+	if w.Code != 999 {
+		t.Errorf("unexpected status code %d", w.Code)
+		return
+	}
+
+	if h := w.Header().Get("content-type"); h != "foo/bar" {
+		t.Errorf("unexpected content-type %s", h)
+		return
+	}
+}