Skip to content
Open
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
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@

## Unreleased

### Features

- Add screenshot attachment button to the Android user feedback widget ([#5828](https://github.com/getsentry/sentry-java/pull/5828))
- Users can now attach a screenshot when submitting feedback. Enabled by default; can be disabled via `SentryFeedbackOptions.setEnableAttachScreenshot(false)` or the `io.sentry.feedback.enable-attach-screenshot` manifest flag.
- Requires the `androidx.activity` `>=1.8.2` dependency

### Improvements

- Skip building Android manifest metadata debug log messages when debug logging is disabled, reducing allocations during SDK init ([#5790](https://github.com/getsentry/sentry-java/pull/5790))
Expand Down
1 change: 1 addition & 0 deletions gradle/libs.versions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ apollo3-kotlin = { module = "com.apollographql.apollo3:apollo-runtime", version
apollo4-kotlin = { module = "com.apollographql.apollo:apollo-runtime", version = "4.1.1" }
androidx-appcompat = { module = "androidx.appcompat:appcompat", version = "1.3.0" }
androidx-annotation = { module = "androidx.annotation:annotation", version = "1.9.1" }
androidx-activity = { module = "androidx.activity:activity", version = "1.8.2" }
androidx-activity-compose = { module = "androidx.activity:activity-compose", version = "1.8.2" }
androidx-compose-foundation = { module = "androidx.compose.foundation:foundation", version.ref = "androidxCompose" }
androidx-compose-foundation-layout = { module = "androidx.compose.foundation:foundation-layout", version.ref = "androidxCompose" }
Expand Down
1 change: 1 addition & 0 deletions sentry-android-core/api/sentry-android-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,7 @@ public abstract interface class io/sentry/android/core/SentryUserFeedbackDialog$
public class io/sentry/android/core/SentryUserFeedbackForm : android/app/AlertDialog {
protected fun onCreate (Landroid/os/Bundle;)V
protected fun onStart ()V
protected fun onStop ()V
public fun setCancelable (Z)V
public fun setOnDismissListener (Landroid/content/DialogInterface$OnDismissListener;)V
public fun show ()V
Expand Down
4 changes: 4 additions & 0 deletions sentry-android-core/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,9 @@ dependencies {
implementation(libs.androidx.lifecycle.common.java8)
implementation(libs.androidx.lifecycle.process)
implementation(libs.androidx.core)
// photo picker for user feedback screenshot attachments
compileOnly(libs.androidx.activity)

implementation(libs.epitaph)

errorprone(libs.errorprone.core)
Expand All @@ -115,6 +118,7 @@ dependencies {
testImplementation(libs.androidx.test.ext.junit)
testImplementation(libs.androidx.test.runner)
testImplementation(libs.awaitility.kotlin)
testImplementation(libs.google.truth)
Comment thread
markushi marked this conversation as resolved.
testImplementation(libs.mockito.kotlin)
testImplementation(libs.mockito.inline)
testImplementation(projects.sentryTestSupport)
Expand Down
5 changes: 5 additions & 0 deletions sentry-android-core/proguard-rules.pro
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@
-dontwarn io.sentry.compose.gestures.ComposeGestureTargetLocator
-dontwarn io.sentry.compose.viewhierarchy.ComposeViewHierarchyExporter

# androidx.activity is a compileOnly dependency, used by the user feedback screenshot picker
# its presence is checked at runtime before use
-dontwarn androidx.activity.ComponentActivity
-dontwarn androidx.activity.result.**

# To ensure that stack traces is unambiguous
# https://developer.android.com/studio/build/shrink-code#decode-stack-trace
-keepattributes LineNumberTable,SourceFile
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,9 @@ final class ManifestMetadataReader {

static final String FEEDBACK_USE_SHAKE_GESTURE = "io.sentry.feedback.use-shake-gesture";

static final String FEEDBACK_ENABLE_ATTACH_SCREENSHOT =
"io.sentry.feedback.enable-attach-screenshot";

static final String SPOTLIGHT_ENABLE = "io.sentry.spotlight.enable";

static final String SPOTLIGHT_CONNECTION_URL = "io.sentry.spotlight.url";
Expand Down Expand Up @@ -723,6 +726,12 @@ static void applyMetadata(
feedbackOptions.setUseShakeGesture(
readBool(
metadata, logger, FEEDBACK_USE_SHAKE_GESTURE, feedbackOptions.isUseShakeGesture()));
feedbackOptions.setEnableAttachScreenshot(
readBool(
metadata,
logger,
FEEDBACK_ENABLE_ATTACH_SCREENSHOT,
feedbackOptions.isEnableAttachScreenshot()));

options.setStrictTraceContinuation(
readBool(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package io.sentry.android.core;

import android.app.Activity;
import android.net.Uri;
import androidx.activity.ComponentActivity;
import androidx.activity.result.ActivityResultLauncher;
import androidx.activity.result.PickVisualMediaRequest;
import androidx.activity.result.contract.ActivityResultContracts;
import io.sentry.SentryOptions;
import io.sentry.util.LoadClass;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

/**
* Launches the androidx photo picker to attach an screenshot to user feedback. All
* androidx.activity references are isolated in this class, so it must only be loaded after {@link
* #isAvailable} has returned true.
*/
final class SentryFeedbackScreenshotPicker {

interface OnScreenshotPicked {
void onScreenshotPicked(@NotNull Uri uri);
}

private final @NotNull ActivityResultLauncher<PickVisualMediaRequest> launcher;

private SentryFeedbackScreenshotPicker(
final @NotNull ActivityResultLauncher<PickVisualMediaRequest> launcher) {
this.launcher = launcher;
}

static boolean isAvailable(
final @NotNull LoadClass loadClass, final @NotNull SentryOptions options) {
return loadClass.isClassAvailable("androidx.activity.ComponentActivity", options)
&& loadClass.isClassAvailable(
"androidx.activity.result.contract.ActivityResultContracts$PickVisualMedia", options);
}

static @Nullable SentryFeedbackScreenshotPicker register(
final @NotNull Activity activity,
final @NotNull SentryFeedbackScreenshotPicker.OnScreenshotPicked callback) {
if (!(activity instanceof ComponentActivity)) {
return null;
}
final @NotNull ActivityResultLauncher<PickVisualMediaRequest> launcher =
((ComponentActivity) activity)
.getActivityResultRegistry()
.register(
"sentry_user_feedback_screenshot_picker",
new ActivityResultContracts.PickVisualMedia(),
(@Nullable Uri uri) -> {
if (uri != null) {
callback.onScreenshotPicked(uri);
}
});
return new SentryFeedbackScreenshotPicker(launcher);
}

void launch() {
launcher.launch(
new PickVisualMediaRequest.Builder()
.setMediaType(ActivityResultContracts.PickVisualMedia.ImageOnly.INSTANCE)
.build());
}

void unregister() {
launcher.unregister();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,24 @@
import android.app.Activity;
import android.app.AlertDialog;
import android.app.Application;
import android.content.ContentResolver;
import android.content.Context;
import android.content.ContextWrapper;
import android.database.Cursor;
import android.net.Uri;
import android.os.Bundle;
import android.provider.OpenableColumns;
import android.view.View;
import android.view.Window;
import android.view.WindowManager;
import android.webkit.MimeTypeMap;
import android.widget.Button;
import android.widget.EditText;
import android.widget.ImageView;
import android.widget.TextView;
import android.widget.Toast;
import io.sentry.Attachment;
import io.sentry.Hint;
import io.sentry.IScopes;
import io.sentry.Sentry;
import io.sentry.SentryFeedbackOptions;
Expand All @@ -23,6 +30,9 @@
import io.sentry.protocol.Feedback;
import io.sentry.protocol.SentryId;
import io.sentry.protocol.User;
import io.sentry.util.FileUtils;
import java.io.IOException;
import java.io.InputStream;
import java.lang.ref.WeakReference;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
Expand All @@ -39,6 +49,9 @@ public class SentryUserFeedbackForm extends AlertDialog {
private @Nullable SentryShakeDetector shakeDetector;
private @Nullable Application.ActivityLifecycleCallbacks shakeLifecycleCallbacks;

private @Nullable SentryFeedbackScreenshotPicker screenshotPicker;
private @Nullable Uri selectedImageUri;

SentryUserFeedbackForm(
final @NotNull Context context,
final int themeResId,
Expand Down Expand Up @@ -191,6 +204,29 @@ protected void onCreate(Bundle savedInstanceState) {
findViewById(R.id.sentry_dialog_user_feedback_edt_description);
final @NotNull Button btnSend = findViewById(R.id.sentry_dialog_user_feedback_btn_send);
final @NotNull Button btnCancel = findViewById(R.id.sentry_dialog_user_feedback_btn_cancel);
final @NotNull Button btnAddScreenshot =
findViewById(R.id.sentry_dialog_user_feedback_btn_add_screenshot);

// The button is made visible in onStart, once the screenshot picker is registered successfully
btnAddScreenshot.setVisibility(View.GONE);
btnAddScreenshot.setOnClickListener(
v -> {
if (selectedImageUri == null) {
if (screenshotPicker != null) {
try {
screenshotPicker.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.

gonna make the same comment that I did for the other usages of Throwable

Sentry.getCurrentScopes()
.getOptions()
.getLogger()
.log(SentryLevel.ERROR, "Failed to launch the screenshot picker.", t);
}
}
} else {
selectedImageUri = null;
btnAddScreenshot.setText(feedbackOptions.getAddScreenshotButtonLabel());
}
});

if (feedbackOptions.isShowBranding()) {
imgLogo.setVisibility(View.VISIBLE);
Expand Down Expand Up @@ -276,7 +312,9 @@ protected void onCreate(Bundle savedInstanceState) {
}

// Capture the feedback. If the ID is empty, it means that the feedback was not sent
final @NotNull SentryId id = Sentry.feedback().capture(feedback);
final @NotNull Hint hint = new Hint();
maybeAddImageAttachment(hint);
final @NotNull SentryId id = Sentry.feedback().capture(feedback, hint);
if (!id.equals(SentryId.EMPTY_ID)) {
Toast.makeText(
getContext(), feedbackOptions.getSuccessMessageText(), Toast.LENGTH_SHORT)
Expand Down Expand Up @@ -331,6 +369,7 @@ protected void onStart() {
edtMessage.setError(null);

final @NotNull SentryOptions options = Sentry.getCurrentScopes().getOptions();
maybeRegisterScreenshotPicker(options);
final @NotNull SentryFeedbackOptions feedbackOptions = options.getFeedbackOptions();
final @Nullable Runnable onFormOpen = feedbackOptions.getOnFormOpen();
if (onFormOpen != null) {
Expand All @@ -340,6 +379,125 @@ protected void onStart() {
currentReplayId = options.getReplayController().getReplayId();
}

@Override
protected void onStop() {
super.onStop();
if (screenshotPicker != null) {
screenshotPicker.unregister();
screenshotPicker = null;
}
}

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);
selectedImageUri = null;
btnAddScreenshot.setText(resolvedFeedbackOptions.getAddScreenshotButtonLabel());

if (!resolvedFeedbackOptions.isEnableAttachScreenshot()) {
return;
}
final @Nullable Activity activity = getActivity(getContext());
if (activity != null
&& SentryFeedbackScreenshotPicker.isAvailable(
resolvedFeedbackOptions.getLoadClass(), options)) {
screenshotPicker =
SentryFeedbackScreenshotPicker.register(
activity, uri -> onScreenshotPicked(options, btnAddScreenshot, uri));
}
if (screenshotPicker != null) {
btnAddScreenshot.setVisibility(View.VISIBLE);
} else {
btnAddScreenshot.setVisibility(View.GONE);
options
.getLogger()
.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.");
Comment on lines +415 to +418

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

}
}

private void onScreenshotPicked(
final @NotNull SentryOptions options,
final @NotNull Button btnAddScreenshot,
final @NotNull Uri uri) {
final long size = getUriSize(getContext().getContentResolver(), uri);
if (size > options.getMaxAttachmentSize()) {
Comment thread
sentry[bot] marked this conversation as resolved.
options
.getLogger()
.log(
SentryLevel.WARNING,
"Selected screenshot is larger than the maxAttachmentSize of %d bytes, dropping it.",
options.getMaxAttachmentSize());
Toast.makeText(
getContext(),
resolvedFeedbackOptions.getScreenshotTooLargeMessageText(),
Toast.LENGTH_SHORT)
.show();
return;
}
selectedImageUri = uri;
btnAddScreenshot.setText(resolvedFeedbackOptions.getRemoveScreenshotButtonLabel());
}

private void maybeAddImageAttachment(final @NotNull Hint hint) {
final @Nullable Uri imageUri = selectedImageUri;
if (imageUri == null) {
return;
}
try {
final @NotNull ContentResolver resolver = getContext().getContentResolver();
final @Nullable String resolvedMime = resolver.getType(imageUri);
final @NotNull String mime = resolvedMime != null ? resolvedMime : "image/png";
final @Nullable String resolvedExt =
MimeTypeMap.getSingleton().getExtensionFromMimeType(mime);
final @NotNull String ext = resolvedExt != null ? resolvedExt : "png";
hint.addAttachment(
new Attachment(
() ->
readUriBytes(
resolver,
imageUri,
Sentry.getCurrentScopes().getOptions().getMaxAttachmentSize()),
"screenshot." + ext,
mime,
"event.attachment",
false));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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)
Fix in CursorΒ Fix in Web

Reviewed by Cursor Bugbot for commit c8c08c1. Configure here.

} 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.

I decided I'm always going to comment whenever we add new usages of this. Sorry to bring it up again.

  1. How will we know that this feature is working in production if we swallow all errors?
  2. Since we're dealing with images, files and attachments how we we know we aren't swallowing a permissions issue or OOM?

Sentry.getCurrentScopes()
.getOptions()
.getLogger()
.log(SentryLevel.ERROR, "Failed to attach image to feedback.", t);
}
}

private static long getUriSize(final @NotNull ContentResolver resolver, final @NotNull Uri uri) {
try (final @Nullable Cursor cursor = resolver.query(uri, null, null, null, null)) {
if (cursor != null && cursor.moveToFirst()) {
final int sizeIndex = cursor.getColumnIndex(OpenableColumns.SIZE);
if (sizeIndex != -1 && !cursor.isNull(sizeIndex)) {
return cursor.getLong(sizeIndex);
}
}
} catch (Throwable ignored) {

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.

same comment here as above.

// if the size cannot be determined, let the attachment size limit handle it at capture time
}
return -1;
}

private static byte[] readUriBytes(
final @NotNull ContentResolver resolver, final @NotNull Uri uri, final long maxSize)
throws IOException {
try (final @Nullable InputStream inputStream = resolver.openInputStream(uri)) {
if (inputStream == null) {
throw new IOException("Unable to open image attachment: " + uri);
}
return FileUtils.inputStreamToByteArray(inputStream, maxSize);
}
}

@Override
public void show() {
// If Sentry is disabled, don't show the dialog, but log a warning
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,16 @@
android:paddingHorizontal="8dp"
android:layout_below="@id/sentry_dialog_user_feedback_txt_description" />

<Button
android:id="@+id/sentry_dialog_user_feedback_btn_add_screenshot"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginTop="8dp"
android:backgroundTint="?android:attr/colorBackground"
android:text="Add a screenshot"
android:visibility="gone"
android:layout_below="@id/sentry_dialog_user_feedback_edt_description" />

<Button
android:id="@+id/sentry_dialog_user_feedback_btn_send"
android:layout_width="match_parent"
Expand All @@ -102,7 +112,7 @@
android:textColor="?android:attr/textColorPrimaryInverse"
android:layout_marginTop="32dp"
android:text="Send Bug Report"
android:layout_below="@id/sentry_dialog_user_feedback_edt_description" />
android:layout_below="@id/sentry_dialog_user_feedback_btn_add_screenshot" />

<Button
android:id="@+id/sentry_dialog_user_feedback_btn_cancel"
Expand Down
Loading
Loading