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
119 changes: 119 additions & 0 deletions capture_options.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
package sentry

import "context"

// CaptureOption configures a single event capture.
type CaptureOption interface {
applyCaptureOption(*captureOptions)
}

type captureOptionFunc func(*captureOptions)

func (f captureOptionFunc) applyCaptureOption(options *captureOptions) { f(options) }

type captureOptions struct {
hint *EventHint
modifiers []EventModifier
legacyScope EventModifier
legacyScopeSet bool
}

// WithEventHint supplies metadata to event processors and before-send hooks.
// When supplied more than once, the last hint wins.
func WithEventHint(hint *EventHint) CaptureOption {
return captureOptionFunc(func(options *captureOptions) { options.hint = hint })
}

// WithEventModifier applies modifier to the captured event. It may be repeated.
func WithEventModifier(modifier EventModifier) CaptureOption {
return captureOptionFunc(func(options *captureOptions) {
if modifier != nil {
options.modifiers = append(options.modifiers, modifier)
}
})
}

// withLegacyScope preserves backward-compatible Hub scope processing and should be removed with the Hub.
func withLegacyScope(scope EventModifier) CaptureOption {
return captureOptionFunc(func(options *captureOptions) {
options.legacyScope = scope
options.legacyScopeSet = true
})
}

// withDefaultOriginalException preserves a caller-supplied hint value while filling the capture default.
func withDefaultOriginalException(exception error) CaptureOption {
return captureOptionFunc(func(options *captureOptions) {
if options.hint == nil {
options.hint = &EventHint{}
}
if options.hint.OriginalException == nil {
hint := *options.hint
hint.OriginalException = exception
options.hint = &hint
}
})
}

// withDefaultRecoveredException preserves a caller-supplied hint value while filling the capture default.
func withDefaultRecoveredException(recovered any) CaptureOption {
return captureOptionFunc(func(options *captureOptions) {
if options.hint == nil {
options.hint = &EventHint{}
}
if options.hint.RecoveredException == nil {
hint := *options.hint
hint.RecoveredException = recovered
options.hint = &hint
}
})
}

func resolveCaptureOptions(ctx context.Context, client Client, options []CaptureOption) (*EventHint, EventModifier, *Scope) {
var resolved captureOptions
for _, option := range options {
if option != nil {
option.applyCaptureOption(&resolved)
}
}
hint := &EventHint{}
if resolved.hint != nil {
*hint = *resolved.hint
}
// Explicit capture context overrides any supplied hint.Context. A nil ctx
// leaves any supplied hint context in place so legacy Hub APIs and scope
// request-context fallback continue to work.
if ctx != nil {
hint.Context = ctx
}
if resolved.legacyScopeSet {
var modifiers []EventModifier
if resolved.legacyScope != nil {
modifiers = append(modifiers, resolved.legacyScope)
}
modifiers = append(modifiers, resolved.modifiers...)
return hint, captureModifierChain(modifiers), nil
}
global := GlobalScope()
isolation := scopeFromContext(ctx)
owner := isolation
if owner == nil {
owner = global
}
modifiers := append([]EventModifier{snapshotScopes(client, global, isolation)}, resolved.modifiers...)
return hint, captureModifierChain(modifiers), owner
}

type captureModifierChain []EventModifier

func (modifiers captureModifierChain) ApplyToEvent(event *Event, hint *EventHint, client Client) *Event {
for _, modifier := range modifiers {
if modifier != nil {
event = modifier.ApplyToEvent(event, hint, client)
}
if event == nil {
return nil
}
}
return event
}
135 changes: 135 additions & 0 deletions capture_options_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
package sentry

import (
"context"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestWithEventHint(t *testing.T) {
t.Parallel()

type contextKey struct{}
captureCtx := context.WithValue(context.Background(), contextKey{}, "capture")

tests := []struct {
name string
options []CaptureOption
want any
}{
{
name: "passes hint to capture pipeline",
options: []CaptureOption{WithEventHint(&EventHint{Data: "hint"})},
want: "hint",
},
{
name: "last hint wins",
options: []CaptureOption{
WithEventHint(&EventHint{Data: "first"}),
WithEventHint(&EventHint{Data: "second"}),
},
want: "second",
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
t.Parallel()

var got *EventHint
client, err := newClient(ClientOptions{
Transport: &MockTransport{},
Integrations: func([]Integration) []Integration {
return nil
},
BeforeSend: func(event *Event, hint *EventHint) *Event {
got = hint
return event
},
})
require.NoError(t, err)

id := client.CaptureMessage(captureCtx, "message", test.options...)
require.NotNil(t, id)
require.NotNil(t, got)
assert.Equal(t, test.want, got.Data)
assert.Equal(t, captureCtx, got.Context)
})
}
}

func TestWithEventModifier(t *testing.T) {
t.Parallel()

tests := []struct {
name string
modifiers []EventModifier
wantID bool
wantMessage string
}{
{
name: "applies modifiers in order",
modifiers: []EventModifier{
testEventModifier(func(event *Event, _ *EventHint, _ Client) *Event {
event.Message += "-first"
return event
}),
nil,
testEventModifier(func(event *Event, _ *EventHint, _ Client) *Event {
event.Message += "-second"
return event
}),
},
wantID: true,
wantMessage: "message-first-second",
},
{
name: "nil event drops capture and stops the chain",
modifiers: []EventModifier{
testEventModifier(func(_ *Event, _ *EventHint, _ Client) *Event { return nil }),
testEventModifier(func(event *Event, _ *EventHint, _ Client) *Event {
event.Message = "not reached"
return event
}),
},
wantID: false,
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
t.Parallel()

transport := &MockTransport{}
client, err := newClient(ClientOptions{
Transport: transport,
Integrations: func([]Integration) []Integration {
return nil
},
})
require.NoError(t, err)

options := make([]CaptureOption, 0, len(test.modifiers))
for _, modifier := range test.modifiers {
options = append(options, WithEventModifier(modifier))
}
id := client.CaptureMessage(context.Background(), "message", options...)
if !test.wantID {
assert.Nil(t, id)
assert.Nil(t, transport.lastEvent)
return
}
require.NotNil(t, id)
require.NotNil(t, transport.lastEvent)
assert.Equal(t, test.wantMessage, transport.lastEvent.Message)
})
}
}

type testEventModifier func(*Event, *EventHint, Client) *Event

func (f testEventModifier) ApplyToEvent(event *Event, hint *EventHint, client Client) *Event {
return f(event, hint, client)
}
78 changes: 25 additions & 53 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -613,34 +613,36 @@ func (client *defaultClient) GetDataCollection() DataCollection {
}

// CaptureMessage captures an arbitrary message.
func (client *defaultClient) CaptureMessage(message string, hint *EventHint, scope EventModifier) *EventID {
event := client.EventFromMessage(message, LevelInfo)
return client.CaptureEvent(event, hint, scope)
func (client *defaultClient) CaptureMessage(ctx context.Context, message string, options ...CaptureOption) *EventID {
return client.CaptureEvent(ctx, client.EventFromMessage(message, LevelInfo), options...)
}

// CaptureException captures an error.
func (client *defaultClient) CaptureException(exception error, hint *EventHint, scope EventModifier) *EventID {
event := client.EventFromException(exception, LevelError)
return client.CaptureEvent(event, hint, scope)
func (client *defaultClient) CaptureException(ctx context.Context, exception error, options ...CaptureOption) *EventID {
return client.CaptureEvent(ctx, client.EventFromException(exception, LevelError), append(options, withDefaultOriginalException(exception))...)
}

// CaptureCheckIn captures a check in.
func (client *defaultClient) CaptureCheckIn(checkIn *CheckIn, monitorConfig *MonitorConfig, scope EventModifier) *EventID {
func (client *defaultClient) CaptureCheckIn(ctx context.Context, checkIn *CheckIn, monitorConfig *MonitorConfig, options ...CaptureOption) *EventID {
event := client.EventFromCheckIn(checkIn, monitorConfig)
if event != nil && event.CheckIn != nil {
client.CaptureEvent(event, nil, scope)
return &event.CheckIn.ID
if event == nil || event.CheckIn == nil {
return nil
}
id := event.CheckIn.ID
if client.CaptureEvent(ctx, event, options...) == nil {
return nil
}
return nil
return &id
}

// CaptureEvent captures an event on the currently active client if any.
//
// The event must already be assembled. Typically, code would instead use
// 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 *defaultClient) CaptureEvent(event *Event, hint *EventHint, scope EventModifier) *EventID {
return client.processEvent(event, hint, scope)
// CaptureEvent captures an event on this client.
func (client *defaultClient) CaptureEvent(ctx context.Context, event *Event, options ...CaptureOption) *EventID {
hint, scope, owner := resolveCaptureOptions(ctx, client, options)
id := client.processEvent(event, hint, scope)
if id != nil && event != nil && event.Type != transactionType && owner != nil {
owner.setLastEventID(*id)
}
Comment thread
cursor[bot] marked this conversation as resolved.
return id
}

func (client *defaultClient) captureLog(log *Log, _ *Scope) bool {
Expand Down Expand Up @@ -716,45 +718,15 @@ func (client *defaultClient) captureMetric(metric *Metric, _ *Scope) bool {
return true
}

// Recover captures a panic.
// Returns EventID if successfully, or nil if there's no error to recover from.
func (client *defaultClient) Recover(err any, hint *EventHint, scope EventModifier) *EventID {
if err == nil {
err = recover()
}

// Normally we would not pass a nil Context, but RecoverWithContext doesn't
// use the Context for communicating deadline nor cancelation. All it does
// is store the Context in the EventHint and there nil means the Context is
// not available.
// nolint: staticcheck
return client.RecoverWithContext(nil, err, hint, scope)
}

// RecoverWithContext captures a panic and passes relevant context object.
// Returns EventID if successfully, or nil if there's no error to recover from.
func (client *defaultClient) RecoverWithContext(
ctx context.Context,
err any,
hint *EventHint,
scope EventModifier,
) *EventID {
// Recover captures a panic or an explicitly supplied recovered value.
func (client *defaultClient) Recover(ctx context.Context, err any, options ...CaptureOption) *EventID {
if err == nil {
err = recover()
}
if err == nil {
return nil
}

if ctx != nil {
if hint == nil {
hint = &EventHint{}
}
if hint.Context == nil {
hint.Context = ctx
}
}

options = append(options, withDefaultRecoveredException(err))
var event *Event
switch err := err.(type) {
case error:
Expand All @@ -764,7 +736,7 @@ func (client *defaultClient) RecoverWithContext(
default:
event = client.EventFromMessage(fmt.Sprintf("%#v", err), LevelFatal)
}
return client.CaptureEvent(event, hint, scope)
return client.CaptureEvent(ctx, event, options...)
}

// Flush waits until the underlying Transport sends any buffered events to the
Expand Down Expand Up @@ -913,7 +885,7 @@ func (client *defaultClient) GetSDKVersion() string {
func (client *defaultClient) processEvent(event *Event, hint *EventHint, scope EventModifier) *EventID {
if event == nil {
err := usageError{fmt.Errorf("%s called with nil event", callerFunctionName())}
return client.CaptureException(err, hint, scope)
return client.processEvent(client.EventFromException(err, LevelError), hint, scope)
}
Comment on lines +888 to 889

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: A recursive call in processEvent when handling a nil event can cause infinite recursion and a stack overflow if a noopClient is used.
Severity: HIGH

Suggested Fix

Revert the logic inside the if event == nil block to call client.CaptureException(err, hint, scope) instead of making a recursive call to processEvent. This avoids the infinite loop by using the original, safe error handling path.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: client.go#L888-L889

Potential issue: The `processEvent` function handles a `nil` event by creating a
`usageError` and making a recursive call to itself with an event generated by
`client.EventFromException`. However, when a `noopClient` is active (a common case when
Sentry is disabled or in test environments), its implementation of `EventFromException`
returns `nil`. This creates an infinite recursion loop: `processEvent(nil)` calls
`processEvent(EventFromException(...))`, which evaluates back to `processEvent(nil)`.
This will lead to a stack overflow, crashing the application.

Did we get this right? 👍 / 👎 to inform future reviews.


// Transactions are sampled by options.TracesSampleRate or
Expand Down
Loading
Loading