Skip to content
Merged
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
- Remove deprecated `beforeScreenshot` ([#2662](https://github.com/getsentry/sentry-dart/pull/2662))
- Remove deprecated loggers ([#2685](https://github.com/getsentry/sentry-dart/pull/2685))
- Remove user segment ([#2687](https://github.com/getsentry/sentry-dart/pull/2687))
- Enable JS SDK native integration by default ([#2688](https://github.com/getsentry/sentry-dart/pull/2688))
- Remove `enableTracing` ([#2695](https://github.com/getsentry/sentry-dart/pull/2695))
- Remove `options.autoAppStart` and `setAppStartEnd` ([#2680](https://github.com/getsentry/sentry-dart/pull/2680))
- Add hint for transactions ([#2675](https://github.com/getsentry/sentry-dart/pull/2675))
Expand Down
2 changes: 1 addition & 1 deletion dart/lib/src/sentry_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class SentryClient {
options,
options.transport,
);
// TODO: Web might change soon to use the JS SDK so we can remove it here later on
// TODO: Use spotlight integration directly through JS SDK, then we can remove isWeb check
final enableFlutterSpotlight = (options.spotlight.enabled &&
(options.platformChecker.isWeb ||
options.platformChecker.platform.isLinux ||
Expand Down
14 changes: 5 additions & 9 deletions flutter/example/integration_test/web_sdk_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import 'dart:js_interop_unsafe';
import 'package:flutter_test/flutter_test.dart';
import 'package:integration_test/integration_test.dart';
import 'package:sentry_flutter/sentry_flutter.dart';
import 'package:sentry_flutter/src/web/javascript_transport.dart';
import 'package:sentry_flutter_example/main.dart' as app;

import 'utils.dart';
Expand Down Expand Up @@ -48,11 +47,14 @@ void main() {
group('Web SDK Integration', () {
IntegrationTestWidgetsFlutterBinding.ensureInitialized();

tearDown(() async {
await Sentry.close();
});

group('enabled', () {
testWidgets('Sentry JS SDK initialized', (tester) async {
await restoreFlutterOnErrorAfter(() async {
await SentryFlutter.init((options) {
options.enableSentryJs = true;
options.dsn = fakeDsn;
}, appRunner: () async {
await tester.pumpWidget(const app.MyApp());
Expand All @@ -72,25 +74,20 @@ void main() {
});

testWidgets('sends the correct envelope', (tester) async {
SentryFlutterOptions? configuredOptions;
SentryEvent? dartEvent;

await restoreFlutterOnErrorAfter(() async {
await SentryFlutter.init((options) {
options.enableSentryJs = true;
options.dsn = fakeDsn;
options.beforeSend = (event, hint) {
dartEvent = event;
return event;
};
configuredOptions = options;
}, appRunner: () async {
await tester.pumpWidget(const app.MyApp());
});
});

expect(configuredOptions!.transport, isA<JavascriptTransport>());

final client = _getClient()!;
final completer = Completer<List<Object?>>();

Expand Down Expand Up @@ -131,7 +128,6 @@ void main() {

await restoreFlutterOnErrorAfter(() async {
await SentryFlutter.init((options) {
options.enableSentryJs = true;
options.dsn = fakeDsn;
options.attachScreenshot = true;

Expand Down Expand Up @@ -165,8 +161,8 @@ void main() {
testWidgets('Sentry JS SDK is not initialized', (tester) async {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we even need this test, as we always enable by default? If not, we can also remove the enabled group.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

depends, this specifically tests that if autoInitializeNativeSdks = false then we won't init the SDK

I forgot the autoInitializeNativeSdks = false

await restoreFlutterOnErrorAfter(() async {
await SentryFlutter.init((options) {
options.enableSentryJs = false;
options.dsn = fakeDsn;
options.autoInitializeNativeSdk = false;
}, appRunner: () async {
await tester.pumpWidget(const app.MyApp());
});
Expand Down
1 change: 0 additions & 1 deletion flutter/example/lib/main.dart
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ Future<void> setupSentry(
options.debug = kDebugMode;
options.spotlight = Spotlight(enabled: true);
options.enableTimeToFullDisplayTracing = true;
options.enableSentryJs = true;

options.maxRequestBodySize = MaxRequestBodySize.always;
options.maxResponseBodySize = MaxResponseBodySize.always;
Expand Down
6 changes: 1 addition & 5 deletions flutter/lib/src/integrations/web_sdk_integration.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import 'package:sentry/sentry.dart';

import '../native/sentry_native_binding.dart';
import '../sentry_flutter_options.dart';
import '../web/javascript_transport.dart';
import '../web/script_loader/sentry_script_loader.dart';
import '../web/sentry_js_bundle.dart';

Expand All @@ -27,7 +26,7 @@ class WebSdkIntegration implements Integration<SentryFlutterOptions> {

@override
FutureOr<void> call(Hub hub, SentryFlutterOptions options) async {
if (!options.enableSentryJs || !options.autoInitializeNativeSdk) {
if (!options.autoInitializeNativeSdk) {
return;
}

Expand All @@ -39,9 +38,6 @@ class WebSdkIntegration implements Integration<SentryFlutterOptions> {
: productionScripts;
await _scriptLoader.loadWebSdk(scripts);
await _web.init(hub);
if (_web.supportsCaptureEnvelope) {
options.transport = JavascriptTransport(_web, options);
}
options.sdk.addIntegration(name);
} catch (exception, stackTrace) {
options.logger(
Expand Down
8 changes: 4 additions & 4 deletions flutter/lib/src/sentry_flutter.dart
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import 'utils/platform_dispatcher_wrapper.dart';
import 'version.dart';
import 'view_hierarchy/view_hierarchy_integration.dart';
import 'web/javascript_transport.dart';

/// Configuration options callback
typedef FlutterOptionsConfiguration = FutureOr<void> Function(
Expand Down Expand Up @@ -125,10 +126,9 @@
// Not all platforms have a native integration.
if (_native != null) {
if (_native!.supportsCaptureEnvelope) {
// Sentry's native web integration is only enabled when enableSentryJs=true.
// Transport configuration happens in web_integration because the configuration
// options aren't available until after the options callback executes.
if (!options.platformChecker.isWeb) {
if (options.platformChecker.isWeb) {
options.transport = JavascriptTransport(_native!, options);

Check warning on line 130 in flutter/lib/src/sentry_flutter.dart

View check run for this annotation

Codecov / codecov/patch

flutter/lib/src/sentry_flutter.dart#L130

Added line #L130 was not covered by tests
} else {
options.transport = FileSystemTransport(_native!, options);
}
}
Expand Down
7 changes: 0 additions & 7 deletions flutter/lib/src/sentry_flutter_options.dart
Original file line number Diff line number Diff line change
Expand Up @@ -284,13 +284,6 @@ class SentryFlutterOptions extends SentryOptions {
/// you must use `SentryWidgetsFlutterBinding.ensureInitialized()` instead.
bool enableFramesTracking = true;

/// Controls initialization of the Sentry Javascript SDK on web platforms.
/// When enabled and [autoInitializeNativeSdk] is true, loads and initializes
/// the JS SDK in the document head.
///
/// Defaults to `false`
bool enableSentryJs = false;

/// By using this, you are disabling native [Breadcrumb] tracking and instead
/// you are just tracking [Breadcrumb]s which result from events available
/// in the current Flutter environment.
Expand Down
51 changes: 0 additions & 51 deletions flutter/test/integrations/web_sdk_integration_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,7 @@ library;

import 'package:flutter_test/flutter_test.dart';
import 'package:mockito/mockito.dart';
import 'package:sentry/src/transport/noop_transport.dart';
import 'package:sentry_flutter/src/integrations/web_sdk_integration.dart';
import 'package:sentry_flutter/src/web/javascript_transport.dart';
import 'package:sentry_flutter/src/web/script_loader/sentry_script_loader.dart';

import '../mocks.dart';
Expand All @@ -27,7 +25,6 @@ void main() {

group('enabled', () {
setUp(() {
fixture.options.enableSentryJs = true;
fixture.options.autoInitializeNativeSdk = true;
});

Expand All @@ -52,17 +49,9 @@ void main() {
_TestScenario(
'with autoInitializeNativeSdk=false',
() {
fixture.options.enableSentryJs = true;
fixture.options.autoInitializeNativeSdk = false;
},
),
_TestScenario(
'with enableSentryJs=false',
() {
fixture.options.enableSentryJs = false;
fixture.options.autoInitializeNativeSdk = true;
},
),
];

for (final scenario in disabledScenarios) {
Expand All @@ -84,46 +73,6 @@ void main() {
}
});

group('transport configuration', () {
test('integration disabled: does not use javascript transport', () async {
fixture.options.enableSentryJs = false;
fixture.options.autoInitializeNativeSdk = false;

expect(fixture.options.transport, isA<NoOpTransport>());

await sut.call(fixture.hub, fixture.options);

expect(fixture.options.transport, isA<NoOpTransport>());
});

test(
'integration enabled and supportsCaptureEnvelope is false: does not use javascript transport',
() async {
fixture.options.enableSentryJs = true;
fixture.options.autoInitializeNativeSdk = true;
when(fixture.web.supportsCaptureEnvelope).thenReturn(false);

expect(fixture.options.transport, isA<NoOpTransport>());

await sut.call(fixture.hub, fixture.options);

expect(fixture.options.transport, isA<NoOpTransport>());
});

test(
'integration enabled and supportsCaptureEnvelope is true: uses javascript transport',
() async {
fixture.options.enableSentryJs = true;
fixture.options.autoInitializeNativeSdk = true;

expect(fixture.options.transport, isA<NoOpTransport>());

await sut.call(fixture.hub, fixture.options);

expect(fixture.options.transport, isA<JavascriptTransport>());
});
});

test('closes resources', () async {
await sut.close();

Expand Down
9 changes: 5 additions & 4 deletions flutter/test/sentry_flutter_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import 'package:sentry_flutter/src/renderer/renderer.dart';
import 'package:sentry_flutter/src/replay/integration.dart';
import 'package:sentry_flutter/src/version.dart';
import 'package:sentry_flutter/src/view_hierarchy/view_hierarchy_integration.dart';
import 'package:sentry_flutter/src/web/javascript_transport.dart';

import 'mocks.dart';
import 'mocks.mocks.dart';
Expand Down Expand Up @@ -336,7 +337,7 @@ void main() {
options: sentryFlutterOptions,
);

expect(transport, isNot(isA<FileSystemTransport>()));
expect(transport, isA<JavascriptTransport>());

testScopeObserver(
options: sentryFlutterOptions,
Expand Down Expand Up @@ -409,7 +410,7 @@ void main() {
options: sentryFlutterOptions,
);

expect(transport, isNot(isA<FileSystemTransport>()));
expect(transport, isA<JavascriptTransport>());

testConfiguration(
integrations: integrations,
Expand Down Expand Up @@ -452,7 +453,7 @@ void main() {
options: sentryFlutterOptions,
);

expect(transport, isNot(isA<FileSystemTransport>()));
expect(transport, isA<JavascriptTransport>());

testConfiguration(
integrations: integrations,
Expand Down Expand Up @@ -495,7 +496,7 @@ void main() {
options: sentryFlutterOptions,
);

expect(transport, isNot(isA<FileSystemTransport>()));
expect(transport, isA<JavascriptTransport>());

testConfiguration(
integrations: integrations,
Expand Down
Loading