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

[v9] Remove deprecated beforeScreenshot #2662

Merged
merged 8 commits into from
Feb 10, 2025
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
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@
## Unreleased 9.0.0

- Increase minimum SDK version requirements to Dart v3.5.0 and Flutter v3.24.0 ([#2643](https://github.com/getsentry/sentry-dart/pull/2643))
- Remove deprecated `beforeScreenshot` ([#2662](https://github.com/getsentry/sentry-dart/pull/2662))

### Dependencies

- Bump Android SDK from v7.20.1 to v8.1.0 ([#2650](https://github.com/getsentry/sentry-dart/pull/2650))
- [changelog](https://github.com/getsentry/sentry-java/blob/main/CHANGELOG.md#810)
- [diff](https://github.com/getsentry/sentry-java/compare/7.20.1...8.1.0)

## Unreleased
## 8.13.0

### Breaking changes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,18 +52,13 @@ class ScreenshotEventProcessor implements EventProcessor {
// skip capturing in case of debouncing (=too many frequent capture requests)
// the BeforeCaptureCallback may overrule the debouncing decision
final shouldDebounce = _debouncer.shouldDebounce();

// ignore: deprecated_member_use_from_same_package
final beforeScreenshot = _options.beforeScreenshot;
final beforeCapture = _options.beforeCaptureScreenshot;
final beforeCaptureScreenshot = _options.beforeCaptureScreenshot;

try {
FutureOr<bool>? result;

if (beforeCapture != null) {
result = beforeCapture(event, hint, shouldDebounce);
} else if (beforeScreenshot != null) {
result = beforeScreenshot(event, hint: hint);
if (beforeCaptureScreenshot != null) {
result = beforeCaptureScreenshot(event, hint, shouldDebounce);
}

bool takeScreenshot = true;
Expand Down
9 changes: 0 additions & 9 deletions flutter/lib/src/sentry_flutter_options.dart
Original file line number Diff line number Diff line change
Expand Up @@ -196,10 +196,6 @@ class SentryFlutterOptions extends SentryOptions {
/// See https://docs.sentry.io/platforms/flutter/troubleshooting/#screenshot-integration-background-crash
bool attachScreenshotOnlyWhenResumed = false;

@Deprecated(
'Will be removed in a future version. Use [beforeCaptureScreenshot] instead')
BeforeScreenshotCallback? beforeScreenshot;

/// Sets a callback which is executed before capturing screenshots. Only
/// relevant if `attachScreenshot` is set to true. When false is returned
/// from the function, no screenshot will be attached.
Expand Down Expand Up @@ -434,11 +430,6 @@ class _SentryFlutterExperimentalOptions {
_privacy ?? SentryPrivacyOptions();
}

@Deprecated(
'Will be removed in a future version. Use [BeforeCaptureCallback] instead')
typedef BeforeScreenshotCallback = FutureOr<bool> Function(SentryEvent event,
{Hint? hint});

/// A callback which can be used to suppress capturing of screenshots.
/// It's called in [ScreenshotEventProcessor] if screenshots are enabled.
/// This gives more fine-grained control over when capturing should be performed,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,96 +134,6 @@ void main() {
});
});

group('beforeScreenshot', () {
testWidgets('does add screenshot if beforeScreenshot returns true',
(tester) async {
// ignore: deprecated_member_use_from_same_package
fixture.options.beforeScreenshot = (SentryEvent event, {Hint? hint}) {
return true;
};
await _addScreenshotAttachment(tester, FlutterRenderer.canvasKit,
added: true, isWeb: false);
});

testWidgets('does add screenshot if async beforeScreenshot returns true',
(tester) async {
// ignore: deprecated_member_use_from_same_package
fixture.options.beforeScreenshot =
(SentryEvent event, {Hint? hint}) async {
await Future<void>.delayed(Duration(milliseconds: 1));
return true;
};
await _addScreenshotAttachment(tester, FlutterRenderer.canvasKit,
added: true, isWeb: false);
});

testWidgets('does not add screenshot if beforeScreenshot returns false',
(tester) async {
// ignore: deprecated_member_use_from_same_package
fixture.options.beforeScreenshot = (SentryEvent event, {Hint? hint}) {
return false;
};
await _addScreenshotAttachment(tester, FlutterRenderer.canvasKit,
added: false, isWeb: false);
});

testWidgets(
'does not add screenshot if async beforeScreenshot returns false',
(tester) async {
// ignore: deprecated_member_use_from_same_package
fixture.options.beforeScreenshot =
(SentryEvent event, {Hint? hint}) async {
await Future<void>.delayed(Duration(milliseconds: 1));
return false;
};
await _addScreenshotAttachment(tester, FlutterRenderer.canvasKit,
added: false, isWeb: false);
});

testWidgets('does add screenshot if beforeScreenshot throws',
(tester) async {
fixture.options.automatedTestMode = false;
// ignore: deprecated_member_use_from_same_package
fixture.options.beforeScreenshot = (SentryEvent event, {Hint? hint}) {
throw Error();
};
await _addScreenshotAttachment(tester, FlutterRenderer.canvasKit,
added: true, isWeb: false);
});

testWidgets('does add screenshot if async beforeScreenshot throws',
(tester) async {
fixture.options.automatedTestMode = false;
// ignore: deprecated_member_use_from_same_package
fixture.options.beforeScreenshot =
(SentryEvent event, {Hint? hint}) async {
await Future<void>.delayed(Duration(milliseconds: 1));
throw Error();
};
await _addScreenshotAttachment(tester, FlutterRenderer.canvasKit,
added: true, isWeb: false);
});

testWidgets('passes event & hint to beforeScreenshot callback',
(tester) async {
SentryEvent? beforeScreenshotEvent;
Hint? beforeScreenshotHint;

// ignore: deprecated_member_use_from_same_package
fixture.options.beforeScreenshot = (SentryEvent event, {Hint? hint}) {
beforeScreenshotEvent = event;
beforeScreenshotHint = hint;
return true;
};

await _addScreenshotAttachment(tester, FlutterRenderer.canvasKit,
added: true, isWeb: false);

expect(beforeScreenshotEvent, event);
expect(beforeScreenshotHint, hint);
});
});

group('beforeCaptureScreenshot', () {
testWidgets('does add screenshot if beforeCapture returns true',
(tester) async {
Expand Down
4 changes: 2 additions & 2 deletions metrics/prepare.sh
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ flutterCreate() {
flutterCreate 'perf-test-app-plain'
flutterCreate 'perf-test-app-with-sentry'

# bump minSdkVersion to 19
# bump minSdkVersion to 21
gradleFile="$targetDir/perf-test-app-with-sentry/android/app/build.gradle"
sed "s/flutter.minSdkVersion/19/g" "$gradleFile" > "$gradleFile.new" && mv "$gradleFile.new" "$gradleFile"
sed "s/flutter.minSdkVersion/21/g" "$gradleFile" > "$gradleFile.new" && mv "$gradleFile.new" "$gradleFile"
Comment on lines +22 to +24
Copy link
Contributor

Choose a reason for hiding this comment

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

we can revert this as well right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Does not build with version 19, it seems most recent version of sentry-android does not support 19?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Bildschirmfoto 2025-02-06 um 11 10 52

Copy link
Contributor

Choose a reason for hiding this comment

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

ah yeah android bumped api from 19 to 21, let's leave it then. but we can do the minSDK bump separately


echo '::group::Patch perf-test-app-with-sentry'
pubspec="$targetDir/perf-test-app-with-sentry/pubspec_overrides.yaml"
Expand Down
Loading