Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support captureFeedback #2230

Open
wants to merge 35 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
249fad9
add captureFeedback methods
denrase Aug 13, 2024
76e733e
add should capture feedback as event test
denrase Aug 13, 2024
5595323
add sentry_feedback_test
denrase Aug 13, 2024
07815d8
update contexts test
denrase Aug 13, 2024
d3488c2
Merge branch 'main' into feat/capture-feedback
denrase Aug 13, 2024
684ef04
test before send feedback
denrase Aug 13, 2024
f3c8951
test hint and event processors
denrase Aug 13, 2024
95d8d9c
basic scope test
denrase Aug 13, 2024
ab350dc
test trace context and attachment behaviour
denrase Aug 13, 2024
26236e3
test sample rate for feedback and fix mock transport calls comparison
denrase Aug 13, 2024
bc22937
add hub tests
denrase Aug 13, 2024
58d83f9
add sentry tests
denrase Aug 13, 2024
74b5529
add changelog entry
denrase Aug 13, 2024
c86b10e
Merge branch 'main' into feat/capture-feedback
denrase Aug 19, 2024
9a7282c
cleanup + comments
denrase Aug 19, 2024
b238583
test envelope item for feedback
denrase Aug 19, 2024
8e12e8f
remove duplacte typedef
denrase Aug 19, 2024
ed773ef
fix test expectation
denrase Aug 19, 2024
ee71696
Deprecate captureUserFeedback
denrase Aug 19, 2024
4b403dd
update depraction info in cl
denrase Aug 19, 2024
b01cef2
format
denrase Aug 19, 2024
1733c9e
add missing option in test
denrase Aug 19, 2024
7b25235
run format
denrase Aug 19, 2024
d5c1f0d
organize imports
denrase Aug 19, 2024
4785da9
add missing method
denrase Aug 19, 2024
4ae6696
fix test epectation
denrase Aug 19, 2024
06a4a5f
ignore deprecations internally
denrase Aug 19, 2024
4bb3ceb
add to integration test, fix analyze errors
denrase Aug 19, 2024
34b2626
Merge branch 'main' into feat/capture-feedback
denrase Sep 5, 2024
5a18fdf
fix cl
denrase Sep 5, 2024
4f368c9
Merge branch 'main' into feat/capture-feedback
denrase Sep 23, 2024
995d288
disable fixture.options.automatedTestMode
denrase Sep 23, 2024
3114a70
update test
denrase Sep 23, 2024
55c6870
fix cl
denrase Sep 23, 2024
f1a8281
Add `SentryFeedbackWidget` (#2240)
denrase Sep 23, 2024
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
2 changes: 2 additions & 0 deletions dart/lib/sentry.dart
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,5 @@ export 'src/utils.dart';
export 'src/spotlight.dart';
// proxy
export 'src/protocol/sentry_proxy.dart';
// feedback
export 'src/protocol/sentry_feedback.dart';
42 changes: 42 additions & 0 deletions dart/lib/src/hub.dart
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,48 @@ class Hub {
}
}

Future<SentryId> captureFeedback(
SentryFeedback feedback, {
Hint? hint,
ScopeCallback? withScope,
}) async {
var sentryId = SentryId.empty();

if (!_isEnabled) {
_options.logger(
SentryLevel.warning,
"Instance is disabled and this 'captureFeedback' call is a no-op.",
);
return sentryId;
}

final item = _peek();
late Scope scope;
final s = _cloneAndRunWithScope(item.scope, withScope);
if (s is Future<Scope>) {
scope = await s;
} else {
scope = s;
}

try {
sentryId = await item.client.captureFeedback(
feedback,
hint: hint,
scope: scope,
);
} catch (exception, stacktrace) {
_options.logger(
SentryLevel.error,
'Error while capturing user feedback',
exception: exception,
stackTrace: stacktrace,
);
}

return sentryId;
}

FutureOr<Scope> _cloneAndRunWithScope(
Scope scope, ScopeCallback? withScope) async {
if (withScope != null) {
Expand Down
13 changes: 13 additions & 0 deletions dart/lib/src/hub_adapter.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import 'metrics/metrics_aggregator.dart';
import 'metrics/metrics_api.dart';
import 'profiling.dart';
import 'protocol.dart';
import 'protocol/sentry_feedback.dart';
import 'scope.dart';
import 'sentry.dart';
import 'sentry_client.dart';
Expand Down Expand Up @@ -196,4 +197,16 @@ class HubAdapter implements Hub {
@override
MetricsAggregator? get metricsAggregator =>
Sentry.currentHub.metricsAggregator;

@override
Future<SentryId> captureFeedback(
SentryFeedback feedback, {
Hint? hint,
ScopeCallback? withScope,
}) =>
Sentry.currentHub.captureFeedback(
feedback,
hint: hint,
withScope: withScope,
);
}
9 changes: 9 additions & 0 deletions dart/lib/src/noop_hub.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import 'metrics/metrics_aggregator.dart';
import 'metrics/metrics_api.dart';
import 'profiling.dart';
import 'protocol.dart';
import 'protocol/sentry_feedback.dart';
import 'scope.dart';
import 'sentry_client.dart';
import 'sentry_options.dart';
Expand Down Expand Up @@ -98,6 +99,14 @@ class NoOpHub implements Hub {
@override
Future<void> captureUserFeedback(SentryUserFeedback userFeedback) async {}

@override
Future<SentryId> captureFeedback(
SentryFeedback feedback, {
Hint? hint,
ScopeCallback? withScope,
}) async =>
SentryId.empty();

@override
ISentrySpan startTransaction(
String name,
Expand Down
22 changes: 22 additions & 0 deletions dart/lib/src/protocol/contexts.dart
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import 'dart:collection';

import '../protocol.dart';
import 'sentry_feedback.dart';

/// The context interfaces provide additional context data.
///
Expand All @@ -19,6 +20,7 @@ class Contexts extends MapView<String, dynamic> {
SentryCulture? culture,
SentryTraceContext? trace,
SentryResponse? response,
SentryFeedback? feedback,
}) : super({
SentryDevice.type: device,
SentryOperatingSystem.type: operatingSystem,
Expand All @@ -29,6 +31,7 @@ class Contexts extends MapView<String, dynamic> {
SentryCulture.type: culture,
SentryTraceContext.type: trace,
SentryResponse.type: response,
SentryFeedback.type: feedback,
});

/// Deserializes [Contexts] from JSON [Map].
Expand Down Expand Up @@ -62,6 +65,9 @@ class Contexts extends MapView<String, dynamic> {
response: data[SentryResponse.type] != null
? SentryResponse.fromJson(Map.from(data[SentryResponse.type]))
: null,
feedback: data[SentryFeedback.type] != null
? SentryFeedback.fromJson(Map.from(data[SentryFeedback.type]))
: null,
);

data.keys
Expand Down Expand Up @@ -136,6 +142,11 @@ class Contexts extends MapView<String, dynamic> {

set response(SentryResponse? value) => this[SentryResponse.type] = value;

/// Feedback context for a FeedbackEvent.
SentryFeedback? get feedback => this[SentryFeedback.type];

set feedback(SentryFeedback? value) => this[SentryFeedback.type] = value;

/// Produces a [Map] that can be serialized to JSON.
Map<String, dynamic> toJson() {
final json = <String, dynamic>{};
Expand Down Expand Up @@ -198,6 +209,13 @@ class Contexts extends MapView<String, dynamic> {
}
break;

case SentryFeedback.type:
final feedbackMap = feedback?.toJson();
if (feedbackMap?.isNotEmpty ?? false) {
json[SentryFeedback.type] = feedbackMap;
}
break;

case SentryRuntime.listType:
if (runtimes.length == 1) {
final runtime = runtimes[0];
Expand Down Expand Up @@ -249,6 +267,7 @@ class Contexts extends MapView<String, dynamic> {
trace: trace?.clone(),
response: response?.clone(),
runtimes: runtimes.map((runtime) => runtime.clone()).toList(),
feedback: feedback?.clone(),
)..addEntries(
entries.where((element) => !_defaultFields.contains(element.key)),
);
Expand All @@ -266,6 +285,7 @@ class Contexts extends MapView<String, dynamic> {
SentryGpu? gpu,
SentryTraceContext? trace,
SentryResponse? response,
SentryFeedback? feedback,
}) =>
Contexts(
device: device ?? this.device,
Expand All @@ -277,6 +297,7 @@ class Contexts extends MapView<String, dynamic> {
culture: culture ?? this.culture,
trace: trace ?? this.trace,
response: response ?? this.response,
feedback: feedback ?? this.feedback,
)..addEntries(
entries.where((element) => !_defaultFields.contains(element.key)),
);
Expand All @@ -292,5 +313,6 @@ class Contexts extends MapView<String, dynamic> {
SentryCulture.type,
SentryTraceContext.type,
SentryResponse.type,
SentryFeedback.type,
];
}
81 changes: 81 additions & 0 deletions dart/lib/src/protocol/sentry_feedback.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
import 'package:meta/meta.dart';

import 'access_aware_map.dart';
import 'sentry_id.dart';

@immutable
class SentryFeedback {
static const type = 'feedback';

SentryFeedback({
required this.message,
this.contactEmail,
this.name,
this.replayId,
this.url,
this.associatedEventId,
this.unknown,
});

final String message;
final String? contactEmail;
final String? name;
final String? replayId;
final String? url;
final SentryId? associatedEventId;

@internal
final Map<String, dynamic>? unknown;

/// Deserializes a [SentryOperatingSystem] from JSON [Map].
factory SentryFeedback.fromJson(Map<String, dynamic> data) {
final json = AccessAwareMap(data);

String? associatedEventId = json['associated_event_id'];

return SentryFeedback(
message: json['message'],
contactEmail: json['contact_email'],
name: json['name'],
replayId: json['replay_id'],
url: json['url'],
associatedEventId:
associatedEventId != null ? SentryId.fromId(associatedEventId) : null,
unknown: json.notAccessed(),
);
}

Map<String, dynamic> toJson() {
return {
...?unknown,
'message': message,
if (contactEmail != null) 'contact_email': contactEmail,
if (name != null) 'name': name,
if (replayId != null) 'replay_id': replayId,
if (url != null) 'url': url,
if (associatedEventId != null)
'associated_event_id': associatedEventId.toString(),
};
}

SentryFeedback copyWith({
String? message,
String? contactEmail,
String? name,
String? replayId,
String? url,
SentryId? associatedEventId,
Map<String, dynamic>? unknown,
}) =>
SentryFeedback(
message: message ?? this.message,
contactEmail: contactEmail ?? this.contactEmail,
name: name ?? this.name,
replayId: replayId ?? this.replayId,
url: url ?? this.url,
associatedEventId: associatedEventId ?? this.associatedEventId,
unknown: unknown ?? this.unknown,
);

SentryFeedback clone() => copyWith();
}
4 changes: 4 additions & 0 deletions dart/lib/src/sentry.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import 'package:meta/meta.dart';

import 'dart_exception_type_identifier.dart';
import 'metrics/metrics_api.dart';
import 'protocol/sentry_feedback.dart';
import 'run_zoned_guarded_integration.dart';
import 'event_processor/enricher/enricher_event_processor.dart';
import 'environment/environment_variables.dart';
Expand Down Expand Up @@ -219,6 +220,9 @@ class Sentry {
static Future<void> captureUserFeedback(SentryUserFeedback userFeedback) =>
_hub.captureUserFeedback(userFeedback);

static Future<void> captureFeedback(SentryFeedback feedback) =>
_hub.captureFeedback(feedback);

/// Close the client SDK
static Future<void> close() async {
final hub = _hub;
Expand Down
36 changes: 34 additions & 2 deletions dart/lib/src/sentry_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import 'hint.dart';
import 'metrics/metric.dart';
import 'metrics/metrics_aggregator.dart';
import 'protocol.dart';
import 'protocol/sentry_feedback.dart';
import 'scope.dart';
import 'sentry_attachment/sentry_attachment.dart';
import 'sentry_baggage.dart';
Expand Down Expand Up @@ -104,7 +105,7 @@ class SentryClient {
return _emptySentryId;
}

if (_sampleRate()) {
if (_sampleRate() && event.type != 'feedback') {
_options.recorder
.recordLostEvent(DiscardReason.sampleRate, _getCategory(event));
_options.logger(
Expand Down Expand Up @@ -161,7 +162,7 @@ class SentryClient {
}

var viewHierarchy = hint.viewHierarchy;
if (viewHierarchy != null) {
if (viewHierarchy != null && event.type != 'feedback') {
attachments.add(viewHierarchy);
}

Expand Down Expand Up @@ -213,6 +214,10 @@ class SentryClient {
return event;
}

if (event.type == 'feedback') {
return event;
}

if (event.exceptions?.isNotEmpty ?? false) {
return event;
}
Expand Down Expand Up @@ -433,6 +438,25 @@ class SentryClient {
return _attachClientReportsAndSend(envelope);
}

/// Reports the [SentryFeedback] and to Sentry.io.
Future<SentryId> captureFeedback(
SentryFeedback feedback, {
Scope? scope,
Hint? hint,
}) {
final feedbackEvent = SentryEvent(
type: 'feedback',
contexts: Contexts(feedback: feedback),
level: SentryLevel.info,
);

return captureEvent(
feedbackEvent,
scope: scope,
hint: hint,
);
}

/// Reports the [metricsBuckets] to Sentry.io.
Future<SentryId> captureMetrics(
Map<int, Iterable<Metric>> metricsBuckets) async {
Expand Down Expand Up @@ -460,6 +484,7 @@ class SentryClient {

final beforeSend = _options.beforeSend;
final beforeSendTransaction = _options.beforeSendTransaction;
final beforeSendFeedback = _options.beforeSendFeedback;
String beforeSendName = 'beforeSend';

try {
Expand All @@ -471,6 +496,13 @@ class SentryClient {
} else {
processedEvent = callbackResult;
}
} else if (event.type == 'feedback' && beforeSendFeedback != null) {
final callbackResult = beforeSendFeedback(event, hint);
if (callbackResult is Future<SentryEvent?>) {
processedEvent = await callbackResult;
} else {
processedEvent = callbackResult;
}
} else if (beforeSend != null) {
final callbackResult = beforeSend(event, hint);
if (callbackResult is Future<SentryEvent?>) {
Expand Down
Loading
Loading