From 13f822ad7f0d43bae877387aff7ef676284f2f57 Mon Sep 17 00:00:00 2001 From: Reed Allman Date: Wed, 26 Jul 2017 04:22:58 -0700 Subject: [PATCH 1/3] catch request panics in goroutine the async stuff uses carlos supervisor thing but in the normal request path we aren't catching any panics and returning a 500 to user (conn just gets closed & server dies). should catch any mistakes we might make, or any one of the 10000 libraries we're importing. closes #150 --- api/server/server.go | 45 ++++++++++++++++++++++++++++---------------- 1 file changed, 29 insertions(+), 16 deletions(-) diff --git a/api/server/server.go b/api/server/server.go index 7bf693e1f..0931de934 100644 --- a/api/server/server.go +++ b/api/server/server.go @@ -14,12 +14,6 @@ import ( "github.com/Sirupsen/logrus" "github.com/ccirello/supervisor" - "github.com/gin-gonic/gin" - "github.com/opentracing/opentracing-go" - "github.com/opentracing/opentracing-go/ext" - "github.com/openzipkin/zipkin-go-opentracing" - "github.com/patrickmn/go-cache" - "github.com/spf13/viper" "github.com/fnproject/fn/api" "github.com/fnproject/fn/api/datastore" "github.com/fnproject/fn/api/id" @@ -28,6 +22,12 @@ import ( "github.com/fnproject/fn/api/mqs" "github.com/fnproject/fn/api/runner" "github.com/fnproject/fn/api/runner/common" + "github.com/gin-gonic/gin" + "github.com/opentracing/opentracing-go" + "github.com/opentracing/opentracing-go/ext" + "github.com/openzipkin/zipkin-go-opentracing" + "github.com/patrickmn/go-cache" + "github.com/spf13/viper" ) const ( @@ -109,7 +109,7 @@ func New(ctx context.Context, ds models.Datastore, mq models.MessageQueue, logDB setMachineId() setTracer() - s.Router.Use(loggerWrap, traceWrap) + s.Router.Use(loggerWrap, traceWrap, panicWrap) s.bindHandlers(ctx) for _, opt := range opts { @@ -203,6 +203,19 @@ func whoAmI() net.IP { return nil } +func panicWrap(c *gin.Context) { + defer func(c *gin.Context) { + if rec := recover(); rec != nil { + err, ok := rec.(error) + if !ok { + err = fmt.Errorf("fn: %v", rec) + } + handleErrorResponse(c, err) + } + }(c) + c.Next() +} + func loggerWrap(c *gin.Context) { ctx, _ := common.LoggerWithFields(c.Request.Context(), extractFields(c)) @@ -295,15 +308,15 @@ func (s *Server) startGears(ctx context.Context) { } const runHeader = ` - ____ __ - / __ \_________ ______/ /__ - / / / / ___/ __ / ___/ / _ \ - / /_/ / / / /_/ / /__/ / __/ - \_________ \__,_/\___/_/\____ - / ____/_ __ ___ _____/ /_( )___ ____ _____ - / /_ / / / / __ \/ ___/ __/ / __ \/ __ \/ ___/ - / __/ / /_/ / / / / /__/ /_/ / /_/ / / / (__ ) - /_/ \____/_/ /_/\___/\__/_/\____/_/ /_/____/ + ____ __ + / __ \_________ ______/ /__ + / / / / ___/ __ / ___/ / _ \ + / /_/ / / / /_/ / /__/ / __/ + \_________ \__,_/\___/_/\____ + / ____/_ __ ___ _____/ /_( )___ ____ _____ + / /_ / / / / __ \/ ___/ __/ / __ \/ __ \/ ___/ + / __/ / /_/ / / / / /__/ /_/ / /_/ / / / (__ ) + /_/ \____/_/ /_/\___/\__/_/\____/_/ /_/____/ ` fmt.Println(runHeader) logrus.Infof("Serving Functions API on address `%s`", listen) From f9948aa4cb014cd37ccd458e3a426dc8c41dd101 Mon Sep 17 00:00:00 2001 From: Reed Allman Date: Wed, 26 Jul 2017 04:28:53 -0700 Subject: [PATCH 2/3] fix up go-fmt'd oracle fn thing --- api/server/server.go | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/api/server/server.go b/api/server/server.go index 0931de934..f4132ce9b 100644 --- a/api/server/server.go +++ b/api/server/server.go @@ -308,16 +308,12 @@ func (s *Server) startGears(ctx context.Context) { } const runHeader = ` - ____ __ - / __ \_________ ______/ /__ - / / / / ___/ __ / ___/ / _ \ - / /_/ / / / /_/ / /__/ / __/ - \_________ \__,_/\___/_/\____ - / ____/_ __ ___ _____/ /_( )___ ____ _____ - / /_ / / / / __ \/ ___/ __/ / __ \/ __ \/ ___/ - / __/ / /_/ / / / / /__/ /_/ / /_/ / / / (__ ) - /_/ \____/_/ /_/\___/\__/_/\____/_/ /_/____/ - ` + ______ + / ____/___ + / /_ / __ \ + / __/ / / / / + /_/ /_/ /_/ +` fmt.Println(runHeader) logrus.Infof("Serving Functions API on address `%s`", listen) From 9cdd3befe3d4b5d520a22060f53eb99c664266ac Mon Sep 17 00:00:00 2001 From: Reed Allman Date: Wed, 26 Jul 2017 04:37:35 -0700 Subject: [PATCH 3/3] add abortion --- api/server/server.go | 1 + 1 file changed, 1 insertion(+) diff --git a/api/server/server.go b/api/server/server.go index f4132ce9b..115e5388f 100644 --- a/api/server/server.go +++ b/api/server/server.go @@ -211,6 +211,7 @@ func panicWrap(c *gin.Context) { err = fmt.Errorf("fn: %v", rec) } handleErrorResponse(c, err) + c.Abort() } }(c) c.Next()