Parcourir la source

errorreport.Reporter instance

DarthSim il y a 5 jours
Parent
commit
10edac91ae

+ 7 - 0
config.go

@@ -4,6 +4,7 @@ import (
 	"github.com/imgproxy/imgproxy/v3/auximageprovider"
 	"github.com/imgproxy/imgproxy/v3/cookies"
 	"github.com/imgproxy/imgproxy/v3/ensure"
+	"github.com/imgproxy/imgproxy/v3/errorreport"
 	"github.com/imgproxy/imgproxy/v3/fetcher"
 	processinghandler "github.com/imgproxy/imgproxy/v3/handlers/processing"
 	streamhandler "github.com/imgproxy/imgproxy/v3/handlers/stream"
@@ -35,6 +36,7 @@ type Config struct {
 	OptionsParser  optionsparser.Config
 	Cookies        cookies.Config
 	Monitoring     monitoring.Config
+	ErrorReport    errorreport.Config
 }
 
 // NewDefaultConfig creates a new default configuration
@@ -54,6 +56,7 @@ func NewDefaultConfig() Config {
 		OptionsParser: optionsparser.NewDefaultConfig(),
 		Cookies:       cookies.NewDefaultConfig(),
 		Monitoring:    monitoring.NewDefaultConfig(),
+		ErrorReport:   errorreport.NewDefaultConfig(),
 	}
 }
 
@@ -111,6 +114,10 @@ func LoadConfigFromEnv(c *Config) (*Config, error) {
 		return nil, err
 	}
 
+	if _, err = errorreport.LoadConfigFromEnv(&c.ErrorReport); err != nil {
+		return nil, err
+	}
+
 	return c, nil
 }
 

+ 13 - 11
errorreport/bugsnag/bugsnag.go

@@ -11,7 +11,13 @@ import (
 // logger is the logger forwarder for bugsnag
 type logger struct{}
 
-type reporter struct{}
+func (l logger) Printf(format string, v ...interface{}) {
+	slog.Debug(fmt.Sprintf(format, v...), "source", "bugsnag")
+}
+
+type reporter struct {
+	notifier *bugsnag.Notifier
+}
 
 func New(config *Config) (*reporter, error) {
 	if err := config.Validate(); err != nil {
@@ -22,21 +28,15 @@ func New(config *Config) (*reporter, error) {
 		return nil, nil
 	}
 
-	bugsnag.Configure(bugsnag.Configuration{
+	notifier := bugsnag.New(bugsnag.Configuration{
 		APIKey:       config.Key,
 		ReleaseStage: config.Stage,
 		PanicHandler: func() {}, // Disable forking the process
 		Logger:       logger{},
+		Synchronous:  true,
 	})
 
-	return &reporter{}, nil
-}
-
-func (l logger) Printf(format string, v ...interface{}) {
-	slog.Debug(
-		fmt.Sprintf(format, v...),
-		"source", "bugsnag",
-	)
+	return &reporter{notifier: notifier}, nil
 }
 
 func (r *reporter) Report(err error, req *http.Request, meta map[string]any) {
@@ -45,7 +45,9 @@ func (r *reporter) Report(err error, req *http.Request, meta map[string]any) {
 		extra.Add("Processing Context", k, v)
 	}
 
-	bugsnag.Notify(err, req, extra)
+	if repErr := r.notifier.Notify(err, req, extra); repErr != nil {
+		slog.Warn("Failed to report error to Bugsnag", "error", repErr)
+	}
 }
 
 func (r *reporter) Close() {

+ 18 - 14
errorreport/errorreport.go

@@ -20,42 +20,46 @@ type reporter interface {
 // metaCtxKey is the context.Context key for request metadata
 type metaCtxKey struct{}
 
-// initialized reporters
-var reporters []reporter
+type Reporter struct {
+	// initialized reporters
+	reporters []reporter
+}
 
 // Init initializes all configured error reporters and returns a Reporter instance.
-func Init(config *Config) error {
+func New(config *Config) (*Reporter, error) {
 	if err := config.Validate(); err != nil {
-		return err
+		return nil, err
 	}
 
-	reporters = make([]reporter, 0)
+	reporters := make([]reporter, 0)
 
 	if r, err := bugsnag.New(&config.Bugsnag); err != nil {
-		return err
+		return nil, err
 	} else if r != nil {
 		reporters = append(reporters, r)
 	}
 
 	if r, err := honeybadger.New(&config.Honeybadger); err != nil {
-		return err
+		return nil, err
 	} else if r != nil {
 		reporters = append(reporters, r)
 	}
 
 	if r, err := sentry.New(&config.Sentry); err != nil {
-		return err
+		return nil, err
 	} else if r != nil {
 		reporters = append(reporters, r)
 	}
 
 	if r, err := airbrake.New(&config.Airbrake); err != nil {
-		return err
+		return nil, err
 	} else if r != nil {
 		reporters = append(reporters, r)
 	}
 
-	return nil
+	return &Reporter{
+		reporters: reporters,
+	}, nil
 }
 
 // StartRequest initializes metadata storage in the request context.
@@ -75,20 +79,20 @@ func SetMetadata(req *http.Request, key string, value any) {
 }
 
 // Report reports an error to all configured reporters with the request and its metadata.
-func Report(err error, req *http.Request) {
+func (r *Reporter) Report(err error, req *http.Request) {
 	meta, ok := req.Context().Value(metaCtxKey{}).(map[string]any)
 	if !ok {
 		meta = nil
 	}
 
-	for _, reporter := range reporters {
+	for _, reporter := range r.reporters {
 		reporter.Report(err, req, meta)
 	}
 }
 
 // Close closes all reporters
-func Close() {
-	for _, reporter := range reporters {
+func (r *Reporter) Close() {
+	for _, reporter := range r.reporters {
 		reporter.Close()
 	}
 }

+ 10 - 5
errorreport/honeybadger/honeybadger.go

@@ -1,6 +1,7 @@
 package honeybadger
 
 import (
+	"log/slog"
 	"net/http"
 	"reflect"
 	"strings"
@@ -14,7 +15,9 @@ var (
 	metaReplacer = strings.NewReplacer("-", "_", " ", "_")
 )
 
-type reporter struct{}
+type reporter struct {
+	client *honeybadger.Client
+}
 
 func New(config *Config) (*reporter, error) {
 	if err := config.Validate(); err != nil {
@@ -25,12 +28,12 @@ func New(config *Config) (*reporter, error) {
 		return nil, nil
 	}
 
-	honeybadger.Configure(honeybadger.Configuration{
+	client := honeybadger.New(honeybadger.Configuration{
 		APIKey: config.Key,
 		Env:    config.Env,
 	})
 
-	return &reporter{}, nil
+	return &reporter{client: client}, nil
 }
 
 func (r *reporter) Report(err error, req *http.Request, meta map[string]any) {
@@ -52,9 +55,11 @@ func (r *reporter) Report(err error, req *http.Request, meta map[string]any) {
 		hbErr.Class = reflect.TypeOf(e.Unwrap()).String()
 	}
 
-	honeybadger.Notify(hbErr, req.URL, extra)
+	if _, repErr := r.client.Notify(hbErr, req.URL, extra); repErr != nil {
+		slog.Warn("Failed to report error to Honeybadger", "error", repErr)
+	}
 }
 
 func (r *reporter) Close() {
-	// noop
+	r.client.Flush()
 }

+ 13 - 9
errorreport/sentry/sentry.go

@@ -13,7 +13,9 @@ const (
 )
 
 // reporter is a Sentry error reporter
-type reporter struct{}
+type reporter struct {
+	hub *sentry.Hub
+}
 
 // New creates and configures a new Sentry reporter
 func New(config *Config) (*reporter, error) {
@@ -25,17 +27,22 @@ func New(config *Config) (*reporter, error) {
 		return nil, nil
 	}
 
-	sentry.Init(sentry.ClientOptions{
+	client, err := sentry.NewClient(sentry.ClientOptions{
 		Dsn:         config.DSN,
 		Release:     config.Release,
 		Environment: config.Environment,
 	})
+	if err != nil {
+		return nil, err
+	}
 
-	return &reporter{}, nil
+	hub := sentry.NewHub(client, sentry.NewScope())
+
+	return &reporter{hub: hub}, nil
 }
 
 func (r *reporter) Report(err error, req *http.Request, meta map[string]any) {
-	hub := sentry.CurrentHub().Clone()
+	hub := r.hub.Clone()
 	hub.Scope().SetRequest(req)
 	hub.Scope().SetLevel(sentry.LevelError)
 
@@ -56,13 +63,10 @@ func (r *reporter) Report(err error, req *http.Request, meta map[string]any) {
 			}
 		}
 
-		eventID := hub.CaptureEvent(event)
-		if eventID != nil {
-			hub.Flush(flushTimeout)
-		}
+		hub.CaptureEvent(event)
 	}
 }
 
 func (r *reporter) Close() {
-	sentry.Flush(flushTimeout)
+	r.hub.Flush(flushTimeout)
 }

+ 1 - 0
handlers/processing/handler.go

@@ -32,6 +32,7 @@ type HandlerContext interface {
 	Processor() *processing.Processor
 	Cookies() *cookies.Cookies
 	Monitoring() *monitoring.Monitoring
+	ErrorReporter() *errorreport.Reporter
 }
 
 // Handler handles image processing requests

+ 2 - 3
handlers/processing/request_methods.go

@@ -7,7 +7,6 @@ import (
 	"net/http"
 	"strconv"
 
-	"github.com/imgproxy/imgproxy/v3/errorreport"
 	"github.com/imgproxy/imgproxy/v3/handlers"
 	"github.com/imgproxy/imgproxy/v3/httpheaders"
 	"github.com/imgproxy/imgproxy/v3/ierrors"
@@ -100,7 +99,7 @@ func (r *request) handleDownloadError(
 
 	// We didn't return, so we have to report error
 	if err.ShouldReport() {
-		errorreport.Report(err, r.req)
+		r.ErrorReporter().Report(err, r.req)
 	}
 
 	slog.Warn(
@@ -142,7 +141,7 @@ func (r *request) getFallbackImage(ctx context.Context) (imagedata.ImageData, ht
 		slog.Warn(err.Error())
 
 		if ierr := r.wrapDownloadingErr(err); ierr.ShouldReport() {
-			errorreport.Report(ierr, r.req)
+			r.ErrorReporter().Report(ierr, r.req)
 		}
 
 		return nil, nil

+ 14 - 1
imgproxy.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/imgproxy/imgproxy/v3/auximageprovider"
 	"github.com/imgproxy/imgproxy/v3/cookies"
+	"github.com/imgproxy/imgproxy/v3/errorreport"
 	"github.com/imgproxy/imgproxy/v3/fetcher"
 	healthhandler "github.com/imgproxy/imgproxy/v3/handlers/health"
 	landinghandler "github.com/imgproxy/imgproxy/v3/handlers/landing"
@@ -49,6 +50,7 @@ type Imgproxy struct {
 	cookies          *cookies.Cookies
 	monitoring       *monitoring.Monitoring
 	config           *Config
+	errorReporter    *errorreport.Reporter
 }
 
 // New creates a new imgproxy instance
@@ -104,6 +106,11 @@ func New(ctx context.Context, config *Config) (*Imgproxy, error) {
 		return nil, err
 	}
 
+	errorReporter, err := errorreport.New(&config.ErrorReport)
+	if err != nil {
+		return nil, err
+	}
+
 	imgproxy := &Imgproxy{
 		workers:          workers,
 		fallbackImage:    fallbackImage,
@@ -116,6 +123,7 @@ func New(ctx context.Context, config *Config) (*Imgproxy, error) {
 		processor:        processor,
 		cookies:          cookies,
 		monitoring:       monitoring,
+		errorReporter:    errorReporter,
 	}
 
 	imgproxy.handlers.Health = healthhandler.New()
@@ -138,7 +146,7 @@ func New(ctx context.Context, config *Config) (*Imgproxy, error) {
 
 // BuildRouter sets up the HTTP routes and middleware
 func (i *Imgproxy) BuildRouter() (*server.Router, error) {
-	r, err := server.NewRouter(&i.config.Server, i.monitoring)
+	r, err := server.NewRouter(&i.config.Server, i.monitoring, i.errorReporter)
 	if err != nil {
 		return nil, err
 	}
@@ -199,6 +207,7 @@ func (i *Imgproxy) StartServer(ctx context.Context, hasStarted chan net.Addr) er
 // Close gracefully shuts down the imgproxy instance
 func (i *Imgproxy) Close(ctx context.Context) {
 	i.monitoring.Stop(ctx)
+	i.errorReporter.Close()
 }
 
 // startMemoryTicker starts a ticker that periodically frees memory and optionally logs memory stats
@@ -259,3 +268,7 @@ func (i *Imgproxy) Cookies() *cookies.Cookies {
 func (i *Imgproxy) Monitoring() *monitoring.Monitoring {
 	return i.monitoring
 }
+
+func (i *Imgproxy) ErrorReporter() *errorreport.Reporter {
+	return i.errorReporter
+}

+ 0 - 11
init.go

@@ -11,7 +11,6 @@ import (
 
 	"github.com/imgproxy/imgproxy/v3/config"
 	"github.com/imgproxy/imgproxy/v3/env"
-	"github.com/imgproxy/imgproxy/v3/errorreport"
 	"github.com/imgproxy/imgproxy/v3/logger"
 	"github.com/imgproxy/imgproxy/v3/vips"
 )
@@ -53,20 +52,10 @@ func Init() error {
 		return vipsErr
 	}
 
-	errCfg, errErr := errorreport.LoadConfigFromEnv(nil)
-	if errErr != nil {
-		return errErr
-	}
-
-	if err := errorreport.Init(errCfg); err != nil {
-		return err
-	}
-
 	return nil
 }
 
 // Shutdown performs global cleanup
 func Shutdown() {
 	vips.Shutdown()
-	errorreport.Close()
 }

+ 1 - 1
server/middlewares.go

@@ -126,7 +126,7 @@ func (r *Router) WithReportError(h RouteHandler) RouteHandler {
 
 		// Report error to error collectors
 		if ierr.ShouldReport() {
-			errorreport.Report(ierr, req)
+			r.errorReporter.Report(ierr, req)
 		}
 
 		// Log response and format the error output

+ 13 - 4
server/router.go

@@ -10,6 +10,7 @@ import (
 
 	nanoid "github.com/matoous/go-nanoid/v2"
 
+	"github.com/imgproxy/imgproxy/v3/errorreport"
 	"github.com/imgproxy/imgproxy/v3/httpheaders"
 	"github.com/imgproxy/imgproxy/v3/monitoring"
 	"github.com/imgproxy/imgproxy/v3/server/responsewriter"
@@ -55,10 +56,17 @@ type Router struct {
 
 	// monitoring is the monitoring instance
 	monitoring *monitoring.Monitoring
+
+	// errorReporter is the error reporter
+	errorReporter *errorreport.Reporter
 }
 
 // NewRouter creates a new Router instance
-func NewRouter(config *Config, monitoring *monitoring.Monitoring) (*Router, error) {
+func NewRouter(
+	config *Config,
+	monitoring *monitoring.Monitoring,
+	errReporter *errorreport.Reporter,
+) (*Router, error) {
 	if err := config.Validate(); err != nil {
 		return nil, err
 	}
@@ -69,9 +77,10 @@ func NewRouter(config *Config, monitoring *monitoring.Monitoring) (*Router, erro
 	}
 
 	return &Router{
-		rwFactory:  rwf,
-		config:     config,
-		monitoring: monitoring,
+		rwFactory:     rwf,
+		config:        config,
+		monitoring:    monitoring,
+		errorReporter: errReporter,
 	}, nil
 }
 

+ 6 - 1
server/router_test.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/stretchr/testify/suite"
 
+	"github.com/imgproxy/imgproxy/v3/errorreport"
 	"github.com/imgproxy/imgproxy/v3/httpheaders"
 	"github.com/imgproxy/imgproxy/v3/monitoring"
 )
@@ -23,8 +24,12 @@ func (s *RouterTestSuite) SetupTest() {
 	m, err := monitoring.New(s.T().Context(), &mc, 1)
 	s.Require().NoError(err)
 
+	erCfg := errorreport.NewDefaultConfig()
+	er, err := errorreport.New(&erCfg)
+	s.Require().NoError(err)
+
 	c.PathPrefix = "/api"
-	r, err := NewRouter(&c, m)
+	r, err := NewRouter(&c, m, er)
 	s.Require().NoError(err)
 
 	s.router = r

+ 14 - 7
server/server_test.go

@@ -9,6 +9,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/imgproxy/imgproxy/v3/errorreport"
 	"github.com/imgproxy/imgproxy/v3/httpheaders"
 	"github.com/imgproxy/imgproxy/v3/monitoring"
 	"github.com/stretchr/testify/suite"
@@ -16,9 +17,10 @@ import (
 
 type ServerTestSuite struct {
 	suite.Suite
-	config      *Config
-	monitoring  *monitoring.Monitoring
-	blankRouter *Router
+	config        *Config
+	monitoring    *monitoring.Monitoring
+	errorReporter *errorreport.Reporter
+	blankRouter   *Router
 }
 
 func (s *ServerTestSuite) SetupTest() {
@@ -32,7 +34,12 @@ func (s *ServerTestSuite) SetupTest() {
 	s.Require().NoError(err)
 	s.monitoring = m
 
-	r, err := NewRouter(s.config, m)
+	erCfg := errorreport.NewDefaultConfig()
+	er, err := errorreport.New(&erCfg)
+	s.Require().NoError(err)
+	s.errorReporter = er
+
+	r, err := NewRouter(s.config, m, er)
 	s.Require().NoError(err)
 	s.blankRouter = r
 }
@@ -58,7 +65,7 @@ func (s *ServerTestSuite) TestStartServerWithInvalidBind() {
 	invalidConfig := NewDefaultConfig()
 	invalidConfig.Bind = "-1.-1.-1.-1" // Invalid address
 
-	r, err := NewRouter(&invalidConfig, s.monitoring)
+	r, err := NewRouter(&invalidConfig, s.monitoring, s.errorReporter)
 	s.Require().NoError(err)
 
 	server, err := Start(cancelWrapper, r)
@@ -125,7 +132,7 @@ func (s *ServerTestSuite) TestWithCORS() {
 			config := NewDefaultConfig()
 			config.CORSAllowOrigin = tt.corsAllowOrigin
 
-			router, err := NewRouter(&config, s.monitoring)
+			router, err := NewRouter(&config, s.monitoring, s.errorReporter)
 			s.Require().NoError(err)
 
 			wrappedHandler := router.WithCORS(s.mockHandler)
@@ -171,7 +178,7 @@ func (s *ServerTestSuite) TestWithSecret() {
 			config := NewDefaultConfig()
 			config.Secret = tt.secret
 
-			router, err := NewRouter(&config, s.monitoring)
+			router, err := NewRouter(&config, s.monitoring, s.errorReporter)
 			s.Require().NoError(err)
 
 			wrappedHandler := router.WithSecret(s.mockHandler)