-
-
Notifications
You must be signed in to change notification settings - Fork 276
Feat: Screenshots on reports #576
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,118 @@ | ||
| import 'dart:async'; | ||
| import 'dart:typed_data'; | ||
| import 'dart:ui' as ui show ImageByteFormat; | ||
|
|
||
| import 'package:flutter/material.dart'; | ||
| import 'package:flutter/rendering.dart'; | ||
| import 'package:flutter/scheduler.dart'; | ||
| import 'package:sentry/sentry.dart'; | ||
|
|
||
| /// Key which is used to identify the [RepaintBoundary] which gets captured | ||
| final _gloablKey = GlobalKey(debugLabel: 'sentry_screenshot'); | ||
|
|
||
| /// You can add screenshots of [child] to crash reports by adding this widget. | ||
| /// Ideally you are adding it around your app widget like in the following | ||
| /// example. | ||
| /// ```dart | ||
| /// runApp(SentryScreenshot(child: App())); | ||
| /// ``` | ||
| /// | ||
| /// Remarks: | ||
| /// - Depending on the place where it's used, you might have a transparent | ||
| /// background. | ||
| /// - Platform Views currently can't be captured. | ||
| /// - It only works on Flutters Canvas Kit Web renderer. For more information | ||
| /// see https://flutter.dev/docs/development/tools/web-renderers | ||
| /// - You can only have one [SentryScreenshot] widget in your widget tree at all | ||
| /// times. | ||
| class SentryScreenshot extends StatefulWidget { | ||
| const SentryScreenshot({Key? key, required this.child, this.hub}) | ||
| : super(key: key); | ||
|
|
||
| final Widget child; | ||
| final Hub? hub; | ||
|
|
||
| @override | ||
| _SentryScreenshotState createState() => _SentryScreenshotState(); | ||
| } | ||
|
|
||
| class _SentryScreenshotState extends State<SentryScreenshot> { | ||
| Hub get hub => widget.hub ?? HubAdapter(); | ||
|
|
||
| @override | ||
| Widget build(BuildContext context) { | ||
| return RepaintBoundary( | ||
| key: _gloablKey, | ||
| child: widget.child, | ||
| ); | ||
| } | ||
|
|
||
| @override | ||
| void initState() { | ||
| super.initState(); | ||
| hub.configureScope((scope) { | ||
| scope.addAttachment(ScreenshotAttachment()); | ||
| }); | ||
| } | ||
|
|
||
| @override | ||
| void dispose() { | ||
| hub.configureScope((scope) { | ||
| // The following doesn't work | ||
| // scope.attachements.remove(ScreenshotAttachment()); | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How are Attachments supposed to be removed?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Scope#clearAttachments()
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But that clears all Attachments, I just want to remove the Screenshot attachement.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yep, that's how it works for all the list, either you clean them all or just add a new one, and all of them are sent.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| }); | ||
| super.dispose(); | ||
| } | ||
| } | ||
|
|
||
| class ScreenshotAttachment implements SentryAttachment { | ||
| ScreenshotAttachment(); | ||
|
|
||
| @override | ||
| String attachmentType = SentryAttachment.typeAttachmentDefault; | ||
|
|
||
| @override | ||
| String? contentType = 'image/png'; | ||
|
|
||
| @override | ||
| String filename = 'screenshot.png'; | ||
|
|
||
| @override | ||
| bool addToTransactions = true; | ||
|
|
||
| @override | ||
| FutureOr<Uint8List> get bytes async { | ||
| //return await createScreenshot() ?? Uint8List.fromList([]); | ||
| final instance = SchedulerBinding.instance; | ||
| if (instance == null) { | ||
| return Uint8List.fromList([]); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe we allow returning null then? it does not make sense to return an array of nothing, unless we filter out before adding the attachment if the array is empty or not
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That would require all Attachments to allow returning null. Is that acceptable?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah not nice, but we should filter attachments with 0 bytes before adding to the envelope |
||
| } | ||
|
|
||
| final _completer = Completer<Uint8List?>(); | ||
| // We add an post frame callback because we aren't able to take a screenshot | ||
| // if there's currently a draw in process. | ||
| instance.addPostFrameCallback((timeStamp) async { | ||
| final image = await createScreenshot(); | ||
| _completer.complete(image); | ||
| }); | ||
| return await _completer.future ?? Uint8List.fromList([]); | ||
| } | ||
|
Comment on lines
+84
to
+99
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm pretty sure this is needed, though in a test it works without this. Maybe it also works on a device without this?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd say that we should keep the callback then, since the chances of an error happening during the draw process is quite high |
||
|
|
||
| @visibleForTesting | ||
| Future<Uint8List?> createScreenshot() async { | ||
| try { | ||
| final renderObject = _gloablKey.currentContext?.findRenderObject(); | ||
|
|
||
| if (renderObject is RenderRepaintBoundary) { | ||
| final image = await renderObject.toImage(pixelRatio: 1); | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add an option for this |
||
| // At the time of writing there's no other image format available which | ||
| // Sentry understands. | ||
| final bytes = await image.toByteData(format: ui.ImageByteFormat.png); | ||
| return bytes?.buffer.asUint8List(); | ||
| } | ||
| } catch (_) {} | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be logged somehow |
||
| return null; | ||
| } | ||
|
|
||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| import 'package:flutter/material.dart'; | ||
| import 'package:flutter_test/flutter_test.dart'; | ||
| import 'package:sentry_flutter/sentry_flutter.dart'; | ||
| import 'package:sentry_flutter/src/screenshot/sentry_screenshot.dart'; | ||
|
|
||
| void main() { | ||
| setUp(() { | ||
| TestWidgetsFlutterBinding.ensureInitialized(); | ||
| }); | ||
|
|
||
| test('ScreenshotAttachment default values', () { | ||
| final attachment = ScreenshotAttachment(); | ||
| expect(attachment.attachmentType, SentryAttachment.typeAttachmentDefault); | ||
| expect(attachment.contentType, 'image/png'); | ||
| expect(attachment.filename, 'screenshot.png'); | ||
| }); | ||
|
|
||
| testWidgets('creates screenshot', (tester) async { | ||
| await tester.runAsync(() async { | ||
| final widget = SentryScreenshot( | ||
| child: MaterialApp( | ||
| home: Scaffold( | ||
| body: Center( | ||
| child: Text('Hello World'), | ||
| ), | ||
| ), | ||
| ), | ||
| ); | ||
|
|
||
| await tester.pumpWidget(widget); | ||
|
|
||
| final screenshot = await ScreenshotAttachment().createScreenshot(); | ||
|
|
||
| expect(screenshot, isNotNull); | ||
| }); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's this issue: flutter/flutter#25306
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also this: flutter/flutter#83856
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is flutter/flutter#83856 a blocker?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't say so but it's definitly not nice. Most Flutter apps I've seen don't have a lot of platform views, so it should be fine for most users.