feat(feedback): Add screenshot attachment button to user feedback widget#5828
feat(feedback): Add screenshot attachment button to user feedback widget#5828markushi wants to merge 10 commits into
Conversation
|
📲 Install BuildsAndroid
|
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 22f4345 | 307.87 ms | 354.51 ms | 46.64 ms |
| 8c7718c | 307.42 ms | 374.84 ms | 67.42 ms |
| bb0ff41 | 321.00 ms | 378.28 ms | 57.28 ms |
| c8125f3 | 397.65 ms | 485.14 ms | 87.49 ms |
| eb95ded | 317.51 ms | 369.08 ms | 51.57 ms |
| f634d01 | 375.06 ms | 420.04 ms | 44.98 ms |
| 9fbb112 | 359.71 ms | 421.85 ms | 62.14 ms |
| d501a7e | 307.33 ms | 341.94 ms | 34.61 ms |
| bb0ff41 | 317.76 ms | 384.66 ms | 66.90 ms |
| 9054d65 | 330.94 ms | 403.24 ms | 72.30 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 22f4345 | 1.58 MiB | 2.29 MiB | 719.83 KiB |
| 8c7718c | 0 B | 0 B | 0 B |
| bb0ff41 | 0 B | 0 B | 0 B |
| c8125f3 | 1.58 MiB | 2.10 MiB | 532.32 KiB |
| eb95ded | 0 B | 0 B | 0 B |
| f634d01 | 1.58 MiB | 2.10 MiB | 533.40 KiB |
| 9fbb112 | 1.58 MiB | 2.11 MiB | 539.18 KiB |
| d501a7e | 0 B | 0 B | 0 B |
| bb0ff41 | 0 B | 0 B | 0 B |
| 9054d65 | 1.58 MiB | 2.29 MiB | 723.38 KiB |
Previous results on branch: feat/feedback-screenshot-attachment
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 86e5409 | 326.20 ms | 351.31 ms | 25.11 ms |
| 676848b | 367.87 ms | 443.02 ms | 75.15 ms |
| 37c6869 | 327.68 ms | 381.06 ms | 53.38 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 86e5409 | 0 B | 0 B | 0 B |
| 676848b | 0 B | 0 B | 0 B |
| 37c6869 | 0 B | 0 B | 0 B |
…entry/sentry-java into feat/feedback-screenshot-attachment
runningcode
left a comment
There was a problem hiding this comment.
Thanks for adding this!
| mime, | ||
| "event.attachment", | ||
| false)); | ||
| } catch (Throwable t) { |
There was a problem hiding this comment.
I decided I'm always going to comment whenever we add new usages of this. Sorry to bring it up again.
- How will we know that this feature is working in production if we swallow all errors?
- Since we're dealing with images, files and attachments how we we know we aren't swallowing a permissions issue or OOM?
| return cursor.getLong(sizeIndex); | ||
| } | ||
| } | ||
| } catch (Throwable ignored) { |
There was a problem hiding this comment.
same comment here as above.
Use term "screenshot" everywhere Improve nullability handling
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c8c08c1. Configure here.
| "screenshot." + ext, | ||
| mime, | ||
| "event.attachment", | ||
| false)); |
There was a problem hiding this comment.
Lazy URI read may drop attachment
Medium Severity
The screenshot Attachment uses a lazy byteProvider that opens the photo-picker Uri later on the async transport/cache path. Picker URIs only grant temporary read access to the host activity, so if that access is gone before serialization, the attachment is dropped while feedback still reports success.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit c8c08c1. Configure here.
| .log( | ||
| SentryLevel.WARNING, | ||
| "Feedback screenshot button won't be shown. It requires the androidx.activity " | ||
| + "dependency and the feedback form being shown from a ComponentActivity."); |
There was a problem hiding this comment.
Bug: The maybeRegisterScreenshotPicker method lacks a guard to prevent re-registering the screenshot picker, which could cause a crash if onStart() is called multiple times.
Severity: MEDIUM
Suggested Fix
In maybeRegisterScreenshotPicker, add a null check to ensure screenshotPicker is not already initialized before calling SentryFeedbackScreenshotPicker.register(). For example: if (screenshotPicker == null) { ... }.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location:
sentry-android-core/src/main/java/io/sentry/android/core/SentryUserFeedbackForm.java#L415-L418
Potential issue: In `SentryUserFeedbackForm`, the `maybeRegisterScreenshotPicker` method
is called from `onStart()` to register an activity result launcher. This registration
uses a hardcoded key. The method lacks a defensive check to see if the
`screenshotPicker` has already been initialized. While the `onStop()` method unregisters
the launcher, it's possible in certain edge-case lifecycle scenarios for `onStart()` to
be invoked twice without an intervening `onStop()`. This would lead to an attempt to
register the same key again, causing an `IllegalStateException` and crashing the app.


📜 Description
Adds an "Add a screenshot" button to the Android user feedback form that opens the
androidx photo picker (
ActivityResultContracts.PickVisualMedia, images only) and sendsthe picked image as an attachment in the same envelope as the feedback.
Screen_recording_20260723_183244.mp4
SentryFeedbackOptions:enableScreenshot(defaulttrue, matching the JS SDK),addScreenshotButtonLabel,removeScreenshotButtonLabel, plus theio.sentry.feedback.enable-screenshotmanifest flagandroidx.activity:activity:1.8.2is added as compileOnly — no new transitivedependency; the button is hidden (with a warning log) when androidx.activity is missing
or the host Activity is not a
ComponentActivitymaxAttachmentSizeat pick time💡 Motivation and Context
💚 How did you test it?
Unit tests + Manual testing.
📝 Checklist
sendDefaultPIIis enabled.