Skip to content

feat(feedback): Add screenshot attachment button to user feedback widget - #5828

Open
markushi wants to merge 19 commits into
mainfrom
feat/feedback-screenshot-attachment
Open

feat(feedback): Add screenshot attachment button to user feedback widget#5828
markushi wants to merge 19 commits into
mainfrom
feat/feedback-screenshot-attachment

Conversation

@markushi

@markushi markushi commented Jul 23, 2026

Copy link
Copy Markdown
Member

📜 Description

Adds an "Add a screenshot" button to the Android user feedback form that opens the
androidx photo picker (ActivityResultContracts.PickVisualMedia, images only) and sends
the picked image as an attachment in the same envelope as the feedback.

Screen_recording_20260813_202418.mp4
image
  • New SentryFeedbackOptions: enableAttachScreenshot (default true, matching the JS SDK),
    addScreenshotButtonLabel, removeScreenshotButtonLabel, screenshotTooLargeMessageText, plus the
    io.sentry.feedback.enable-attach-screenshot manifest flag
  • androidx.activity:activity:1.8.2 is added as compileOnly — no new transitive
    dependency; the button is hidden (with a warning log) when androidx.activity is missing
    or the host Activity is not a ComponentActivity
  • The image is size-checked against maxAttachmentSize at pick time

Fixed along the way

The form was not scrollable, so in landscape or at large font scales the send and cancel
buttons were pushed off screen and the dialog could not be dismissed. It is now wrapped in
a ScrollView, and the three full width buttons were replaced with the framework dialog
button bar (cancel and send in a right aligned bar, screenshot as a borderless full width
action). No changelog entry, as it is bundled with this feature.

💡 Motivation and Context

💚 How did you test it?

Unit tests + Manual testing.

📝 Checklist

  • I added GH Issue ID & Linear ID
  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.
  • Public API changes reviewed by another Mobile SDK team member or implemented according to the develop docs spec.

@linear-code

linear-code Bot commented Jul 23, 2026

Copy link
Copy Markdown

JAVA-538

@github-actions

github-actions Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor
Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against 82e8007

@sentry

sentry Bot commented Jul 23, 2026

Copy link
Copy Markdown

📲 Install Builds

Android

🔗 App Name App ID Version Configuration
SDK Size io.sentry.tests.size 8.53.0 (1) release

⚙️ sentry-android Build Distribution Settings

@github-actions

github-actions Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 355.46 ms 419.13 ms 63.67 ms
Size 0 B 0 B 0 B

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
fc5ccaf 279.11 ms 353.34 ms 74.23 ms
96eeafa 361.43 ms 455.07 ms 93.63 ms
44472da 324.77 ms 360.60 ms 35.83 ms
694d587 305.45 ms 378.38 ms 72.94 ms
694d587 379.62 ms 400.80 ms 21.18 ms
bb0ff41 321.00 ms 378.28 ms 57.28 ms
0d66c0b 327.83 ms 403.40 ms 75.57 ms
e2dce0b 308.96 ms 360.10 ms 51.14 ms
ce0a49e 532.00 ms 609.96 ms 77.96 ms
ad8da22 365.86 ms 427.00 ms 61.14 ms

App size

Revision Plain With Sentry Diff
fc5ccaf 1.58 MiB 2.13 MiB 557.54 KiB
96eeafa 1.58 MiB 2.19 MiB 620.21 KiB
44472da 0 B 0 B 0 B
694d587 1.58 MiB 2.19 MiB 620.06 KiB
694d587 1.58 MiB 2.19 MiB 620.06 KiB
bb0ff41 0 B 0 B 0 B
0d66c0b 1.58 MiB 2.29 MiB 720.89 KiB
e2dce0b 0 B 0 B 0 B
ce0a49e 1.58 MiB 2.10 MiB 532.94 KiB
ad8da22 1.58 MiB 2.29 MiB 719.83 KiB

Previous results on branch: feat/feedback-screenshot-attachment

Startup times

Revision Plain With Sentry Diff
37c6869 327.68 ms 381.06 ms 53.38 ms
86e5409 326.20 ms 351.31 ms 25.11 ms
f26f131 316.70 ms 333.33 ms 16.63 ms
71243e7 313.69 ms 370.22 ms 56.54 ms
676848b 367.87 ms 443.02 ms 75.15 ms

App size

Revision Plain With Sentry Diff
37c6869 0 B 0 B 0 B
86e5409 0 B 0 B 0 B
f26f131 0 B 0 B 0 B
71243e7 0 B 0 B 0 B
676848b 0 B 0 B 0 B

@markushi
markushi marked this pull request as ready for review July 23, 2026 17:27
Comment thread sentry/src/main/java/io/sentry/util/FileUtils.java
Comment thread sentry/src/main/java/io/sentry/util/FileUtils.java Outdated

@runningcode runningcode left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for adding this!

Comment thread sentry/src/main/java/io/sentry/util/FileUtils.java Outdated
Comment thread sentry-android-core/build.gradle.kts
Comment thread sentry/src/main/java/io/sentry/SentryFeedbackOptions.java Outdated
Comment thread sentry/src/main/java/io/sentry/SentryFeedbackOptions.java Outdated
markushi added 2 commits July 24, 2026 10:39
Use term "screenshot" everywhere
Improve nullability handling
markushi and others added 2 commits August 12, 2026 12:48
…hment paths

Routes the three catch-all blocks around the screenshot picker through
ExceptionUtils.rethrowIfFatal, so OOM, ThreadDeath and linkage errors
propagate instead of being hidden, and logs the previously silent
failure in getUriSize.

Also moves the androidx.activity availability check out of
SentryFeedbackScreenshotPicker, so the class is only loaded once the
check passed, as its javadoc requires.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
markushi and others added 2 commits August 13, 2026 12:52
… buttons

The form was not scrollable, so in landscape or at large font scales the
send and cancel buttons were pushed off screen and the dialog could not
be dismissed. Wraps it in a ScrollView.

Also replaces the three full width buttons with the framework dialog
button bar: cancel and send sit in a right aligned bar, and the
screenshot button becomes a borderless full width action.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…istration

ActivityResultRegistry.registerKey returns early for a key that is already
registered, so re-registering would not have thrown.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@markushi
markushi requested a review from runningcode August 13, 2026 18:25
@markushi markushi added the sanity-check PR needs a lightweight review for obvious issues label Aug 13, 2026
markushi and others added 2 commits August 13, 2026 20:54
…hot-attachment

# Conflicts:
#	CHANGELOG.md
#	sentry-android-core/src/main/java/io/sentry/android/core/SentryUserFeedbackForm.java
#	sentry-android-core/src/test/java/io/sentry/android/core/SentryUserFeedbackFormTest.kt
#	sentry/api/sentry.api
#	sentry/src/main/java/io/sentry/SentryFeedbackOptions.java
#	sentry/src/main/java/io/sentry/SentryOptions.java
#	sentry/src/test/java/io/sentry/SentryFeedbackOptionsTest.kt
…ed types

AGENTS.md now forbids new catch (Throwable). Replaces the three broad
catches with the types the guarded code can actually throw:

- launch() documents ActivityNotFoundException and the registry throws
  IllegalStateException for an unregistered launcher
- the ContentResolver calls cross into the provider process, where Binder
  can only deliver the exceptions Parcel marshals, all RuntimeException

Fatal throwables now propagate because they are never caught, so
ExceptionUtils.rethrowIfFatal is no longer needed here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 5edad4a. Configure here.

callback.onScreenshotPicked(uri);
}
});
return new SentryFeedbackScreenshotPicker(launcher);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Shared picker key breaks stacked forms

Medium Severity

The screenshot picker always registers on the host ActivityResultRegistry under the fixed key sentry_user_feedback_screenshot_picker. A second SentryUserFeedbackForm on the same ComponentActivity overwrites that registration. Dismissing the first form then calls unregister() on the shared key and tears down the still-visible form's launcher, so its add-screenshot action fails.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 5edad4a. Configure here.

androidx.activity is a compileOnly dependency, so the version in the app's
apk is outside our control and may be missing the photo picker APIs this
was compiled against. Catches LinkageError locally at the two call sites
that link against it, then falls back to the sanctioned
ExceptionUtils.rethrowIfFatal pattern, as ExceptionUtils documents and
SentrySQLiteDriver.hasConnectionPool demonstrates.

Registration is now guarded too, so a version mismatch hides the
screenshot button instead of breaking the whole form.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comment on lines 415 to +417

final @NotNull SentryOptions options = Sentry.getCurrentScopes().getOptions();
maybeRegisterScreenshotPicker(options);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The screenshot button remains visible after being disabled. If isEnableAttachScreenshot is set to false between form displays, the button's visibility is not updated, leaving it incorrectly visible.
Severity: LOW

Suggested Fix

In the maybeRegisterScreenshotPicker() method, ensure the button's visibility is always set regardless of the isEnableAttachScreenshot() value. When disabled, explicitly set btnAddScreenshot.setVisibility(View.GONE) before the early return to correctly reflect the current configuration.

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-L417

Potential issue: The visibility of the 'attach screenshot' button is not correctly
updated if the `SentryFeedbackOptions.isEnableAttachScreenshot()` setting is changed
while the feedback form is not visible. If the form is shown with screenshots enabled,
then hidden, and then the setting is disabled, the button will incorrectly remain
visible when the form is shown again. This happens because the
`maybeRegisterScreenshotPicker()` method has an early return when the feature is
disabled, which skips the logic that would hide the button. The button's visibility
state from the previous showing persists.

Comment on lines 415 to +417

final @NotNull SentryOptions options = Sentry.getCurrentScopes().getOptions();
maybeRegisterScreenshotPicker(options);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The screenshot button remains visible after being disabled. If isEnableAttachScreenshot is set to false between form displays, the button's visibility is not updated, leaving it incorrectly visible.
Severity: LOW

Suggested Fix

In the maybeRegisterScreenshotPicker() method, ensure the button's visibility is always set regardless of the isEnableAttachScreenshot() value. When disabled, explicitly set btnAddScreenshot.setVisibility(View.GONE) before the early return to correctly reflect the current configuration.

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-L417

Potential issue: The visibility of the 'attach screenshot' button is not correctly
updated if the `SentryFeedbackOptions.isEnableAttachScreenshot()` setting is changed
while the feedback form is not visible. If the form is shown with screenshots enabled,
then hidden, and then the setting is disabled, the button will incorrectly remain
visible when the form is shown again. This happens because the
`maybeRegisterScreenshotPicker()` method has an early return when the feature is
disabled, which skips the logic that would hide the button. The button's visibility
state from the previous showing persists.

@runningcode runningcode left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

looks better! i think we can simplify the stuff around calling the picker!

if (photoPicker != null) {
try {
photoPicker.launch();
} catch (Throwable t) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the fix! Do we know what the exception that is thrown when the device doesn't have a screenshot picker? I would favor just catching that one but I'll accept this as well.

private void maybeRegisterScreenshotPicker(final @NotNull SentryOptions options) {
// Clear any previously selected image so subsequent show() calls start with a fresh form
final @NotNull Button btnAddScreenshot =
findViewById(R.id.sentry_dialog_user_feedback_btn_add_screenshot);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i feel i should mention that using this view system is considered deprecated by google now 😢

*
* @return true if the attach screenshot button is shown
*/
public boolean isEnableAttachScreenshot() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If we prefix with get instead of is we get a nicer synthetic getter in Kotlin like attachScreenshotEnabled

Suggested change
public boolean isEnableAttachScreenshot() {
public boolean getAttachScreenshotEnabled() {

SentryFeedbackScreenshotPicker.register(
activity, uri -> onScreenshotPicked(options, btnAddScreenshot, uri));
} catch (LinkageError e) {
// This is where androidx.activity is linked for the first time. It is a compileOnly

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we check this in isScreenshotPickerAvailable ?

if (selectedImageUri == null) {
if (screenshotPicker != null) {
try {
screenshotPicker.launch();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The docs for ScreenShotPicker suggest we should only load the class once SentryUserFeedbackForm#isScreenshotPickerAvailable returns true and we do that. So it seems to me that it can only be not null if we already know the class is available so I'm wondering why we have to wrapp all the calls in try catch?

My suggestion to improve this would be can we find a way to check all the prerequisites beforehand so we don't have to wrap every call with a complicated try/catch block?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Whilst the class could be available, we have no control of the actual version of the library, as it's app defined. So in theory a breaking API change / or change in behavior could cause a crash here.

final @NotNull InputStream input, final long maxLength)
throws IOException, SecurityException {

try (final BufferedInputStream inputStream = new BufferedInputStream(input);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The stream from ContentResolver.openInputStream() in readUriBytes is closed twice, which may throw an exception if the underlying stream implementation doesn't handle double-closing gracefully, failing feedback submission.
Severity: MEDIUM

Suggested Fix

Prevent the double-close of the stream. One approach is to remove the try-with-resources block in readUriBytes and rely solely on inputStreamToByteArray to manage and close the stream. This delegates the responsibility of closing the stream to the utility method that consumes it.

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/src/main/java/io/sentry/util/FileUtils.java#L138

Potential issue: In `readUriBytes`, a stream is obtained from
`ContentResolver.openInputStream(uri)` and managed by a try-with-resources block. This
stream is then passed to `inputStreamToByteArray`, which also wraps the stream in its
own try-with-resources block using a `BufferedInputStream`. Consequently, the stream is
closed once when `inputStreamToByteArray` completes, and then a second time when the
`readUriBytes` try-with-resources block completes. The stream returned by
`ContentResolver.openInputStream()` is not guaranteed to have an idempotent `close()`
method. This double-close can throw an exception, causing the user feedback submission
to fail when an image is attached.

Also affects:

  • sentry/src/main/java/io/sentry/util/FileUtils.java:118~121
  • sentry-android-core/src/main/java/io/sentry/android/core/SentryUserFeedbackForm.java:587~592

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sanity-check PR needs a lightweight review for obvious issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add file attachment support to user feedback widget

2 participants