From caa7ca0f90dbda79977efd590d3713a9b8f148eb Mon Sep 17 00:00:00 2001 From: niksis02 Date: Tue, 23 Sep 2025 22:19:05 +0400 Subject: [PATCH] feat: implements fiber panic recovery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fiber includes a built-in panic recovery middleware that catches panics in route handlers and middlewares, preventing the server from crashing and allowing it to recover. Alongside this, a stack trace handler has been implemented to store system panics in the context locals (stack). Both the S3 API server and the Admin server use a global error handler to catch unexpected exceptions and recovered panics. The middleware’s logic is to log the panic or internal error and return an S3-style internal server error response. Additionally, dedicated **Panic** and **InternalError** loggers have been added to the `s3api` debug logger to record system panics and internal errors in the console. --- cmd/versitygw/main.go | 26 ++++-------------- debuglogger/logger.go | 34 +++++++++++++++++++---- s3api/admin-server.go | 20 ++++++++++++-- s3api/controllers/base.go | 3 +- s3api/server.go | 55 +++++++++++++++++++++++++++++++++++-- s3api/server_test.go | 54 ------------------------------------ s3api/utils/context-keys.go | 3 +- 7 files changed, 107 insertions(+), 88 deletions(-) diff --git a/cmd/versitygw/main.go b/cmd/versitygw/main.go index fc831e2..6d40588 100644 --- a/cmd/versitygw/main.go +++ b/cmd/versitygw/main.go @@ -25,7 +25,6 @@ import ( "os" "strings" - "github.com/gofiber/fiber/v2" "github.com/urfave/cli/v2" "github.com/versity/versitygw/auth" "github.com/versity/versitygw/backend" @@ -604,15 +603,6 @@ func runGateway(ctx context.Context, be backend.Backend) error { }() } - app := fiber.New(fiber.Config{ - AppName: "versitygw", - ServerHeader: "VERSITYGW", - StreamRequestBody: true, - DisableKeepalive: !keepAlive, - Network: fiber.NetworkTCP, - DisableStartupMessage: true, - }) - var opts []s3api.Option if certFile != "" || keyFile != "" { @@ -644,11 +634,12 @@ func runGateway(ctx context.Context, be backend.Backend) error { if virtualDomain != "" { opts = append(opts, s3api.WithHostStyle(virtualDomain)) } - + if keepAlive { + opts = append(opts, s3api.WithKeepAlive()) + } if debug { debuglogger.SetDebugEnabled() } - if iamDebug { debuglogger.SetIAMDebugEnabled() } @@ -733,7 +724,7 @@ func runGateway(ctx context.Context, be backend.Backend) error { return fmt.Errorf("init bucket event notifications: %w", err) } - srv, err := s3api.New(app, be, middlewares.RootUserConfig{ + srv, err := s3api.New(be, middlewares.RootUserConfig{ Access: rootUserAccess, Secret: rootUserSecret, }, port, region, iam, loggers.S3Logger, loggers.AdminLogger, evSender, metricsManager, opts...) @@ -744,13 +735,6 @@ func runGateway(ctx context.Context, be backend.Backend) error { var admSrv *s3api.S3AdminServer if admPort != "" { - admApp := fiber.New(fiber.Config{ - AppName: "versitygw", - ServerHeader: "VERSITYGW", - Network: fiber.NetworkTCP, - DisableStartupMessage: true, - }) - var opts []s3api.AdminOpt if admCertFile != "" || admKeyFile != "" { @@ -774,7 +758,7 @@ func runGateway(ctx context.Context, be backend.Backend) error { opts = append(opts, s3api.WithAdminDebug()) } - admSrv = s3api.NewAdminServer(admApp, be, middlewares.RootUserConfig{Access: rootUserAccess, Secret: rootUserSecret}, admPort, region, iam, loggers.AdminLogger, opts...) + admSrv = s3api.NewAdminServer(be, middlewares.RootUserConfig{Access: rootUserAccess, Secret: rootUserSecret}, admPort, region, iam, loggers.AdminLogger, opts...) } if !quiet { diff --git a/debuglogger/logger.go b/debuglogger/logger.go index c38d493..c2b5493 100644 --- a/debuglogger/logger.go +++ b/debuglogger/logger.go @@ -18,6 +18,7 @@ import ( "fmt" "log" "net/http" + "os" "strings" "sync/atomic" @@ -25,18 +26,39 @@ import ( ) type Color string +type prefix string const ( green Color = "\033[32m" yellow Color = "\033[33m" blue Color = "\033[34m" + red Color = "\033[31m" Purple Color = "\033[0;35m" + prefixPanic prefix = "[PANIC]: " + prefixInernalError prefix = "[INTERNAL ERROR]: " + prefixInfo prefix = "[INFO]: " + prefixDebug prefix = "[DEBUG]: " + reset = "\033[0m" borderChar = "─" boxWidth = 120 ) +// Panic prints the panics out in the console +func Panic(er error) { + printError(prefixPanic, er) +} + +// InernalError prints the internal error out in the console +func InernalError(er error) { + printError(prefixInernalError, er) +} + +func printError(prefix prefix, er error) { + fmt.Fprintf(os.Stderr, string(red)+string(prefix)+"%v"+reset+"\n", er) +} + // Logs http request details: headers, body, params, query args func LogFiberRequestDetails(ctx *fiber.Ctx) { // Log the full request url @@ -102,8 +124,8 @@ func Logf(format string, v ...any) { if !debugEnabled.Load() { return } - debugPrefix := "[DEBUG]: " - fmt.Printf(string(yellow)+debugPrefix+format+reset+"\n", v...) + + fmt.Printf(string(yellow)+string(prefixDebug)+format+reset+"\n", v...) } // Infof prints out green info block with [INFO]: prefix @@ -111,8 +133,8 @@ func Infof(format string, v ...any) { if !debugEnabled.Load() { return } - debugPrefix := "[INFO]: " - fmt.Printf(string(green)+debugPrefix+format+reset+"\n", v...) + + fmt.Printf(string(green)+string(prefixInfo)+format+reset+"\n", v...) } var debugIAMEnabled atomic.Bool @@ -133,8 +155,8 @@ func IAMLogf(format string, v ...any) { if !debugIAMEnabled.Load() { return } - debugPrefix := "[DEBUG]: " - fmt.Printf(string(yellow)+debugPrefix+format+reset+"\n", v...) + + fmt.Printf(string(yellow)+string(prefixDebug)+format+reset+"\n", v...) } // PrintInsideHorizontalBorders prints the text inside horizontal diff --git a/s3api/admin-server.go b/s3api/admin-server.go index 979f479..8fed246 100644 --- a/s3api/admin-server.go +++ b/s3api/admin-server.go @@ -19,6 +19,7 @@ import ( "github.com/gofiber/fiber/v2" "github.com/gofiber/fiber/v2/middleware/logger" + "github.com/gofiber/fiber/v2/middleware/recover" "github.com/versity/versitygw/auth" "github.com/versity/versitygw/backend" "github.com/versity/versitygw/s3api/controllers" @@ -36,9 +37,8 @@ type S3AdminServer struct { debug bool } -func NewAdminServer(app *fiber.App, be backend.Backend, root middlewares.RootUserConfig, port, region string, iam auth.IAMService, l s3log.AuditLogger, opts ...AdminOpt) *S3AdminServer { +func NewAdminServer(be backend.Backend, root middlewares.RootUserConfig, port, region string, iam auth.IAMService, l s3log.AuditLogger, opts ...AdminOpt) *S3AdminServer { server := &S3AdminServer{ - app: app, backend: be, router: new(S3AdminRouter), port: port, @@ -48,6 +48,22 @@ func NewAdminServer(app *fiber.App, be backend.Backend, root middlewares.RootUse opt(server) } + app := fiber.New(fiber.Config{ + AppName: "versitygw", + ServerHeader: "VERSITYGW", + Network: fiber.NetworkTCP, + DisableStartupMessage: true, + ErrorHandler: globalErrorHandler, + }) + + server.app = app + + app.Use(recover.New( + recover.Config{ + EnableStackTrace: true, + StackTraceHandler: stackTraceHandler, + })) + // Logging middlewares if !server.quiet { app.Use(logger.New(logger.Config{ diff --git a/s3api/controllers/base.go b/s3api/controllers/base.go index c56103c..3e1374c 100644 --- a/s3api/controllers/base.go +++ b/s3api/controllers/base.go @@ -18,7 +18,6 @@ import ( "encoding/xml" "fmt" "net/http" - "os" "github.com/gofiber/fiber/v2" "github.com/versity/versitygw/auth" @@ -201,7 +200,7 @@ func ProcessController(ctx *fiber.Ctx, controller Controller, s3action string, s return ctx.Send(s3err.GetAPIErrorResponse(serr, "", "", "")) } - fmt.Fprintf(os.Stderr, "Internal Error, %v\n", err) + debuglogger.InernalError(err) ctx.Status(http.StatusInternalServerError) // If the error is not 's3err.APIError' return 'InternalError' diff --git a/s3api/server.go b/s3api/server.go index d1859bd..01dd657 100644 --- a/s3api/server.go +++ b/s3api/server.go @@ -20,12 +20,15 @@ import ( "github.com/gofiber/fiber/v2" "github.com/gofiber/fiber/v2/middleware/logger" + "github.com/gofiber/fiber/v2/middleware/recover" "github.com/versity/versitygw/auth" "github.com/versity/versitygw/backend" "github.com/versity/versitygw/debuglogger" "github.com/versity/versitygw/metrics" "github.com/versity/versitygw/s3api/controllers" "github.com/versity/versitygw/s3api/middlewares" + "github.com/versity/versitygw/s3api/utils" + "github.com/versity/versitygw/s3err" "github.com/versity/versitygw/s3event" "github.com/versity/versitygw/s3log" ) @@ -38,12 +41,12 @@ type S3ApiServer struct { cert *tls.Certificate quiet bool readonly bool + keepAlive bool health string virtualDomain string } func New( - app *fiber.App, be backend.Backend, root middlewares.RootUserConfig, port, region string, @@ -55,7 +58,6 @@ func New( opts ...Option, ) (*S3ApiServer, error) { server := &S3ApiServer{ - app: app, backend: be, router: new(S3ApiRouter), port: port, @@ -65,6 +67,25 @@ func New( opt(server) } + app := fiber.New(fiber.Config{ + AppName: "versitygw", + ServerHeader: "VERSITYGW", + StreamRequestBody: true, + DisableKeepalive: !server.keepAlive, + Network: fiber.NetworkTCP, + DisableStartupMessage: true, + ErrorHandler: globalErrorHandler, + }) + + server.app = app + + // initialize the panic recovery middleware + app.Use(recover.New( + recover.Config{ + EnableStackTrace: true, + StackTraceHandler: stackTraceHandler, + })) + // Logging middlewares if !server.quiet { app.Use(logger.New(logger.Config{ @@ -132,9 +153,39 @@ func WithHostStyle(virtualDomain string) Option { return func(s *S3ApiServer) { s.virtualDomain = virtualDomain } } +// WithKeepAlive enables the server keep alive +func WithKeepAlive() Option { + return func(s *S3ApiServer) { s.keepAlive = true } +} + func (sa *S3ApiServer) Serve() (err error) { if sa.cert != nil { return sa.app.ListenTLSWithCertificate(sa.port, *sa.cert) } return sa.app.Listen(sa.port) } + +// stackTraceHandler stores the system panics +// in the context locals +func stackTraceHandler(ctx *fiber.Ctx, e any) { + utils.ContextKeyStack.Set(ctx, e) +} + +// globalErrorHandler catches the errors before reaching to +// the handlers and any system panics +func globalErrorHandler(ctx *fiber.Ctx, er error) error { + if utils.ContextKeyStack.IsSet(ctx) { + // if stack is set, it means the stack trace + // has caught a panic + // log it as a panic log + debuglogger.Panic(er) + } else { + // otherwise log it as an internal error + debuglogger.InernalError(er) + } + + ctx.Status(http.StatusInternalServerError) + + return ctx.Send(s3err.GetAPIErrorResponse( + s3err.GetAPIError(s3err.ErrInternalError), "", "", "")) +} diff --git a/s3api/server_test.go b/s3api/server_test.go index 64aa4fc..aede682 100644 --- a/s3api/server_test.go +++ b/s3api/server_test.go @@ -16,66 +16,12 @@ package s3api import ( "crypto/tls" - "reflect" "testing" "github.com/gofiber/fiber/v2" - "github.com/versity/versitygw/auth" "github.com/versity/versitygw/backend" - "github.com/versity/versitygw/s3api/middlewares" ) -func TestNew(t *testing.T) { - type args struct { - app *fiber.App - be backend.Backend - port string - root middlewares.RootUserConfig - } - - app := fiber.New() - be := backend.BackendUnsupported{} - router := S3ApiRouter{} - port := ":7070" - - tests := []struct { - name string - args args - wantS3ApiServer *S3ApiServer - wantErr bool - }{ - { - name: "Create S3 api server", - args: args{ - app: app, - be: be, - port: port, - root: middlewares.RootUserConfig{}, - }, - wantS3ApiServer: &S3ApiServer{ - app: app, - port: port, - router: &router, - backend: be, - }, - wantErr: false, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - gotS3ApiServer, err := New(tt.args.app, tt.args.be, tt.args.root, - tt.args.port, "us-east-1", &auth.IAMServiceInternal{}, nil, nil, nil, nil) - if (err != nil) != tt.wantErr { - t.Errorf("New() error = %v, wantErr %v", err, tt.wantErr) - return - } - if !reflect.DeepEqual(gotS3ApiServer, tt.wantS3ApiServer) { - t.Errorf("New() = %v, want %v", gotS3ApiServer, tt.wantS3ApiServer) - } - }) - } -} - func TestS3ApiServer_Serve(t *testing.T) { tests := []struct { name string diff --git a/s3api/utils/context-keys.go b/s3api/utils/context-keys.go index 2157240..fdb3cf1 100644 --- a/s3api/utils/context-keys.go +++ b/s3api/utils/context-keys.go @@ -19,7 +19,7 @@ import ( ) // Region, StartTime, IsRoot, Account, AccessKey context locals -// are set to defualut values in middlewares.SetDefaultValues +// are set to default values in middlewares.SetDefaultValues // to avoid the nil interface conversions type ContextKey string @@ -35,6 +35,7 @@ const ( ContextKeySkipResBodyLog ContextKey = "skip-res-body-log" ContextKeyBodyReader ContextKey = "body-reader" ContextKeySkip ContextKey = "__skip" + ContextKeyStack ContextKey = "stack" ) func (ck ContextKey) Values() []ContextKey {