Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 53 additions & 4 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,7 @@ type ClientOptions struct {
// instances. It must be created with NewClient.
type Client struct {
mu sync.RWMutex
disabled bool
options ClientOptions
dsn *protocol.Dsn
eventProcessors []EventProcessor
Expand Down Expand Up @@ -413,7 +414,7 @@ func NewClient(options ClientOptions) (*Client, error) {
var err error
dsn, err = protocol.NewDsn(options.Dsn)
if err != nil {
return nil, err
return NewNoopClient(), err
}
}

Expand Down Expand Up @@ -459,6 +460,39 @@ func NewClient(options ClientOptions) (*Client, error) {
return &client, nil
}

// NewNoopClient returns a non-nil client that safely discards telemetry.
func NewNoopClient() *Client {
return &Client{
disabled: true,
options: ClientOptions{
DisableLogs: true,
DisableMetrics: true,
DisableTelemetryBuffer: true,
MaxErrorDepth: maxErrorDepth,
MaxSpans: defaultMaxSpans,
TraceIgnoreStatusCodes: [][]int{{404}},
},
sdkIdentifier: sdkIdentifier,
sdkVersion: SDKVersion,
Transport: new(noopTransport),
reportRecorder: report.NoopRecorder(),
reportProvider: report.NoopProvider(),
}
}

// IsEnabled reports whether the client processes telemetry.
func (client *Client) IsEnabled() bool {
return client != nil && !client.disabled
}

// normalizeClient guarantees a non-nil client for internal and Hub callers.
func normalizeClient(client *Client) *Client {
if client == nil {
return NewNoopClient()
}
return client
}

func (client *Client) setupTransport() {
opts := client.options
transport := opts.Transport
Expand Down Expand Up @@ -605,7 +639,7 @@ func (client *Client) Options() ClientOptions {
// GetDataCollection returns a copy of the resolved data collection
// configuration used by the client.
func (client *Client) GetDataCollection() DataCollection {
if client == nil || client.options.DataCollection == nil {
if !client.IsEnabled() || client.options.DataCollection == nil {
return DataCollection{}
}
return *cloneDataCollection(client.options.DataCollection)
Expand Down Expand Up @@ -639,11 +673,14 @@ func (client *Client) CaptureCheckIn(checkIn *CheckIn, monitorConfig *MonitorCon
// the utility methods like CaptureException. The return value is the
// event ID. In case Sentry is disabled or event was dropped, the return value will be nil.
func (client *Client) CaptureEvent(event *Event, hint *EventHint, scope EventModifier) *EventID {
if !client.IsEnabled() {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check-in succeeds on disabled client

Medium Severity

CaptureCheckIn still builds a check-in and returns its ID after the hub nil-client guard was removed. CaptureEvent now drops the payload when the client is disabled, so callers get a non-nil ID even though nothing was captured. Cron flows that treat a returned ID as success will look healthy while monitors never receive the check-in.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 8fc2ecf. Configure here.

return nil
}
return client.processEvent(event, hint, scope)
}

func (client *Client) captureLog(log *Log, _ *Scope) bool {
if log == nil {
if !client.IsEnabled() || log == nil {
return false
}

Expand Down Expand Up @@ -677,7 +714,7 @@ func (client *Client) captureLog(log *Log, _ *Scope) bool {
}

func (client *Client) captureMetric(metric *Metric, _ *Scope) bool {
if metric == nil {
if !client.IsEnabled() || metric == nil {
return false
}

Expand Down Expand Up @@ -710,6 +747,9 @@ func (client *Client) captureMetric(metric *Metric, _ *Scope) bool {
// Recover captures a panic.
// Returns EventID if successfully, or nil if there's no error to recover from.
func (client *Client) Recover(err any, hint *EventHint, scope EventModifier) *EventID {
if !client.IsEnabled() {
return nil
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disabled Recover skips panic recovery

Medium Severity

Recover returns immediately when the client is disabled, before calling recover(). Direct use as a panic handler with a noop client therefore leaves the original panic unrecovered. RecoverWithContext does not have this early return, so the two APIs now behave inconsistently for disabled clients.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 8fc2ecf. Configure here.

if err == nil {
err = recover()
}
Expand Down Expand Up @@ -770,6 +810,9 @@ func (client *Client) RecoverWithContext(
// the network synchronously, configure it to use the HTTPSyncTransport in the
// call to Init.
func (client *Client) Flush(timeout time.Duration) bool {
if !client.IsEnabled() {
return true
}
if client.batchLogger != nil || client.batchMeter != nil || client.telemetryProcessor != nil {
ctx, cancel := context.WithTimeout(context.Background(), timeout)
defer cancel()
Expand All @@ -791,6 +834,9 @@ func (client *Client) Flush(timeout time.Duration) bool {
// configure the SDK to use HTTPSyncTransport during initialization with Init.

func (client *Client) FlushWithContext(ctx context.Context) bool {
if !client.IsEnabled() {
return true
}
if client.batchLogger != nil {
client.batchLogger.Flush(ctx.Done())
}
Expand All @@ -808,6 +854,9 @@ func (client *Client) FlushWithContext(ctx context.Context) bool {
// Close should be called after Flush and before terminating the program
// otherwise some events may be lost.
func (client *Client) Close() {
if !client.IsEnabled() {
return
}
if client.telemetryProcessor != nil {
client.telemetryProcessor.Close(5 * time.Second)
}
Expand Down
4 changes: 2 additions & 2 deletions dynamic_sampling_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func DynamicSamplingContextFromTransaction(span *Span) DynamicSamplingContext {
scope := hub.Scope()
client := hub.Client()

if client == nil || scope == nil {
if !client.IsEnabled() || scope == nil {
return DynamicSamplingContext{
Entries: map[string]string{},
Frozen: false,
Expand Down Expand Up @@ -122,7 +122,7 @@ func (d DynamicSamplingContext) String() string {
func DynamicSamplingContextFromScope(scope *Scope, client *Client) DynamicSamplingContext {
entries := map[string]string{}

if client == nil || scope == nil {
if !client.IsEnabled() || scope == nil {
return DynamicSamplingContext{
Entries: entries,
Frozen: false,
Expand Down
2 changes: 1 addition & 1 deletion echo/sentryecho.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func (h *handler) handle(next echo.HandlerFunc) echo.HandlerFunc {
hub = sentry.CurrentHub().Clone()
}

if client := hub.Client(); client != nil {
if client := hub.Client(); client.IsEnabled() {
client.SetSDKIdentifier(sdkIdentifier)
}

Expand Down
2 changes: 1 addition & 1 deletion fasthttp/sentryfasthttp.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func (h *Handler) Handle(handler fasthttp.RequestHandler) fasthttp.RequestHandle
hub = sentry.CurrentHub().Clone()
}

if client := hub.Client(); client != nil {
if client := hub.Client(); client.IsEnabled() {
client.SetSDKIdentifier(sdkIdentifier)
}

Expand Down
2 changes: 1 addition & 1 deletion fiber/sentryfiber.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func (h *handler) handle(ctx *fiber.Ctx) error {
hub = sentry.CurrentHub().Clone()
}

if client := hub.Client(); client != nil {
if client := hub.Client(); client.IsEnabled() {
client.SetSDKIdentifier(sdkIdentifier)
}

Expand Down
2 changes: 1 addition & 1 deletion fiberv3/sentryfiber.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func (h *handler) handle(ctx fiber.Ctx) error {
hub = sentry.CurrentHub().Clone()
}

if client := hub.Client(); client != nil {
if client := hub.Client(); client.IsEnabled() {
client.SetSDKIdentifier(sdkIdentifier)
}

Expand Down
2 changes: 1 addition & 1 deletion gin/sentrygin.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func (h *handler) handle(c *gin.Context) {
hub = sentry.CurrentHub().Clone()
}

if client := hub.Client(); client != nil {
if client := hub.Client(); client.IsEnabled() {
client.SetSDKIdentifier(sdkIdentifier)
}

Expand Down
2 changes: 1 addition & 1 deletion grpc/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func hubFromClientContext(ctx context.Context) context.Context {
ctx = sentry.SetHubOnContext(ctx, hub)
}

if client := hub.Client(); client != nil {
if client := hub.Client(); client.IsEnabled() {
client.SetSDKIdentifier(sdkIdentifier)
}

Expand Down
2 changes: 1 addition & 1 deletion grpc/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func hubFromServerContext(ctx context.Context) *sentry.Hub {
hub = sentry.CurrentHub().Clone()
}

if client := hub.Client(); client != nil {
if client := hub.Client(); client.IsEnabled() {
client.SetSDKIdentifier(sdkIdentifier)
}

Expand Down
2 changes: 1 addition & 1 deletion http/sentryhttp.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func (h *Handler) handle(handler http.Handler) http.HandlerFunc {
ctx = sentry.SetHubOnContext(ctx, hub)
}

if client := hub.Client(); client != nil {
if client := hub.Client(); client.IsEnabled() {
client.SetSDKIdentifier(sdkIdentifier)
}

Expand Down
6 changes: 3 additions & 3 deletions httpclient/sentryhttpclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func NewSentryRoundTripper(originalRoundTripper http.RoundTripper, opts ...Sentr
var propagateTraceparent bool
if hub := sentry.CurrentHub(); hub != nil {
client := hub.Client()
if client != nil {
if client.IsEnabled() {
clientOptions := client.Options()
if clientOptions.TracePropagationTargets != nil {
tracePropagationTargets = clientOptions.TracePropagationTargets
Expand Down Expand Up @@ -83,12 +83,12 @@ type SentryRoundTripper struct {

func dataCollectionFromRequest(request *http.Request) sentry.DataCollection {
if hub := sentry.GetHubFromContext(request.Context()); hub != nil {
if client := hub.Client(); client != nil {
if client := hub.Client(); client.IsEnabled() {
return client.GetDataCollection()
}
}
if hub := sentry.CurrentHub(); hub != nil {
if client := hub.Client(); client != nil {
if client := hub.Client(); client.IsEnabled() {
return client.GetDataCollection()
}
}
Expand Down
44 changes: 11 additions & 33 deletions hub.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ const (
)

// currentHub is the initial Hub with no Client bound and an empty Scope.
var currentHub = NewHub(nil, NewScope())
var currentHub = NewHub(NewNoopClient(), NewScope())

// Hub is the central object that manages scopes and clients.
//
Expand Down Expand Up @@ -51,14 +51,14 @@ type layer struct {
func (l *layer) Client() *Client {
l.mu.RLock()
defer l.mu.RUnlock()
return l.client
return normalizeClient(l.client)
}

// SetClient sets the layer's client. Safe for concurrent use.
func (l *layer) SetClient(c *Client) {
l.mu.Lock()
defer l.mu.Unlock()
l.client = c
l.client = normalizeClient(c)
}

type stack []*layer
Expand All @@ -67,7 +67,7 @@ type stack []*layer
func NewHub(client *Client, scope *Scope) *Hub {
hub := Hub{
stack: &stack{{
client: client,
client: normalizeClient(client),
scope: scope,
}},
}
Expand Down Expand Up @@ -218,7 +218,7 @@ func (hub *Hub) CaptureEvent(event *Event) *EventID {
// CaptureEventWithHint is like CaptureEvent but additionally accepts an EventHint.
func (hub *Hub) CaptureEventWithHint(event *Event, hint *EventHint) *EventID {
client, scope := hub.Client(), hub.Scope()
if client == nil || scope == nil {
if scope == nil {
return nil
}
eventID := client.CaptureEvent(event, hint, scope)
Expand All @@ -236,7 +236,7 @@ func (hub *Hub) CaptureEventWithHint(event *Event, hint *EventHint) *EventID {
// Returns EventID if successfully, or nil if there's no Scope or Client available.
func (hub *Hub) CaptureMessage(message string) *EventID {
client, scope := hub.Client(), hub.Scope()
if client == nil || scope == nil {
if scope == nil {
return nil
}
eventID := client.CaptureMessage(message, nil, scope)
Expand All @@ -254,7 +254,7 @@ func (hub *Hub) CaptureMessage(message string) *EventID {
// Returns EventID if successfully, or nil if there's no Scope or Client available.
func (hub *Hub) CaptureException(exception error) *EventID {
client, scope := hub.Client(), hub.Scope()
if client == nil || scope == nil {
if scope == nil {
return nil
}
eventID := client.CaptureException(exception, &EventHint{OriginalException: exception}, scope)
Expand All @@ -272,10 +272,6 @@ func (hub *Hub) CaptureException(exception error) *EventID {
// Returns CheckInID if the check-in was captured successfully, or nil otherwise.
func (hub *Hub) CaptureCheckIn(checkIn *CheckIn, monitorConfig *MonitorConfig) *EventID {
client, scope := hub.Client(), hub.Scope()
if client == nil {
return nil

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check-in returns ID when disabled

Medium Severity

CaptureCheckIn no longer returns nil when no real client is bound. The hub-level client == nil guard was removed, but Client.CaptureCheckIn never checks IsEnabled() and still returns a check-in ID after CaptureEvent discards the event. Callers that treat a non-nil ID as a successful capture will think the check-in was recorded when the noop client dropped it.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 7ecc977. Configure here.

}

return client.CaptureCheckIn(checkIn, monitorConfig, scope)
}

Expand All @@ -286,12 +282,6 @@ func (hub *Hub) CaptureCheckIn(checkIn *CheckIn, monitorConfig *MonitorConfig) *
func (hub *Hub) AddBreadcrumb(breadcrumb *Breadcrumb, hint *BreadcrumbHint) {
client := hub.Client()

// If there's no client, just store it on the scope straight away
if client == nil {
hub.Scope().AddBreadcrumb(breadcrumb, defaultMaxBreadcrumbs)
return
}

limit := client.options.MaxBreadcrumbs
switch {
case limit < 0:
Expand Down Expand Up @@ -321,7 +311,7 @@ func (hub *Hub) Recover(err interface{}) *EventID {
err = recover()
}
client, scope := hub.Client(), hub.Scope()
if client == nil || scope == nil {
if scope == nil {
return nil
}
return client.Recover(err, &EventHint{RecoveredException: err}, scope)
Expand All @@ -335,7 +325,7 @@ func (hub *Hub) RecoverWithContext(ctx context.Context, err interface{}) *EventI
err = recover()
}
client, scope := hub.Client(), hub.Scope()
if client == nil || scope == nil {
if scope == nil {
return nil
}
return client.RecoverWithContext(ctx, err, &EventHint{RecoveredException: err}, scope)
Expand All @@ -353,13 +343,7 @@ func (hub *Hub) RecoverWithContext(ctx context.Context, err interface{}) *EventI
// the network synchronously, configure it to use the HTTPSyncTransport in the
// call to Init.
func (hub *Hub) Flush(timeout time.Duration) bool {
client := hub.Client()

if client == nil {
return false
}

return client.Flush(timeout)
return hub.Client().Flush(timeout)
}

// FlushWithContext waits until the underlying Transport sends any buffered events
Expand All @@ -375,13 +359,7 @@ func (hub *Hub) Flush(timeout time.Duration) bool {
// configure the SDK to use HTTPSyncTransport during initialization with Init.

func (hub *Hub) FlushWithContext(ctx context.Context) bool {
client := hub.Client()

if client == nil {
return false
}

return client.FlushWithContext(ctx)
return hub.Client().FlushWithContext(ctx)
}

// GetTraceparent returns the current Sentry traceparent string, to be used as a HTTP header value
Expand Down
Loading
Loading