Skip to content

Commit

Permalink
Make hint non-nullable in BeforeSendCallback, `BeforeBreadcrumbCa…
Browse files Browse the repository at this point in the history
…ll` and `EventProcessor` (#1574)
  • Loading branch information
denrase authored Sep 4, 2023
1 parent c59de47 commit 052a368
Show file tree
Hide file tree
Showing 47 changed files with 279 additions and 234 deletions.
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@
- Older self-hosted sentry instances will drop transactions containing unfinished spans.
- This change was introduced in [relay/#1690](https://github.com/getsentry/relay/pull/1690) and released with [22.12.0](https://github.com/getsentry/relay/releases/tag/22.12.0)
- Do not leak extensions of external classes ([#1576](https://github.com/getsentry/sentry-dart/pull/1576))

- Make `hint` non-nullable in `BeforeSendCallback`, `BeforeBreadcrumbCall` and `EventProcessor` ([#1574](https://github.com/getsentry/sentry-dart/pull/1574))
- This will affect your callbacks, making this a breaking change.

## Unreleased

### Fixes
Expand Down
2 changes: 1 addition & 1 deletion dart/example/bin/example.dart
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ Future<void> decode() async {

class TagEventProcessor implements EventProcessor {
@override
SentryEvent? apply(SentryEvent event, {hint}) {
SentryEvent? apply(SentryEvent event, hint) {
return event..tags?.addAll({'page-locale': 'en-us'});
}
}
2 changes: 1 addition & 1 deletion dart/example_web/web/main.dart
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ Future<void> parseData() async {

class TagEventProcessor implements EventProcessor {
@override
SentryEvent? apply(SentryEvent event, {hint}) {
SentryEvent? apply(SentryEvent event, Hint hint) {
return event..tags?.addAll({'page-locale': 'en-us'});
}
}
6 changes: 3 additions & 3 deletions dart/lib/src/event_processor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import 'protocol.dart';
/// null in case the event will be dropped and not sent.
abstract class EventProcessor {
FutureOr<SentryEvent?> apply(
SentryEvent event, {
Hint? hint,
});
SentryEvent event,
Hint hint,
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class DeduplicationEventProcessor implements EventProcessor {
final SentryOptions _options;

@override
SentryEvent? apply(SentryEvent event, {Hint? hint}) {
SentryEvent? apply(SentryEvent event, Hint hint) {
if (event is SentryTransaction) {
return event;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class IoEnricherEventProcessor implements EnricherEventProcessor {
final SentryOptions _options;

@override
SentryEvent? apply(SentryEvent event, {Hint? hint}) {
SentryEvent? apply(SentryEvent event, Hint hint) {
// If there's a native integration available, it probably has better
// information available than Flutter.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class WebEnricherEventProcessor implements EnricherEventProcessor {
final SentryOptions _options;

@override
SentryEvent? apply(SentryEvent event, {Hint? hint}) {
SentryEvent? apply(SentryEvent event, Hint hint) {
// Web has no native integration, so no need to check for it

final contexts = event.contexts.copyWith(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class IoExceptionEventProcessor implements ExceptionEventProcessor {
final SentryOptions _options;

@override
SentryEvent? apply(SentryEvent event, {Hint? hint}) {
SentryEvent? apply(SentryEvent event, Hint hint) {
final throwable = event.throwable;
if (throwable is HttpException) {
return _applyHttpException(throwable, event);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,5 @@ ExceptionEventProcessor exceptionEventProcessor(SentryOptions _) =>

class WebExcptionEventProcessor implements ExceptionEventProcessor {
@override
SentryEvent apply(SentryEvent event, {Hint? hint}) => event;
SentryEvent apply(SentryEvent event, Hint hint) => event;
}
8 changes: 4 additions & 4 deletions dart/lib/src/hint.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ import 'sentry_attachment/sentry_attachment.dart';
/// Example:
///
/// ```dart
/// options.beforeSend = (event, {hint}) {
/// final syntheticException = hint?.get(TypeCheckHint.syntheticException);
/// options.beforeSend = (event, hint) {
/// final syntheticException = hint.get(TypeCheckHint.syntheticException);
/// if (syntheticException is FlutterErrorDetails) {
/// // Do something with hint data
/// }
Expand All @@ -28,14 +28,14 @@ import 'sentry_attachment/sentry_attachment.dart';
/// ```dart
/// import 'dart:convert';
///
/// options.beforeSend = (event, {hint}) {
/// options.beforeSend = (event, hint) {
/// final text = 'This event should not be sent happen in prod. Investigate.';
/// final textAttachment = SentryAttachment.fromIntList(
/// utf8.encode(text),
/// 'event_info.txt',
/// contentType: 'text/plain',
/// );
/// hint?.attachments.add(textAttachment);
/// hint.attachments.add(textAttachment);
/// return event;
/// };
/// ```
Expand Down
16 changes: 8 additions & 8 deletions dart/lib/src/scope.dart
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ class Scope {

Scope(this._options);

Breadcrumb? _addBreadCrumbSync(Breadcrumb breadcrumb, {Hint? hint}) {
Breadcrumb? _addBreadCrumbSync(Breadcrumb breadcrumb, Hint hint) {
// bail out if maxBreadcrumbs is zero
if (_options.maxBreadcrumbs == 0) {
return null;
Expand All @@ -155,7 +155,7 @@ class Scope {
try {
processedBreadcrumb = _options.beforeBreadcrumb!(
processedBreadcrumb,
hint: hint,
hint,
);
if (processedBreadcrumb == null) {
_options.logger(
Expand Down Expand Up @@ -189,7 +189,7 @@ class Scope {

/// Adds a breadcrumb to the breadcrumbs queue
Future<void> addBreadcrumb(Breadcrumb breadcrumb, {Hint? hint}) async {
final addedBreadcrumb = _addBreadCrumbSync(breadcrumb, hint: hint);
final addedBreadcrumb = _addBreadCrumbSync(breadcrumb, hint ?? Hint());
if (addedBreadcrumb != null) {
await _callScopeObservers((scopeObserver) async =>
await scopeObserver.addBreadcrumb(addedBreadcrumb));
Expand Down Expand Up @@ -275,9 +275,9 @@ class Scope {
}

Future<SentryEvent?> applyToEvent(
SentryEvent event, {
Hint? hint,
}) async {
SentryEvent event,
Hint hint,
) async {
event = event.copyWith(
transaction: event.transaction ?? transaction,
user: _mergeUsers(user, event.user),
Expand Down Expand Up @@ -320,7 +320,7 @@ class Scope {
SentryEvent? processedEvent = event;
for (final processor in _eventProcessors) {
try {
final e = processor.apply(processedEvent!, hint: hint);
final e = processor.apply(processedEvent!, hint);
if (e is Future<SentryEvent?>) {
processedEvent = await e;
} else {
Expand Down Expand Up @@ -429,7 +429,7 @@ class Scope {
}

for (final breadcrumb in List.from(_breadcrumbs)) {
clone._addBreadCrumbSync(breadcrumb);
clone._addBreadCrumbSync(breadcrumb, Hint());
}

for (final eventProcessor in List.from(_eventProcessors)) {
Expand Down
29 changes: 16 additions & 13 deletions dart/lib/src/sentry_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ class SentryClient {
hint ??= Hint();

if (scope != null) {
preparedEvent = await scope.applyToEvent(preparedEvent, hint: hint);
preparedEvent = await scope.applyToEvent(preparedEvent, hint);
} else {
_options.logger(
SentryLevel.debug, 'No scope to apply on event was provided');
Expand All @@ -89,8 +89,8 @@ class SentryClient {

preparedEvent = await _runEventProcessors(
preparedEvent,
hint,
eventProcessors: _options.eventProcessors,
hint: hint,
);

// dropped by event processors
Expand All @@ -100,7 +100,7 @@ class SentryClient {

preparedEvent = await _runBeforeSend(
preparedEvent,
hint: hint,
hint,
);

// dropped by beforeSend
Expand Down Expand Up @@ -288,9 +288,11 @@ class SentryClient {
SentryTransaction? preparedTransaction =
_prepareEvent(transaction) as SentryTransaction;

final hint = Hint();

if (scope != null) {
preparedTransaction =
await scope.applyToEvent(preparedTransaction) as SentryTransaction?;
preparedTransaction = await scope.applyToEvent(preparedTransaction, hint)
as SentryTransaction?;
} else {
_options.logger(
SentryLevel.debug, 'No scope to apply on transaction was provided');
Expand All @@ -303,6 +305,7 @@ class SentryClient {

preparedTransaction = await _runEventProcessors(
preparedTransaction,
hint,
eventProcessors: _options.eventProcessors,
) as SentryTransaction?;

Expand All @@ -312,7 +315,7 @@ class SentryClient {
}

preparedTransaction =
await _runBeforeSend(preparedTransaction) as SentryTransaction?;
await _runBeforeSend(preparedTransaction, hint) as SentryTransaction?;

// dropped by beforeSendTransaction
if (preparedTransaction == null) {
Expand Down Expand Up @@ -354,9 +357,9 @@ class SentryClient {
void close() => _options.httpClient.close();

Future<SentryEvent?> _runBeforeSend(
SentryEvent event, {
Hint? hint,
}) async {
SentryEvent event,
Hint hint,
) async {
SentryEvent? eventOrTransaction = event;

final beforeSend = _options.beforeSend;
Expand All @@ -373,7 +376,7 @@ class SentryClient {
eventOrTransaction = e;
}
} else if (beforeSend != null) {
final e = beforeSend(event, hint: hint);
final e = beforeSend(event, hint);
if (e is Future<SentryEvent?>) {
eventOrTransaction = await e;
} else {
Expand Down Expand Up @@ -404,14 +407,14 @@ class SentryClient {
}

Future<SentryEvent?> _runEventProcessors(
SentryEvent event, {
Hint? hint,
SentryEvent event,
Hint hint, {
required List<EventProcessor> eventProcessors,
}) async {
SentryEvent? processedEvent = event;
for (final processor in eventProcessors) {
try {
final e = processor.apply(processedEvent!, hint: hint);
final e = processor.apply(processedEvent!, hint);
if (e is Future<SentryEvent?>) {
processedEvent = await e;
} else {
Expand Down
12 changes: 6 additions & 6 deletions dart/lib/src/sentry_options.dart
Original file line number Diff line number Diff line change
Expand Up @@ -444,9 +444,9 @@ class SentryOptions {
/// This function is called with an SDK specific event object and can return a modified event
/// object or nothing to skip reporting the event
typedef BeforeSendCallback = FutureOr<SentryEvent?> Function(
SentryEvent event, {
Hint? hint,
});
SentryEvent event,
Hint hint,
);

/// This function is called with an SDK specific transaction object and can return a modified transaction
/// object or nothing to skip reporting the transaction
Expand All @@ -457,9 +457,9 @@ typedef BeforeSendTransactionCallback = FutureOr<SentryTransaction?> Function(
/// This function is called with an SDK specific breadcrumb object before the breadcrumb is added
/// to the scope. When nothing is returned from the function, the breadcrumb is dropped
typedef BeforeBreadcrumbCallback = Breadcrumb? Function(
Breadcrumb? breadcrumb, {
Hint? hint,
});
Breadcrumb? breadcrumb,
Hint hint,
);

/// Used to provide timestamp for logging.
typedef ClockProvider = DateTime Function();
Expand Down
24 changes: 12 additions & 12 deletions dart/test/event_processor/deduplication_event_processor_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,33 +15,33 @@ void main() {
final sut = fixture.getSut(true);
var ogEvent = _createEvent('foo');

expect(sut.apply(ogEvent), isNotNull);
expect(sut.apply(ogEvent), isNull);
expect(sut.apply(ogEvent, Hint()), isNotNull);
expect(sut.apply(ogEvent, Hint()), isNull);
});

test('does not deduplicate if disabled', () {
final sut = fixture.getSut(false);
var ogEvent = _createEvent('foo');

expect(sut.apply(ogEvent), isNotNull);
expect(sut.apply(ogEvent), isNotNull);
expect(sut.apply(ogEvent, Hint()), isNotNull);
expect(sut.apply(ogEvent, Hint()), isNotNull);
});

test('does not deduplicate if different events', () {
final sut = fixture.getSut(true);
var fooEvent = _createEvent('foo');
var barEvent = _createEvent('bar');

expect(sut.apply(fooEvent), isNotNull);
expect(sut.apply(barEvent), isNotNull);
expect(sut.apply(fooEvent, Hint()), isNotNull);
expect(sut.apply(barEvent, Hint()), isNotNull);
});

test('does not deduplicate transaction', () {
final sut = fixture.getSut(true);
final transaction = _createTransaction(fixture.hub);

expect(sut.apply(transaction), isNotNull);
expect(sut.apply(transaction), isNotNull);
expect(sut.apply(transaction, Hint()), isNotNull);
expect(sut.apply(transaction, Hint()), isNotNull);
});

test('exceptions to keep for deduplication', () {
Expand All @@ -51,10 +51,10 @@ void main() {
var barEvent = _createEvent('bar');
var fooBarEvent = _createEvent('foo bar');

expect(sut.apply(fooEvent), isNotNull);
expect(sut.apply(barEvent), isNotNull);
expect(sut.apply(fooBarEvent), isNotNull);
expect(sut.apply(fooEvent), isNotNull);
expect(sut.apply(fooEvent, Hint()), isNotNull);
expect(sut.apply(barEvent, Hint()), isNotNull);
expect(sut.apply(fooBarEvent, Hint()), isNotNull);
expect(sut.apply(fooEvent, Hint()), isNotNull);
});

test('integration test', () async {
Expand Down
Loading

0 comments on commit 052a368

Please sign in to comment.