From 8682a9ece019cab72739ecf38456eea9def6bedb Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Thu, 22 Dec 2022 11:20:57 +0100 Subject: [PATCH 01/24] Add intial implemention for Android View Hierarchy support --- .../api/sentry-android-core.api | 7 + .../core/AndroidOptionsInitializer.java | 2 + .../android/core/ManifestMetadataReader.java | 4 + .../android/core/SentryAndroidOptions.java | 11 + .../core/ViewHierarchyEventProcessor.java | 121 ++++++++++ .../internal/gestures/ViewTargetSelector.java | 25 --- .../core/internal/gestures/ViewUtils.java | 6 +- .../core/ManifestMetadataReaderTest.kt | 13 ++ .../src/main/AndroidManifest.xml | 3 + sentry/api/sentry.api | 63 ++++++ .../src/main/java/io/sentry/Attachment.java | 83 ++++++- sentry/src/main/java/io/sentry/Hint.java | 9 + .../main/java/io/sentry/JsonSerializer.java | 4 + .../src/main/java/io/sentry/SentryClient.java | 5 + .../io/sentry/protocol/ViewHierarchy.java | 98 +++++++++ .../io/sentry/protocol/ViewHierarchyNode.java | 208 ++++++++++++++++++ .../ViewHierarchyNodeSerializationTest.kt | 78 +++++++ .../ViewHierarchySerializationTest.kt | 64 ++++++ .../test/resources/json/view_hierarchy.json | 6 + .../resources/json/view_hierarchy_node.json | 17 ++ 20 files changed, 799 insertions(+), 28 deletions(-) create mode 100644 sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java delete mode 100644 sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/ViewTargetSelector.java create mode 100644 sentry/src/main/java/io/sentry/protocol/ViewHierarchy.java create mode 100644 sentry/src/main/java/io/sentry/protocol/ViewHierarchyNode.java create mode 100644 sentry/src/test/java/io/sentry/protocol/ViewHierarchyNodeSerializationTest.kt create mode 100644 sentry/src/test/java/io/sentry/protocol/ViewHierarchySerializationTest.kt create mode 100644 sentry/src/test/resources/json/view_hierarchy.json create mode 100644 sentry/src/test/resources/json/view_hierarchy_node.json diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index 05316fa0d4..37f2a97482 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -152,6 +152,7 @@ public final class io/sentry/android/core/SentryAndroidOptions : io/sentry/Sentr public fun isAnrEnabled ()Z public fun isAnrReportInDebug ()Z public fun isAttachScreenshot ()Z + public fun isAttachViewHierarchy ()Z public fun isCollectAdditionalContext ()Z public fun isEnableActivityLifecycleBreadcrumbs ()Z public fun isEnableActivityLifecycleTracingAutoFinish ()Z @@ -164,6 +165,7 @@ public final class io/sentry/android/core/SentryAndroidOptions : io/sentry/Sentr public fun setAnrReportInDebug (Z)V public fun setAnrTimeoutIntervalMillis (J)V public fun setAttachScreenshot (Z)V + public fun setAttachViewHierarchy (Z)V public fun setCollectAdditionalContext (Z)V public fun setDebugImagesLoader (Lio/sentry/android/core/IDebugImagesLoader;)V public fun setEnableActivityLifecycleBreadcrumbs (Z)V @@ -235,6 +237,11 @@ public final class io/sentry/android/core/UserInteractionIntegration : android/a public fun register (Lio/sentry/IHub;Lio/sentry/SentryOptions;)V } +public final class io/sentry/android/core/ViewHierarchyEventProcessor : io/sentry/EventProcessor { + public fun (Lio/sentry/android/core/SentryAndroidOptions;)V + public fun process (Lio/sentry/SentryEvent;Lio/sentry/Hint;)Lio/sentry/SentryEvent; +} + public final class io/sentry/android/core/cache/AndroidEnvelopeCache : io/sentry/cache/EnvelopeCache { public fun (Lio/sentry/android/core/SentryAndroidOptions;)V public fun getDirectory ()Ljava/io/File; diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java index 620fbed05c..1a716af418 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java @@ -217,6 +217,8 @@ private static void installDefaultIntegrations( SentryLevel.WARNING, "ActivityLifecycle, FragmentLifecycle and UserInteraction Integrations need an Application class to be installed."); } + options.addEventProcessor(new ViewHierarchyEventProcessor(options)); + if (isTimberAvailable) { options.addIntegration(new SentryTimberIntegration()); } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java b/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java index d05560d792..60bd3e338e 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java @@ -75,6 +75,7 @@ final class ManifestMetadataReader { static final String IDLE_TIMEOUT = "io.sentry.traces.idle-timeout"; static final String ATTACH_SCREENSHOT = "io.sentry.attach-screenshot"; + static final String ATTACH_VIEW_HIERARCHY = "io.sentry.attach-view-hierarchy"; static final String CLIENT_REPORTS_ENABLE = "io.sentry.send-client-reports"; static final String COLLECT_ADDITIONAL_CONTEXT = "io.sentry.additional-context"; @@ -220,6 +221,9 @@ static void applyMetadata( options.setAttachScreenshot( readBool(metadata, logger, ATTACH_SCREENSHOT, options.isAttachScreenshot())); + options.setAttachViewHierarchy( + readBool(metadata, logger, ATTACH_VIEW_HIERARCHY, options.isAttachViewHierarchy())); + options.setSendClientReports( readBool(metadata, logger, CLIENT_REPORTS_ENABLE, options.isSendClientReports())); diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java index ca1f9a03c1..03a697d13d 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java @@ -96,6 +96,9 @@ public final class SentryAndroidOptions extends SentryOptions { /** Enables or disables the attach screenshot feature when an error happened. */ private boolean attachScreenshot; + /** Enables or disables the attach view hierarchy feature when an error happened. */ + private boolean attachViewHierarchy; + /** * Enables or disables collecting of device information which requires Inter-Process Communication * (IPC) @@ -329,6 +332,14 @@ public void setAttachScreenshot(boolean attachScreenshot) { this.attachScreenshot = attachScreenshot; } + public boolean isAttachViewHierarchy() { + return attachViewHierarchy; + } + + public void setAttachViewHierarchy(boolean attachViewHierarchy) { + this.attachViewHierarchy = attachViewHierarchy; + } + public boolean isCollectAdditionalContext() { return collectAdditionalContext; } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java new file mode 100644 index 0000000000..7508b96161 --- /dev/null +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java @@ -0,0 +1,121 @@ +package io.sentry.android.core; + +import android.app.Activity; +import android.view.View; +import android.view.ViewGroup; +import io.sentry.Attachment; +import io.sentry.EventProcessor; +import io.sentry.Hint; +import io.sentry.ILogger; +import io.sentry.SentryEvent; +import io.sentry.SentryLevel; +import io.sentry.android.core.internal.gestures.ViewUtils; +import io.sentry.protocol.ViewHierarchy; +import io.sentry.protocol.ViewHierarchyNode; +import io.sentry.util.Objects; +import java.util.ArrayList; +import java.util.List; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +/** ViewHierarchyEventProcessor responsible for taking a snapshot of the current view hierarchy. */ +@ApiStatus.Internal +public final class ViewHierarchyEventProcessor implements EventProcessor { + + private final @NotNull SentryAndroidOptions options; + + public ViewHierarchyEventProcessor(final @NotNull SentryAndroidOptions options) { + this.options = Objects.requireNonNull(options, "SentryAndroidOptions is required"); + } + + @Override + public @NotNull SentryEvent process(final @NotNull SentryEvent event, @NotNull Hint hint) { + if (!event.isErrored()) { + return event; + } + + if (!options.isAttachViewHierarchy()) { + this.options.getLogger().log(SentryLevel.DEBUG, "attachViewHierarchy is disabled."); + return event; + } + + final Activity activity = CurrentActivityHolder.getInstance().getActivity(); + if (activity == null) { + return event; + } + + final ViewHierarchy viewHierarchy = snapshotViewHierarchy(activity, options.getLogger()); + if (viewHierarchy != null) { + hint.setViewHierarchy(Attachment.fromViewHierarchy(viewHierarchy)); + } + + return event; + } + + @Nullable + private static ViewHierarchy snapshotViewHierarchy(Activity activity, ILogger logger) { + final List windows = new ArrayList<>(); + final ViewHierarchy viewHierarchy = new ViewHierarchy("android_view_system", windows); + + final @Nullable View decorView = activity.getWindow().peekDecorView(); + if (decorView == null) { + return viewHierarchy; + } + + final @NotNull ViewHierarchyNode decorNode = viewToNode(decorView); + windows.add(decorNode); + addChildren(decorView, decorNode); + + return viewHierarchy; + } + + private static void addChildren(View view, ViewHierarchyNode viewNode) { + if (!(view instanceof ViewGroup)) { + return; + } + + final @NotNull ViewGroup viewGroup = ((ViewGroup) view); + final int childCount = viewGroup.getChildCount(); + if (childCount == 0) { + return; + } + + final @NotNull List childNodes = new ArrayList<>(childCount); + for (int i = 0; i < childCount; i++) { + final @Nullable View child = viewGroup.getChildAt(i); + if (child != null) { + final @NotNull ViewHierarchyNode childNode = viewToNode(child); + childNodes.add(childNode); + addChildren(child, childNode); + } + } + viewNode.setChildren(childNodes); + } + + @NotNull + private static ViewHierarchyNode viewToNode(final View view) { + @NotNull final ViewHierarchyNode node = new ViewHierarchyNode(); + + @Nullable String className = view.getClass().getCanonicalName(); + if (className == null) { + className = view.getClass().getSimpleName(); + } + node.setType(className); + + try { + final String identifier = ViewUtils.getResourceId(view); + node.setIdentifier(identifier); + } catch (Exception e) { + // ignored + } + node.setX((double) view.getX()); + node.setY((double) view.getY()); + node.setWidth((double) view.getWidth()); + node.setHeight((double) view.getHeight()); + node.setAlpha((double) view.getAlpha()); + node.setVisible(view.getVisibility() == View.VISIBLE); + + return node; + } +} diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/ViewTargetSelector.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/ViewTargetSelector.java deleted file mode 100644 index d2da63ada9..0000000000 --- a/sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/ViewTargetSelector.java +++ /dev/null @@ -1,25 +0,0 @@ -package io.sentry.android.core.internal.gestures; - -import android.view.View; -import org.jetbrains.annotations.NotNull; - -interface ViewTargetSelector { - /** - * Defines whether the given {@code view} should be selected from the view hierarchy. - * - * @param view - the view to be selected. - * @return true, when the view should be selected, false otherwise. - */ - boolean select(@NotNull View view); - - /** - * Defines whether the view from the select method is eligible for children traversal, in case - * it's a ViewGroup. - * - * @return true, when the ViewGroup is sufficient to be selected and children traversal is not - * necessary. - */ - default boolean skipChildren() { - return false; - } -} diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/ViewUtils.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/ViewUtils.java index 68360451c9..348d552559 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/ViewUtils.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/ViewUtils.java @@ -10,10 +10,11 @@ import io.sentry.util.Objects; import java.util.LinkedList; import java.util.Queue; +import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -final class ViewUtils { +public final class ViewUtils { /** * Finds a target view, that has been selected/clicked by the given coordinates x and y and the @@ -85,7 +86,8 @@ static String getResourceIdWithFallback(final @NotNull View view) { * @return human-readable view id * @throws Resources.NotFoundException in case the view id was not found */ - static String getResourceId(final @NotNull View view) throws Resources.NotFoundException { + @ApiStatus.Internal + public static String getResourceId(final @NotNull View view) throws Resources.NotFoundException { final int viewId = view.getId(); final Resources resources = view.getContext().getResources(); String resourceId = ""; diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt index 9835a380f4..a99bee37d6 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt @@ -949,6 +949,19 @@ class ManifestMetadataReaderTest { assertTrue(fixture.options.isAttachScreenshot) } + @Test + fun `applyMetadata reads attach viewhierarchy to options`() { + // Arrange + val bundle = bundleOf(ManifestMetadataReader.ATTACH_VIEW_HIERARCHY to true) + val context = fixture.getContext(metaData = bundle) + + // Act + ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider) + + // Assert + assertTrue(fixture.options.isAttachViewHierarchy) + } + @Test fun `applyMetadata reads attach screenshots and keep default value if not found`() { // Arrange diff --git a/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml b/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml index 9e82bdc8b3..ebe16ed7b5 100644 --- a/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml +++ b/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml @@ -135,6 +135,9 @@ + + + diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index db6c9eeb6f..318f2e508c 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -7,12 +7,15 @@ public final class io/sentry/Attachment { public fun (Ljava/lang/String;)V public fun (Ljava/lang/String;Ljava/lang/String;)V public fun (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)V + public fun (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Z)V public fun (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Z)V public fun (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ZLjava/lang/String;)V public fun ([BLjava/lang/String;)V public fun ([BLjava/lang/String;Ljava/lang/String;)V + public fun ([BLjava/lang/String;Ljava/lang/String;Ljava/lang/String;Z)V public fun ([BLjava/lang/String;Ljava/lang/String;Z)V public static fun fromScreenshot ([B)Lio/sentry/Attachment; + public static fun fromViewHierarchy (Lio/sentry/protocol/ViewHierarchy;)Lio/sentry/Attachment; public fun getAttachmentType ()Ljava/lang/String; public fun getBytes ()[B public fun getContentType ()Ljava/lang/String; @@ -262,10 +265,12 @@ public final class io/sentry/Hint { public fun getAs (Ljava/lang/String;Ljava/lang/Class;)Ljava/lang/Object; public fun getAttachments ()Ljava/util/List; public fun getScreenshot ()Lio/sentry/Attachment; + public fun getViewHierarchy ()Lio/sentry/Attachment; public fun remove (Ljava/lang/String;)V public fun replaceAttachments (Ljava/util/List;)V public fun set (Ljava/lang/String;Ljava/lang/Object;)V public fun setScreenshot (Lio/sentry/Attachment;)V + public fun setViewHierarchy (Lio/sentry/Attachment;)V public static fun withAttachment (Lio/sentry/Attachment;)Lio/sentry/Hint; public static fun withAttachments (Ljava/util/List;)Lio/sentry/Hint; } @@ -3339,6 +3344,64 @@ public final class io/sentry/protocol/User$JsonKeys { public fun ()V } +public final class io/sentry/protocol/ViewHierarchy : io/sentry/JsonSerializable, io/sentry/JsonUnknown { + public fun (Ljava/lang/String;Ljava/util/List;)V + public fun getUnknown ()Ljava/util/Map; + public fun serialize (Lio/sentry/JsonObjectWriter;Lio/sentry/ILogger;)V + public fun setUnknown (Ljava/util/Map;)V +} + +public final class io/sentry/protocol/ViewHierarchy$Deserializer : io/sentry/JsonDeserializer { + public fun ()V + public fun deserialize (Lio/sentry/JsonObjectReader;Lio/sentry/ILogger;)Lio/sentry/protocol/ViewHierarchy; + public synthetic fun deserialize (Lio/sentry/JsonObjectReader;Lio/sentry/ILogger;)Ljava/lang/Object; +} + +public final class io/sentry/protocol/ViewHierarchy$JsonKeys { + public static final field RENDERING_SYSTEM Ljava/lang/String; + public static final field WINDOWS Ljava/lang/String; + public fun ()V +} + +public final class io/sentry/protocol/ViewHierarchyNode : io/sentry/JsonSerializable, io/sentry/JsonUnknown { + public fun ()V + public fun getUnknown ()Ljava/util/Map; + public fun serialize (Lio/sentry/JsonObjectWriter;Lio/sentry/ILogger;)V + public fun setAlpha (Ljava/lang/Double;)V + public fun setChildren (Ljava/util/List;)V + public fun setHeight (Ljava/lang/Double;)V + public fun setIdentifier (Ljava/lang/String;)V + public fun setRenderingSystem (Ljava/lang/String;)V + public fun setTag (Ljava/lang/String;)V + public fun setType (Ljava/lang/String;)V + public fun setUnknown (Ljava/util/Map;)V + public fun setVisible (Ljava/lang/Boolean;)V + public fun setWidth (Ljava/lang/Double;)V + public fun setX (Ljava/lang/Double;)V + public fun setY (Ljava/lang/Double;)V +} + +public final class io/sentry/protocol/ViewHierarchyNode$Deserializer : io/sentry/JsonDeserializer { + public fun ()V + public fun deserialize (Lio/sentry/JsonObjectReader;Lio/sentry/ILogger;)Lio/sentry/protocol/ViewHierarchyNode; + public synthetic fun deserialize (Lio/sentry/JsonObjectReader;Lio/sentry/ILogger;)Ljava/lang/Object; +} + +public final class io/sentry/protocol/ViewHierarchyNode$JsonKeys { + public static final field ALPHA Ljava/lang/String; + public static final field CHILDREN Ljava/lang/String; + public static final field HEIGHT Ljava/lang/String; + public static final field IDENTIFIER Ljava/lang/String; + public static final field RENDERING_SYSTEM Ljava/lang/String; + public static final field TAG Ljava/lang/String; + public static final field TYPE Ljava/lang/String; + public static final field VISIBLE Ljava/lang/String; + public static final field WIDTH Ljava/lang/String; + public static final field X Ljava/lang/String; + public static final field Y Ljava/lang/String; + public fun ()V +} + public final class io/sentry/transport/AsyncHttpTransport : io/sentry/transport/ITransport { public fun (Lio/sentry/SentryOptions;Lio/sentry/transport/RateLimiter;Lio/sentry/transport/ITransportGate;Lio/sentry/RequestDetails;)V public fun (Lio/sentry/transport/QueuedThreadPoolExecutor;Lio/sentry/SentryOptions;Lio/sentry/transport/RateLimiter;Lio/sentry/transport/ITransportGate;Lio/sentry/transport/HttpConnection;)V diff --git a/sentry/src/main/java/io/sentry/Attachment.java b/sentry/src/main/java/io/sentry/Attachment.java index 5e4113c96e..6dc24d5a84 100644 --- a/sentry/src/main/java/io/sentry/Attachment.java +++ b/sentry/src/main/java/io/sentry/Attachment.java @@ -1,6 +1,12 @@ package io.sentry; +import io.sentry.protocol.ViewHierarchy; +import java.io.BufferedWriter; +import java.io.ByteArrayOutputStream; import java.io.File; +import java.io.OutputStreamWriter; +import java.io.Writer; +import java.nio.charset.Charset; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -18,6 +24,7 @@ public final class Attachment { /** A standard attachment without special meaning */ private static final String DEFAULT_ATTACHMENT_TYPE = "event.attachment"; + // private static final String VIEW_HIERARCHY_ATTACHMENT_TYPE = "event.view_hierarchy"; /** * Initializes an Attachment with bytes and a filename. Sets addToTransaction to false @@ -59,9 +66,29 @@ public Attachment( final @NotNull String filename, final @Nullable String contentType, final boolean addToTransactions) { + this(bytes, filename, contentType, DEFAULT_ATTACHMENT_TYPE, addToTransactions); + } + + /** + * Initializes an Attachment with bytes, a filename, a content type, and addToTransactions. + * + * @param bytes The bytes of file. + * @param filename The name of the attachment to display in Sentry. + * @param contentType The content type of the attachment. + * @param attachmentType the attachment type. + * @param addToTransactions true if the SDK should add this attachment to every + * {@link ITransaction} or set to false if it shouldn't. + */ + public Attachment( + final @NotNull byte[] bytes, + final @NotNull String filename, + final @Nullable String contentType, + final @Nullable String attachmentType, + final boolean addToTransactions) { this.bytes = bytes; this.filename = filename; this.contentType = contentType; + this.attachmentType = attachmentType; this.addToTransactions = addToTransactions; } @@ -110,7 +137,34 @@ public Attachment( final @NotNull String pathname, final @NotNull String filename, final @Nullable String contentType) { - this(pathname, filename, contentType, false); + this(pathname, filename, contentType, DEFAULT_ATTACHMENT_TYPE, false); + } + + /** + * Initializes an Attachment with a path, a filename, a content type, and addToTransactions. + * + *

The file located at the pathname is read lazily when the SDK captures an event or + * transaction not when the attachment is initialized. The pathname string is converted into an + * abstract pathname before reading the file. + * + * @param pathname The pathname string of the file to upload as an attachment. + * @param filename The name of the attachment to display in Sentry. + * @param contentType The content type of the attachment. + * @param attachmentType The attachment type. + * @param addToTransactions true if the SDK should add this attachment to every + * {@link ITransaction} or set to false if it shouldn't. + */ + public Attachment( + final @NotNull String pathname, + final @NotNull String filename, + final @Nullable String contentType, + final @Nullable String attachmentType, + final boolean addToTransactions) { + this.pathname = pathname; + this.filename = filename; + this.contentType = contentType; + this.attachmentType = attachmentType; + this.addToTransactions = addToTransactions; } /** @@ -230,4 +284,31 @@ boolean isAddToTransactions() { public static @NotNull Attachment fromScreenshot(final byte[] screenshotBytes) { return new Attachment(screenshotBytes, "screenshot.png", "image/png", false); } + + /** + * Creates a new View Hierarchy Attachment + * + * @param viewHierarchy the View Hierarchy + * @return the Attachment + */ + public static @Nullable Attachment fromViewHierarchy(final ViewHierarchy viewHierarchy) { + final Charset UTF_8 = Charset.forName("UTF-8"); + try (final ByteArrayOutputStream stream = new ByteArrayOutputStream(); + final Writer writer = new BufferedWriter(new OutputStreamWriter(stream, UTF_8))) { + final ISerializer serializer = Sentry.getCurrentHub().getOptions().getSerializer(); + serializer.serialize(viewHierarchy, writer); + return new Attachment( + stream.toByteArray(), + "view-hierarchy.json", + "'application/json", + DEFAULT_ATTACHMENT_TYPE, // TODO replace with VIEW_HIERARCHY_ATTACHMENT_TYPE, + false); + } catch (Exception e) { + Sentry.getCurrentHub() + .getOptions() + .getLogger() + .log(SentryLevel.ERROR, "Could not serialize the ViewHierarchy", e); + } + return null; + } } diff --git a/sentry/src/main/java/io/sentry/Hint.java b/sentry/src/main/java/io/sentry/Hint.java index fa7ade6461..689860b3e5 100644 --- a/sentry/src/main/java/io/sentry/Hint.java +++ b/sentry/src/main/java/io/sentry/Hint.java @@ -28,6 +28,7 @@ public final class Hint { private final @NotNull Map internalStorage = new HashMap(); private final @NotNull List attachments = new ArrayList<>(); private @Nullable Attachment screenshot = null; + private @Nullable Attachment viewHierarchy = null; public static @NotNull Hint withAttachment(@Nullable Attachment attachment) { @NotNull final Hint hint = new Hint(); @@ -117,6 +118,14 @@ public void setScreenshot(@Nullable Attachment screenshot) { return screenshot; } + public void setViewHierarchy(@Nullable Attachment viewHierarchy) { + this.viewHierarchy = viewHierarchy; + } + + public @Nullable Attachment getViewHierarchy() { + return viewHierarchy; + } + private boolean isCastablePrimitive(@Nullable Object hintValue, @NotNull Class clazz) { Class nonPrimitiveClass = PRIMITIVE_MAPPINGS.get(clazz.getCanonicalName()); return hintValue != null diff --git a/sentry/src/main/java/io/sentry/JsonSerializer.java b/sentry/src/main/java/io/sentry/JsonSerializer.java index b409110573..e3de57e971 100644 --- a/sentry/src/main/java/io/sentry/JsonSerializer.java +++ b/sentry/src/main/java/io/sentry/JsonSerializer.java @@ -26,6 +26,8 @@ import io.sentry.protocol.SentryThread; import io.sentry.protocol.SentryTransaction; import io.sentry.protocol.User; +import io.sentry.protocol.ViewHierarchy; +import io.sentry.protocol.ViewHierarchyNode; import io.sentry.util.Objects; import java.io.BufferedOutputStream; import java.io.BufferedWriter; @@ -108,6 +110,8 @@ public JsonSerializer(@NotNull SentryOptions options) { deserializersByClass.put(User.class, new User.Deserializer()); deserializersByClass.put(UserFeedback.class, new UserFeedback.Deserializer()); deserializersByClass.put(ClientReport.class, new ClientReport.Deserializer()); + deserializersByClass.put(ViewHierarchyNode.class, new ViewHierarchyNode.Deserializer()); + deserializersByClass.put(ViewHierarchy.class, new ViewHierarchy.Deserializer()); } // Deserialize diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index 7fa67ae63e..f90496eafc 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -232,6 +232,11 @@ private boolean shouldSendSessionUpdateForDroppedEvent( attachments.add(screenshot); } + @Nullable final Attachment viewHierarchy = hint.getViewHierarchy(); + if (viewHierarchy != null) { + attachments.add(viewHierarchy); + } + return attachments; } diff --git a/sentry/src/main/java/io/sentry/protocol/ViewHierarchy.java b/sentry/src/main/java/io/sentry/protocol/ViewHierarchy.java new file mode 100644 index 0000000000..a4197c739b --- /dev/null +++ b/sentry/src/main/java/io/sentry/protocol/ViewHierarchy.java @@ -0,0 +1,98 @@ +package io.sentry.protocol; + +import io.sentry.ILogger; +import io.sentry.JsonDeserializer; +import io.sentry.JsonObjectReader; +import io.sentry.JsonObjectWriter; +import io.sentry.JsonSerializable; +import io.sentry.JsonUnknown; +import io.sentry.vendor.gson.stream.JsonToken; +import java.io.IOException; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +public final class ViewHierarchy implements JsonUnknown, JsonSerializable { + + public static final class JsonKeys { + public static final String RENDERING_SYSTEM = "rendering_system"; + public static final String WINDOWS = "windows"; + } + + private final @Nullable String renderingSystem; + private final @Nullable List windows; + private @Nullable Map unknown; + + public ViewHierarchy( + @Nullable String renderingSystem, @Nullable List windows) { + this.renderingSystem = renderingSystem; + this.windows = windows; + } + + @Override + public void serialize(@NotNull JsonObjectWriter writer, @NotNull ILogger logger) + throws IOException { + writer.beginObject(); + if (renderingSystem != null) { + writer.name(JsonKeys.RENDERING_SYSTEM).value(renderingSystem); + } + if (windows != null) { + writer.name(JsonKeys.WINDOWS).value(logger, windows); + } + if (unknown != null) { + for (String key : unknown.keySet()) { + Object value = unknown.get(key); + writer.name(key).value(logger, value); + } + } + writer.endObject(); + } + + @Override + public @Nullable Map getUnknown() { + return unknown; + } + + @Override + public void setUnknown(@Nullable Map unknown) { + this.unknown = unknown; + } + + public static final class Deserializer implements JsonDeserializer { + + @Override + public @NotNull ViewHierarchy deserialize( + @NotNull JsonObjectReader reader, @NotNull ILogger logger) throws Exception { + + @Nullable String renderingSystem = null; + @Nullable List windows = null; + @Nullable Map unknown = null; + + reader.beginObject(); + while (reader.peek() == JsonToken.NAME) { + final String nextName = reader.nextName(); + switch (nextName) { + case JsonKeys.RENDERING_SYSTEM: + renderingSystem = reader.nextStringOrNull(); + break; + case JsonKeys.WINDOWS: + windows = reader.nextList(logger, new ViewHierarchyNode.Deserializer()); + break; + default: + if (unknown == null) { + unknown = new HashMap<>(); + } + reader.nextUnknown(logger, unknown, nextName); + break; + } + } + reader.endObject(); + + final ViewHierarchy viewHierarchy = new ViewHierarchy(renderingSystem, windows); + viewHierarchy.setUnknown(unknown); + return viewHierarchy; + } + } +} diff --git a/sentry/src/main/java/io/sentry/protocol/ViewHierarchyNode.java b/sentry/src/main/java/io/sentry/protocol/ViewHierarchyNode.java new file mode 100644 index 0000000000..a04c6dea63 --- /dev/null +++ b/sentry/src/main/java/io/sentry/protocol/ViewHierarchyNode.java @@ -0,0 +1,208 @@ +package io.sentry.protocol; + +import io.sentry.ILogger; +import io.sentry.JsonDeserializer; +import io.sentry.JsonObjectReader; +import io.sentry.JsonObjectWriter; +import io.sentry.JsonSerializable; +import io.sentry.JsonUnknown; +import io.sentry.vendor.gson.stream.JsonToken; +import java.io.IOException; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +public final class ViewHierarchyNode implements JsonUnknown, JsonSerializable { + + public static final class JsonKeys { + public static final String RENDERING_SYSTEM = "rendering_system"; + public static final String TYPE = "type"; + public static final String IDENTIFIER = "identifier"; + public static final String TAG = "tag"; + public static final String WIDTH = "width"; + public static final String HEIGHT = "height"; + public static final String X = "x"; + public static final String Y = "y"; + public static final String VISIBLE = "visible"; + public static final String ALPHA = "alpha"; + public static final String CHILDREN = "children"; + } + + private @Nullable String renderingSystem; + private @Nullable String type; + private @Nullable String identifier; + private @Nullable String tag; + private @Nullable Double width; + private @Nullable Double height; + private @Nullable Double x; + private @Nullable Double y; + private @Nullable Boolean visible; + private @Nullable Double alpha; + private @Nullable List children; + private @Nullable Map unknown; + + public ViewHierarchyNode() {} + + public void setRenderingSystem(String renderingSystem) { + this.renderingSystem = renderingSystem; + } + + public void setType(String type) { + this.type = type; + } + + public void setIdentifier(final @Nullable String identifier) { + this.identifier = identifier; + } + + public void setTag(final @Nullable String tag) { + this.tag = tag; + } + + public void setWidth(final @Nullable Double width) { + this.width = width; + } + + public void setHeight(final @Nullable Double height) { + this.height = height; + } + + public void setX(final @Nullable Double x) { + this.x = x; + } + + public void setY(final @Nullable Double y) { + this.y = y; + } + + public void setVisible(final @Nullable Boolean visible) { + this.visible = visible; + } + + public void setAlpha(final @Nullable Double alpha) { + this.alpha = alpha; + } + + public void setChildren(final @Nullable List children) { + this.children = children; + } + + @Override + public void serialize(@NotNull JsonObjectWriter writer, @NotNull ILogger logger) + throws IOException { + writer.beginObject(); + if (renderingSystem != null) { + writer.name(JsonKeys.RENDERING_SYSTEM).value(renderingSystem); + } + if (type != null) { + writer.name(JsonKeys.TYPE).value(type); + } + if (identifier != null) { + writer.name(JsonKeys.IDENTIFIER).value(identifier); + } + if (tag != null) { + writer.name(JsonKeys.TAG).value(tag); + } + if (width != null) { + writer.name(JsonKeys.WIDTH).value(width); + } + if (height != null) { + writer.name(JsonKeys.HEIGHT).value(height); + } + if (x != null) { + writer.name(JsonKeys.X).value(x); + } + if (y != null) { + writer.name(JsonKeys.Y).value(y); + } + if (visible != null) { + writer.name(JsonKeys.VISIBLE).value(visible); + } + if (alpha != null) { + writer.name(JsonKeys.ALPHA).value(alpha); + } + if (children != null && !children.isEmpty()) { + writer.name(JsonKeys.CHILDREN).value(logger, children); + } + + if (unknown != null) { + for (String key : unknown.keySet()) { + Object value = unknown.get(key); + writer.name(key).value(logger, value); + } + } + writer.endObject(); + } + + @Override + public @Nullable Map getUnknown() { + return unknown; + } + + @Override + public void setUnknown(@Nullable Map unknown) { + this.unknown = unknown; + } + + public static final class Deserializer implements JsonDeserializer { + + @Override + public @NotNull ViewHierarchyNode deserialize( + @NotNull JsonObjectReader reader, @NotNull ILogger logger) throws Exception { + @Nullable Map unknown = null; + @NotNull final ViewHierarchyNode node = new ViewHierarchyNode(); + + reader.beginObject(); + while (reader.peek() == JsonToken.NAME) { + final String nextName = reader.nextName(); + switch (nextName) { + case JsonKeys.RENDERING_SYSTEM: + node.renderingSystem = reader.nextStringOrNull(); + break; + case JsonKeys.TYPE: + node.type = reader.nextStringOrNull(); + break; + case JsonKeys.IDENTIFIER: + node.identifier = reader.nextStringOrNull(); + break; + case JsonKeys.TAG: + node.tag = reader.nextStringOrNull(); + break; + case JsonKeys.WIDTH: + node.width = reader.nextDoubleOrNull(); + break; + case JsonKeys.HEIGHT: + node.height = reader.nextDoubleOrNull(); + break; + case JsonKeys.X: + node.x = reader.nextDoubleOrNull(); + break; + case JsonKeys.Y: + node.y = reader.nextDoubleOrNull(); + break; + case JsonKeys.VISIBLE: + node.visible = reader.nextBooleanOrNull(); + break; + case JsonKeys.ALPHA: + node.alpha = reader.nextDoubleOrNull(); + break; + case JsonKeys.CHILDREN: + node.children = reader.nextList(logger, this); + break; + default: + if (unknown == null) { + unknown = new HashMap<>(); + } + reader.nextUnknown(logger, unknown, nextName); + break; + } + } + reader.endObject(); + + node.setUnknown(unknown); + return node; + } + } +} diff --git a/sentry/src/test/java/io/sentry/protocol/ViewHierarchyNodeSerializationTest.kt b/sentry/src/test/java/io/sentry/protocol/ViewHierarchyNodeSerializationTest.kt new file mode 100644 index 0000000000..77f9f20383 --- /dev/null +++ b/sentry/src/test/java/io/sentry/protocol/ViewHierarchyNodeSerializationTest.kt @@ -0,0 +1,78 @@ +package io.sentry.protocol + +import io.sentry.FileFromResources +import io.sentry.ILogger +import io.sentry.JsonObjectReader +import io.sentry.JsonObjectWriter +import io.sentry.JsonSerializable +import org.junit.Test +import org.mockito.kotlin.mock +import java.io.StringReader +import java.io.StringWriter +import kotlin.test.assertEquals + +class ViewHierarchyNodeSerializationTest { + + private class Fixture { + val logger = mock() + + fun getSut() = ViewHierarchyNode().apply { + setType("com.example.ui.FancyButton") + setIdentifier("button_logout") + setChildren( + listOf( + ViewHierarchyNode().apply { + setRenderingSystem("compose") + setType("Clickable") + } + ) + ) + setWidth(100.0) + setHeight(200.0) + setX(0.0) + setY(2.0) + setVisible(true) + setAlpha(1.0) + unknown = mapOf( + "extra_property" to 42 + ) + } + } + + private val fixture = Fixture() + + @Test + fun serialize() { + val expected = sanitizedFile("json/view_hierarchy_node.json") + val actual = serialize(fixture.getSut()) + assertEquals(expected, actual) + } + + @Test + fun deserialize() { + val expectedJson = sanitizedFile("json/view_hierarchy_node.json") + val actual = deserialize(expectedJson) + val actualJson = serialize(actual) + assertEquals(expectedJson, actualJson) + } + + // Helper + + private fun sanitizedFile(path: String): String { + return FileFromResources.invoke(path) + .replace(Regex("[\n\r]"), "") + .replace(" ", "") + } + + private fun serialize(jsonSerializable: JsonSerializable): String { + val wrt = StringWriter() + val jsonWrt = JsonObjectWriter(wrt, 100) + jsonSerializable.serialize(jsonWrt, fixture.logger) + return wrt.toString() + } + + private fun deserialize(json: String): ViewHierarchyNode { + val reader = JsonObjectReader(StringReader(json)) + return ViewHierarchyNode.Deserializer().deserialize(reader, fixture.logger) + } +} diff --git a/sentry/src/test/java/io/sentry/protocol/ViewHierarchySerializationTest.kt b/sentry/src/test/java/io/sentry/protocol/ViewHierarchySerializationTest.kt new file mode 100644 index 0000000000..b2fc9c4e02 --- /dev/null +++ b/sentry/src/test/java/io/sentry/protocol/ViewHierarchySerializationTest.kt @@ -0,0 +1,64 @@ +package io.sentry.protocol + +import io.sentry.FileFromResources +import io.sentry.ILogger +import io.sentry.JsonObjectReader +import io.sentry.JsonObjectWriter +import io.sentry.JsonSerializable +import org.junit.Test +import org.mockito.kotlin.mock +import java.io.StringReader +import java.io.StringWriter +import kotlin.test.assertEquals + +class ViewHierarchySerializationTest { + + private class Fixture { + val logger = mock() + fun getSut() = ViewHierarchy( + "android_view_system", + listOf( + ViewHierarchyNode().apply { + setType("com.example.ui.FancyButton") + } + ) + ) + } + + private val fixture = Fixture() + + @Test + fun serialize() { + val expected = sanitizedFile("json/view_hierarchy.json") + val actual = serialize(fixture.getSut()) + assertEquals(expected, actual) + } + + @Test + fun deserialize() { + val expectedJson = sanitizedFile("json/view_hierarchy.json") + val actual = deserialize(expectedJson) + val actualJson = serialize(actual) + assertEquals(expectedJson, actualJson) + } + + // Helper + + private fun sanitizedFile(path: String): String { + return FileFromResources.invoke(path) + .replace(Regex("[\n\r]"), "") + .replace(" ", "") + } + + private fun serialize(jsonSerializable: JsonSerializable): String { + val wrt = StringWriter() + val jsonWrt = JsonObjectWriter(wrt, 100) + jsonSerializable.serialize(jsonWrt, fixture.logger) + return wrt.toString() + } + + private fun deserialize(json: String): ViewHierarchy { + val reader = JsonObjectReader(StringReader(json)) + return ViewHierarchy.Deserializer().deserialize(reader, fixture.logger) + } +} diff --git a/sentry/src/test/resources/json/view_hierarchy.json b/sentry/src/test/resources/json/view_hierarchy.json new file mode 100644 index 0000000000..5f6da6e6d4 --- /dev/null +++ b/sentry/src/test/resources/json/view_hierarchy.json @@ -0,0 +1,6 @@ +{ + "rendering_system": "android_view_system", + "windows": [{ + "type": "com.example.ui.FancyButton" + }] +} diff --git a/sentry/src/test/resources/json/view_hierarchy_node.json b/sentry/src/test/resources/json/view_hierarchy_node.json new file mode 100644 index 0000000000..adb6daafc6 --- /dev/null +++ b/sentry/src/test/resources/json/view_hierarchy_node.json @@ -0,0 +1,17 @@ +{ + "type": "com.example.ui.FancyButton", + "identifier": "button_logout", + "width": 100.0, + "height": 200.0, + "x": 0.0, + "y": 2.0, + "visible": true, + "alpha": 1.0, + "children": [ + { + "rendering_system": "compose", + "type": "Clickable" + } + ], + "extra_property": 42 +} From 5e82c3a4e12ef8b47458157a3fffd5d54f9cbdeb Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Thu, 29 Dec 2022 10:11:00 +0100 Subject: [PATCH 02/24] Move vh attachment serialization to processor, add tests --- .../api/sentry-android-core.api | 1 + .../core/ViewHierarchyEventProcessor.java | 34 ++- .../core/ViewHierarchyEventProcessorTest.kt | 208 ++++++++++++++++++ sentry/api/sentry.api | 15 +- .../src/main/java/io/sentry/Attachment.java | 34 +-- .../io/sentry/protocol/ViewHierarchy.java | 10 + .../io/sentry/protocol/ViewHierarchyNode.java | 55 +++++ 7 files changed, 321 insertions(+), 36 deletions(-) create mode 100644 sentry-android-core/src/test/java/io/sentry/android/core/ViewHierarchyEventProcessorTest.kt diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index 37f2a97482..2011627710 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -240,6 +240,7 @@ public final class io/sentry/android/core/UserInteractionIntegration : android/a public final class io/sentry/android/core/ViewHierarchyEventProcessor : io/sentry/EventProcessor { public fun (Lio/sentry/android/core/SentryAndroidOptions;)V public fun process (Lio/sentry/SentryEvent;Lio/sentry/Hint;)Lio/sentry/SentryEvent; + public static fun snapshotViewHierarchy (Landroid/app/Activity;)Lio/sentry/protocol/ViewHierarchy; } public final class io/sentry/android/core/cache/AndroidEnvelopeCache : io/sentry/cache/EnvelopeCache { diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java index 7508b96161..affed0bb88 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java @@ -6,13 +6,17 @@ import io.sentry.Attachment; import io.sentry.EventProcessor; import io.sentry.Hint; -import io.sentry.ILogger; import io.sentry.SentryEvent; import io.sentry.SentryLevel; import io.sentry.android.core.internal.gestures.ViewUtils; import io.sentry.protocol.ViewHierarchy; import io.sentry.protocol.ViewHierarchyNode; import io.sentry.util.Objects; +import java.io.BufferedWriter; +import java.io.ByteArrayOutputStream; +import java.io.OutputStreamWriter; +import java.io.Writer; +import java.nio.charset.Charset; import java.util.ArrayList; import java.util.List; import org.jetbrains.annotations.ApiStatus; @@ -40,21 +44,33 @@ public ViewHierarchyEventProcessor(final @NotNull SentryAndroidOptions options) return event; } - final Activity activity = CurrentActivityHolder.getInstance().getActivity(); + final @Nullable Activity activity = CurrentActivityHolder.getInstance().getActivity(); if (activity == null) { return event; } - final ViewHierarchy viewHierarchy = snapshotViewHierarchy(activity, options.getLogger()); - if (viewHierarchy != null) { - hint.setViewHierarchy(Attachment.fromViewHierarchy(viewHierarchy)); + try { + final @NotNull ViewHierarchy viewHierarchy = snapshotViewHierarchy(activity); + @SuppressWarnings("CharsetObjectCanBeUsed") + final Charset UTF8 = Charset.forName("UTF-8"); + + try (final ByteArrayOutputStream stream = new ByteArrayOutputStream(); + final Writer writer = new BufferedWriter(new OutputStreamWriter(stream, UTF8))) { + + options.getSerializer().serialize(viewHierarchy, writer); + + final Attachment attachment = Attachment.fromViewHierarchy(stream.toByteArray()); + hint.setViewHierarchy(attachment); + } + } catch (Throwable t) { + options.getLogger().log(SentryLevel.ERROR, "Could not snapshot ViewHierarchy", t); } return event; } - @Nullable - private static ViewHierarchy snapshotViewHierarchy(Activity activity, ILogger logger) { + @NotNull + public static ViewHierarchy snapshotViewHierarchy(Activity activity) { final List windows = new ArrayList<>(); final ViewHierarchy viewHierarchy = new ViewHierarchy("android_view_system", windows); @@ -70,7 +86,7 @@ private static ViewHierarchy snapshotViewHierarchy(Activity activity, ILogger lo return viewHierarchy; } - private static void addChildren(View view, ViewHierarchyNode viewNode) { + private static void addChildren(@NotNull View view, @NotNull ViewHierarchyNode parentNode) { if (!(view instanceof ViewGroup)) { return; } @@ -90,7 +106,7 @@ private static void addChildren(View view, ViewHierarchyNode viewNode) { addChildren(child, childNode); } } - viewNode.setChildren(childNodes); + parentNode.setChildren(childNodes); } @NotNull diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ViewHierarchyEventProcessorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ViewHierarchyEventProcessorTest.kt new file mode 100644 index 0000000000..e7626b669c --- /dev/null +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ViewHierarchyEventProcessorTest.kt @@ -0,0 +1,208 @@ +package io.sentry.android.core + +import android.app.Activity +import android.view.View +import android.view.ViewGroup +import android.view.Window +import androidx.test.ext.junit.runners.AndroidJUnit4 +import io.sentry.Hint +import io.sentry.SentryEvent +import io.sentry.protocol.SentryException +import org.junit.runner.RunWith +import org.mockito.kotlin.mock +import org.mockito.kotlin.whenever +import java.lang.IllegalStateException +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertNull + +@RunWith(AndroidJUnit4::class) +class ViewHierarchyEventProcessorTest { + private class Fixture { + val activity = mock() + val window = mock() + val view = mock() + val options = SentryAndroidOptions().apply { + dsn = "https://key@sentry.io/proj" + } + + init { + whenever(view.width).thenReturn(1) + whenever(view.height).thenReturn(1) + whenever(window.decorView).thenReturn(view) + whenever(window.peekDecorView()).thenReturn(view) + whenever(activity.window).thenReturn(window) + + CurrentActivityHolder.getInstance().setActivity(activity) + } + + fun getSut(attachViewHierarchy: Boolean = false): ViewHierarchyEventProcessor { + options.isAttachViewHierarchy = attachViewHierarchy + return ViewHierarchyEventProcessor(options) + } + + fun process( + attachViewHierarchy: Boolean, + event: SentryEvent + ): Pair { + val processor = getSut(attachViewHierarchy) + val hint = Hint() + processor.process(event, hint) + + return Pair(event, hint) + } + } + + private lateinit var fixture: Fixture + + @BeforeTest + fun `set up`() { + fixture = Fixture() + } + + @Test + fun `when an event errored, the view hierarchy should not attached if the feature is disabled`() { + val (event, hint) = fixture.process( + false, + SentryEvent().apply { + exceptions = listOf(SentryException()) + } + ) + + assertNotNull(event) + assertNull(hint.viewHierarchy) + } + + @Test + fun `when an event errored, the view hierarchy should be attached if the feature is enabled`() { + val (event, hint) = fixture.process( + true, + SentryEvent().apply { + exceptions = listOf(SentryException()) + } + ) + + assertNotNull(event) + assertNotNull(hint.viewHierarchy) + } + + @Test + fun `when an event did not error, the view hierarchy should be attached if the feature is enabled`() { + val (event, hint) = fixture.process( + false, + SentryEvent(null) + ) + + assertNotNull(event) + assertNull(hint.viewHierarchy) + } + + @Test + fun `when there's no current activity the viewhierarchy is null`() { + CurrentActivityHolder.getInstance().clearActivity() + + val (event, hint) = fixture.process( + true, + SentryEvent().apply { + exceptions = listOf(SentryException()) + } + ) + + assertNotNull(event) + assertNull(hint.viewHierarchy) + } + + @Test + fun `when retrieving the viewHierarchy crashes no view hierarchy is collected`() { + whenever(fixture.view.width).thenThrow(IllegalStateException("invalid ui state")) + val (event, hint) = fixture.process( + true, + SentryEvent().apply { + exceptions = listOf(SentryException()) + } + ) + + assertNotNull(event) + assertNull(hint.viewHierarchy) + } + + @Test + fun `snapshot of android view is properly created`() { + val content = mockedView( + 0.0f, + 1.0f, + 200, + 400, + 1f, + View.VISIBLE, + listOf( + mockedView(10.0f, 11.0f, 100, 101, 0.5f, View.VISIBLE), + mockedView(20.0f, 21.0f, 200, 201, 1f, View.INVISIBLE) + ) + ) + + val activity = mock() + val window = mock() + whenever(window.decorView).thenReturn(content) + whenever(window.peekDecorView()).thenReturn(content) + whenever(activity.window).thenReturn(window) + + val viewHierarchy = ViewHierarchyEventProcessor.snapshotViewHierarchy(activity) + assertEquals("android_view_system", viewHierarchy.renderingSystem) + assertEquals(1, viewHierarchy.windows!!.size) + + val contentNode = viewHierarchy.windows!![0] + assertEquals(200.0, contentNode.width) + assertEquals(400.0, contentNode.height) + assertEquals(0.0, contentNode.x) + assertEquals(1.0, contentNode.y) + assertEquals(true, contentNode.visible) + assertEquals(2, contentNode.children!!.size) + + contentNode.children!![0].apply { + assertEquals(100.0, width) + assertEquals(101.0, height) + assertEquals(10.0, x) + assertEquals(11.0, y) + assertEquals(true, visible) + assertEquals(null, children) + } + + contentNode.children!![1].apply { + assertEquals(200.0, width) + assertEquals(201.0, height) + assertEquals(20.0, x) + assertEquals(21.0, y) + assertEquals(false, visible) + assertEquals(null, children) + } + } + + private fun mockedView( + x: Float, + y: Float, + width: Int, + height: Int, + alpha: Float, + visibility: Int, + children: List = emptyList() + ): View { + val view = mock() + + whenever(view.x).thenReturn(x) + whenever(view.y).thenReturn(y) + whenever(view.width).thenReturn(width) + whenever(view.height).thenReturn(height) + whenever(view.alpha).thenReturn(alpha) + whenever(view.visibility).thenReturn(visibility) + whenever(view.childCount).thenReturn(children.size) + + for (i in children.indices) { + whenever(view.getChildAt(i)).thenReturn(children[i]) + } + + return view + } +} diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 318f2e508c..fcf9685ac6 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -15,7 +15,7 @@ public final class io/sentry/Attachment { public fun ([BLjava/lang/String;Ljava/lang/String;Ljava/lang/String;Z)V public fun ([BLjava/lang/String;Ljava/lang/String;Z)V public static fun fromScreenshot ([B)Lio/sentry/Attachment; - public static fun fromViewHierarchy (Lio/sentry/protocol/ViewHierarchy;)Lio/sentry/Attachment; + public static fun fromViewHierarchy ([B)Lio/sentry/Attachment; public fun getAttachmentType ()Ljava/lang/String; public fun getBytes ()[B public fun getContentType ()Ljava/lang/String; @@ -3346,7 +3346,9 @@ public final class io/sentry/protocol/User$JsonKeys { public final class io/sentry/protocol/ViewHierarchy : io/sentry/JsonSerializable, io/sentry/JsonUnknown { public fun (Ljava/lang/String;Ljava/util/List;)V + public fun getRenderingSystem ()Ljava/lang/String; public fun getUnknown ()Ljava/util/Map; + public fun getWindows ()Ljava/util/List; public fun serialize (Lio/sentry/JsonObjectWriter;Lio/sentry/ILogger;)V public fun setUnknown (Ljava/util/Map;)V } @@ -3365,7 +3367,18 @@ public final class io/sentry/protocol/ViewHierarchy$JsonKeys { public final class io/sentry/protocol/ViewHierarchyNode : io/sentry/JsonSerializable, io/sentry/JsonUnknown { public fun ()V + public fun getAlpha ()Ljava/lang/Double; + public fun getChildren ()Ljava/util/List; + public fun getHeight ()Ljava/lang/Double; + public fun getIdentifier ()Ljava/lang/String; + public fun getRenderingSystem ()Ljava/lang/String; + public fun getTag ()Ljava/lang/String; + public fun getType ()Ljava/lang/String; public fun getUnknown ()Ljava/util/Map; + public fun getVisible ()Ljava/lang/Boolean; + public fun getWidth ()Ljava/lang/Double; + public fun getX ()Ljava/lang/Double; + public fun getY ()Ljava/lang/Double; public fun serialize (Lio/sentry/JsonObjectWriter;Lio/sentry/ILogger;)V public fun setAlpha (Ljava/lang/Double;)V public fun setChildren (Ljava/util/List;)V diff --git a/sentry/src/main/java/io/sentry/Attachment.java b/sentry/src/main/java/io/sentry/Attachment.java index 6dc24d5a84..d6099eacb9 100644 --- a/sentry/src/main/java/io/sentry/Attachment.java +++ b/sentry/src/main/java/io/sentry/Attachment.java @@ -1,12 +1,6 @@ package io.sentry; -import io.sentry.protocol.ViewHierarchy; -import java.io.BufferedWriter; -import java.io.ByteArrayOutputStream; import java.io.File; -import java.io.OutputStreamWriter; -import java.io.Writer; -import java.nio.charset.Charset; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -288,27 +282,15 @@ boolean isAddToTransactions() { /** * Creates a new View Hierarchy Attachment * - * @param viewHierarchy the View Hierarchy + * @param viewHierarchyBytes the serialized View Hierarchy * @return the Attachment */ - public static @Nullable Attachment fromViewHierarchy(final ViewHierarchy viewHierarchy) { - final Charset UTF_8 = Charset.forName("UTF-8"); - try (final ByteArrayOutputStream stream = new ByteArrayOutputStream(); - final Writer writer = new BufferedWriter(new OutputStreamWriter(stream, UTF_8))) { - final ISerializer serializer = Sentry.getCurrentHub().getOptions().getSerializer(); - serializer.serialize(viewHierarchy, writer); - return new Attachment( - stream.toByteArray(), - "view-hierarchy.json", - "'application/json", - DEFAULT_ATTACHMENT_TYPE, // TODO replace with VIEW_HIERARCHY_ATTACHMENT_TYPE, - false); - } catch (Exception e) { - Sentry.getCurrentHub() - .getOptions() - .getLogger() - .log(SentryLevel.ERROR, "Could not serialize the ViewHierarchy", e); - } - return null; + public static @NotNull Attachment fromViewHierarchy(final byte[] viewHierarchyBytes) { + return new Attachment( + viewHierarchyBytes, + "view-hierarchy.json", + "'application/json", + DEFAULT_ATTACHMENT_TYPE, // TODO replace with VIEW_HIERARCHY_ATTACHMENT_TYPE, + false); } } diff --git a/sentry/src/main/java/io/sentry/protocol/ViewHierarchy.java b/sentry/src/main/java/io/sentry/protocol/ViewHierarchy.java index a4197c739b..12e5bf7931 100644 --- a/sentry/src/main/java/io/sentry/protocol/ViewHierarchy.java +++ b/sentry/src/main/java/io/sentry/protocol/ViewHierarchy.java @@ -31,6 +31,16 @@ public ViewHierarchy( this.windows = windows; } + @Nullable + public String getRenderingSystem() { + return renderingSystem; + } + + @Nullable + public List getWindows() { + return windows; + } + @Override public void serialize(@NotNull JsonObjectWriter writer, @NotNull ILogger logger) throws IOException { diff --git a/sentry/src/main/java/io/sentry/protocol/ViewHierarchyNode.java b/sentry/src/main/java/io/sentry/protocol/ViewHierarchyNode.java index a04c6dea63..fcc4353edd 100644 --- a/sentry/src/main/java/io/sentry/protocol/ViewHierarchyNode.java +++ b/sentry/src/main/java/io/sentry/protocol/ViewHierarchyNode.java @@ -89,6 +89,61 @@ public void setChildren(final @Nullable List children) { this.children = children; } + @Nullable + public String getRenderingSystem() { + return renderingSystem; + } + + @Nullable + public String getType() { + return type; + } + + @Nullable + public String getIdentifier() { + return identifier; + } + + @Nullable + public String getTag() { + return tag; + } + + @Nullable + public Double getWidth() { + return width; + } + + @Nullable + public Double getHeight() { + return height; + } + + @Nullable + public Double getX() { + return x; + } + + @Nullable + public Double getY() { + return y; + } + + @Nullable + public Boolean getVisible() { + return visible; + } + + @Nullable + public Double getAlpha() { + return alpha; + } + + @Nullable + public List getChildren() { + return children; + } + @Override public void serialize(@NotNull JsonObjectWriter writer, @NotNull ILogger logger) throws IOException { From 426d75a5b2210cf392515eaa82e2e85c12b75582 Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Thu, 29 Dec 2022 14:32:22 +0100 Subject: [PATCH 03/24] Add changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c6989b370..bac55b720f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Features - Disable Android concurrent profiling ([#2434](https://github.com/getsentry/sentry-java/pull/2434)) +- Add Android View Hierarchy support ([#2440](https://github.com/getsentry/sentry-java/pull/2440)) ### Fixes From 603c073ea83bb1043840fcf8b65cfa87de5d232a Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Thu, 29 Dec 2022 14:37:29 +0100 Subject: [PATCH 04/24] Switch sentry-compose-helper to kotlin mutliplatform Fixes resolve errors when performing Android Studio gradle sync --- .../api/sentry-compose-helper.api | 5 +++ sentry-compose-helper/build.gradle.kts | 35 +++++++++++-------- .../gestures/ComposeGestureTargetLocator.java | 0 3 files changed, 25 insertions(+), 15 deletions(-) create mode 100644 sentry-compose-helper/api/sentry-compose-helper.api rename sentry-compose-helper/src/{main => jvmMain}/java/io/sentry/compose/gestures/ComposeGestureTargetLocator.java (100%) diff --git a/sentry-compose-helper/api/sentry-compose-helper.api b/sentry-compose-helper/api/sentry-compose-helper.api new file mode 100644 index 0000000000..b9fe8287fc --- /dev/null +++ b/sentry-compose-helper/api/sentry-compose-helper.api @@ -0,0 +1,5 @@ +public final class io/sentry/compose/gestures/ComposeGestureTargetLocator : io/sentry/internal/gestures/GestureTargetLocator { + public fun ()V + public fun locate (Ljava/lang/Object;FFLio/sentry/internal/gestures/UiElement$Type;)Lio/sentry/internal/gestures/UiElement; +} + diff --git a/sentry-compose-helper/build.gradle.kts b/sentry-compose-helper/build.gradle.kts index 0243cacb44..734fd1143f 100644 --- a/sentry-compose-helper/build.gradle.kts +++ b/sentry-compose-helper/build.gradle.kts @@ -1,13 +1,30 @@ import org.jetbrains.kotlin.gradle.tasks.KotlinCompile plugins { - `java-library` - jacoco + kotlin("multiplatform") id("org.jetbrains.compose") + `java-library` id(Config.QualityPlugins.gradleVersions) id(Config.BuildPlugins.buildConfig) version Config.BuildPlugins.buildConfigVersion } +kotlin { + jvm { + withJava() + } + + sourceSets { + val jvmMain by getting { + dependencies { + implementation(projects.sentry) + + compileOnly(compose.runtime) + compileOnly(compose.ui) + } + } + } +} + configure { sourceCompatibility = JavaVersion.VERSION_1_8 targetCompatibility = JavaVersion.VERSION_1_8 @@ -17,23 +34,11 @@ tasks.withType().configureEach { kotlinOptions.jvmTarget = JavaVersion.VERSION_1_8.toString() } -dependencies { - implementation(projects.sentry) - implementation(compose.runtime) - implementation(compose.ui) -} - -configure { - test { - java.srcDir("src/test/java") - } -} - val embeddedJar by configurations.creating { isCanBeConsumed = true isCanBeResolved = false } artifacts { - add("embeddedJar", File("$buildDir/libs/sentry-compose-helper-$version.jar")) + add("embeddedJar", File("$buildDir/libs/sentry-compose-helper-jvm-$version.jar")) } diff --git a/sentry-compose-helper/src/main/java/io/sentry/compose/gestures/ComposeGestureTargetLocator.java b/sentry-compose-helper/src/jvmMain/java/io/sentry/compose/gestures/ComposeGestureTargetLocator.java similarity index 100% rename from sentry-compose-helper/src/main/java/io/sentry/compose/gestures/ComposeGestureTargetLocator.java rename to sentry-compose-helper/src/jvmMain/java/io/sentry/compose/gestures/ComposeGestureTargetLocator.java From ccc324baccfb2c8fca0e6f1e2d53ecfcbe8f718e Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Fri, 30 Dec 2022 11:09:04 +0100 Subject: [PATCH 05/24] Extract Screenshot activity callbacks to CurrentActivityIntegration This way the CurrentActivityHolder can provide the activity to both the screenshot and viewhierarchy integration --- .../api/sentry-android-core.api | 25 ++-- .../core/AndroidOptionsInitializer.java | 4 +- .../core/CurrentActivityIntegration.java | 79 +++++++++++++ .../core/ScreenshotEventProcessor.java | 77 +------------ .../core/AndroidOptionsInitializerTest.kt | 9 ++ .../core/CurrentActivityIntegrationTest.kt | 107 ++++++++++++++++++ .../core/ScreenshotEventProcessorTest.kt | 57 ++-------- 7 files changed, 223 insertions(+), 135 deletions(-) create mode 100644 sentry-android-core/src/main/java/io/sentry/android/core/CurrentActivityIntegration.java create mode 100644 sentry-android-core/src/test/java/io/sentry/android/core/CurrentActivityIntegrationTest.kt diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index 2011627710..174590b3d3 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -89,6 +89,19 @@ public class io/sentry/android/core/CurrentActivityHolder { public fun setActivity (Landroid/app/Activity;)V } +public final class io/sentry/android/core/CurrentActivityIntegration : android/app/Application$ActivityLifecycleCallbacks, io/sentry/Integration, java/io/Closeable { + public fun (Landroid/app/Application;)V + public fun close ()V + public fun onActivityCreated (Landroid/app/Activity;Landroid/os/Bundle;)V + public fun onActivityDestroyed (Landroid/app/Activity;)V + public fun onActivityPaused (Landroid/app/Activity;)V + public fun onActivityResumed (Landroid/app/Activity;)V + public fun onActivitySaveInstanceState (Landroid/app/Activity;Landroid/os/Bundle;)V + public fun onActivityStarted (Landroid/app/Activity;)V + public fun onActivityStopped (Landroid/app/Activity;)V + public fun register (Lio/sentry/IHub;Lio/sentry/SentryOptions;)V +} + public abstract class io/sentry/android/core/EnvelopeFileObserverIntegration : io/sentry/Integration, java/io/Closeable { public fun ()V public fun close ()V @@ -121,16 +134,8 @@ public final class io/sentry/android/core/PhoneStateBreadcrumbsIntegration : io/ public fun register (Lio/sentry/IHub;Lio/sentry/SentryOptions;)V } -public final class io/sentry/android/core/ScreenshotEventProcessor : android/app/Application$ActivityLifecycleCallbacks, io/sentry/EventProcessor, java/io/Closeable { - public fun (Landroid/app/Application;Lio/sentry/android/core/SentryAndroidOptions;Lio/sentry/android/core/BuildInfoProvider;)V - public fun close ()V - public fun onActivityCreated (Landroid/app/Activity;Landroid/os/Bundle;)V - public fun onActivityDestroyed (Landroid/app/Activity;)V - public fun onActivityPaused (Landroid/app/Activity;)V - public fun onActivityResumed (Landroid/app/Activity;)V - public fun onActivitySaveInstanceState (Landroid/app/Activity;Landroid/os/Bundle;)V - public fun onActivityStarted (Landroid/app/Activity;)V - public fun onActivityStopped (Landroid/app/Activity;)V +public final class io/sentry/android/core/ScreenshotEventProcessor : io/sentry/EventProcessor { + public fun (Lio/sentry/android/core/SentryAndroidOptions;Lio/sentry/android/core/BuildInfoProvider;)V public fun process (Lio/sentry/SentryEvent;Lio/sentry/Hint;)Lio/sentry/SentryEvent; } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java index 3de0435a53..8e6599592d 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java @@ -209,12 +209,12 @@ private static void installDefaultIntegrations( options.addIntegration( new ActivityLifecycleIntegration( (Application) context, buildInfoProvider, activityFramesTracker)); + options.addIntegration(new CurrentActivityIntegration((Application) context)); options.addIntegration(new UserInteractionIntegration((Application) context, loadClass)); if (isFragmentAvailable) { options.addIntegration(new FragmentLifecycleIntegration((Application) context, true, true)); } - options.addEventProcessor( - new ScreenshotEventProcessor((Application) context, options, buildInfoProvider)); + options.addEventProcessor(new ScreenshotEventProcessor(options, buildInfoProvider)); } else { options .getLogger() diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/CurrentActivityIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/CurrentActivityIntegration.java new file mode 100644 index 0000000000..da8ce117b1 --- /dev/null +++ b/sentry-android-core/src/main/java/io/sentry/android/core/CurrentActivityIntegration.java @@ -0,0 +1,79 @@ +package io.sentry.android.core; + +import android.app.Activity; +import android.app.Application; +import android.os.Bundle; +import androidx.annotation.NonNull; +import androidx.annotation.Nullable; +import io.sentry.IHub; +import io.sentry.Integration; +import io.sentry.SentryOptions; +import java.io.Closeable; +import java.io.IOException; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; + +@ApiStatus.Internal +public final class CurrentActivityIntegration + implements Integration, Closeable, Application.ActivityLifecycleCallbacks { + + private final @NotNull Application application; + + public CurrentActivityIntegration(@NotNull Application application) { + this.application = application; + } + + @Override + public void register(@NotNull IHub hub, @NotNull SentryOptions options) { + application.registerActivityLifecycleCallbacks(this); + } + + @Override + public void onActivityCreated(@NonNull Activity activity, @Nullable Bundle savedInstanceState) { + setCurrentActivity(activity); + } + + @Override + public void onActivityStarted(@NonNull Activity activity) { + setCurrentActivity(activity); + } + + @Override + public void onActivityResumed(@NonNull Activity activity) { + setCurrentActivity(activity); + } + + @Override + public void onActivityPaused(@NonNull Activity activity) { + cleanCurrentActivity(activity); + } + + @Override + public void onActivityStopped(@NonNull Activity activity) { + cleanCurrentActivity(activity); + } + + @Override + public void onActivitySaveInstanceState(@NonNull Activity activity, @NonNull Bundle outState) {} + + @Override + public void onActivityDestroyed(@NonNull Activity activity) { + cleanCurrentActivity(activity); + } + + @Override + public void close() throws IOException { + application.unregisterActivityLifecycleCallbacks(this); + CurrentActivityHolder.getInstance().clearActivity(); + } + + private void cleanCurrentActivity(@NonNull Activity activity) { + if (CurrentActivityHolder.getInstance().getActivity() == activity) { + CurrentActivityHolder.getInstance().clearActivity(); + } + } + + private void setCurrentActivity(@NonNull Activity activity) { + CurrentActivityHolder.getInstance().setActivity(activity); + } +} diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ScreenshotEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/ScreenshotEventProcessor.java index 041d441579..3bb6d91d8c 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ScreenshotEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ScreenshotEventProcessor.java @@ -4,10 +4,6 @@ import static io.sentry.android.core.internal.util.ScreenshotUtils.takeScreenshot; import android.app.Activity; -import android.app.Application; -import android.os.Bundle; -import androidx.annotation.NonNull; -import androidx.annotation.Nullable; import io.sentry.Attachment; import io.sentry.EventProcessor; import io.sentry.Hint; @@ -15,8 +11,6 @@ import io.sentry.SentryLevel; import io.sentry.util.HintUtils; import io.sentry.util.Objects; -import java.io.Closeable; -import java.io.IOException; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; @@ -25,41 +19,27 @@ * captured. */ @ApiStatus.Internal -public final class ScreenshotEventProcessor - implements EventProcessor, Application.ActivityLifecycleCallbacks, Closeable { +public final class ScreenshotEventProcessor implements EventProcessor { - private final @NotNull Application application; private final @NotNull SentryAndroidOptions options; private final @NotNull BuildInfoProvider buildInfoProvider; - private boolean lifecycleCallbackInstalled = true; public ScreenshotEventProcessor( - final @NotNull Application application, final @NotNull SentryAndroidOptions options, final @NotNull BuildInfoProvider buildInfoProvider) { - this.application = Objects.requireNonNull(application, "Application is required"); this.options = Objects.requireNonNull(options, "SentryAndroidOptions is required"); this.buildInfoProvider = Objects.requireNonNull(buildInfoProvider, "BuildInfoProvider is required"); - - application.registerActivityLifecycleCallbacks(this); } @SuppressWarnings("NullAway") @Override public @NotNull SentryEvent process(final @NotNull SentryEvent event, @NotNull Hint hint) { - if (!lifecycleCallbackInstalled || !event.isErrored()) { + if (!event.isErrored()) { return event; } if (!options.isAttachScreenshot()) { - application.unregisterActivityLifecycleCallbacks(this); - lifecycleCallbackInstalled = false; - - this.options - .getLogger() - .log( - SentryLevel.DEBUG, - "attachScreenshot is disabled, ScreenshotEventProcessor isn't installed."); + this.options.getLogger().log(SentryLevel.DEBUG, "attachScreenshot is disabled."); return event; } @@ -77,55 +57,4 @@ public ScreenshotEventProcessor( hint.set(ANDROID_ACTIVITY, activity); return event; } - - @Override - public void onActivityCreated(@NonNull Activity activity, @Nullable Bundle savedInstanceState) { - CurrentActivityHolder.getInstance().setActivity(activity); - } - - @Override - public void onActivityStarted(@NonNull Activity activity) { - setCurrentActivity(activity); - } - - @Override - public void onActivityResumed(@NonNull Activity activity) { - setCurrentActivity(activity); - } - - @Override - public void onActivityPaused(@NonNull Activity activity) { - cleanCurrentActivity(activity); - } - - @Override - public void onActivityStopped(@NonNull Activity activity) { - cleanCurrentActivity(activity); - } - - @Override - public void onActivitySaveInstanceState(@NonNull Activity activity, @NonNull Bundle outState) {} - - @Override - public void onActivityDestroyed(@NonNull Activity activity) { - cleanCurrentActivity(activity); - } - - @Override - public void close() throws IOException { - if (options.isAttachScreenshot()) { - application.unregisterActivityLifecycleCallbacks(this); - CurrentActivityHolder.getInstance().clearActivity(); - } - } - - private void cleanCurrentActivity(@NonNull Activity activity) { - if (CurrentActivityHolder.getInstance().getActivity() == activity) { - CurrentActivityHolder.getInstance().clearActivity(); - } - } - - private void setCurrentActivity(@NonNull Activity activity) { - CurrentActivityHolder.getInstance().setActivity(activity); - } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt index 5b8dadf097..ef77c71e51 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt @@ -398,6 +398,15 @@ class AndroidOptionsInitializerTest { assertTrue { fixture.sentryOptions.envelopeDiskCache is AndroidEnvelopeCache } } + @Test + fun `CurrentActivityIntegration is added by default`() { + fixture.initSut(useRealContext = true) + + val actual = + fixture.sentryOptions.integrations.firstOrNull { it is CurrentActivityIntegration } + assertNotNull(actual) + } + @Test fun `When Activity Frames Tracking is enabled, the Activity Frames Tracker should be available`() { fixture.initSut( diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/CurrentActivityIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/CurrentActivityIntegrationTest.kt new file mode 100644 index 0000000000..6330623121 --- /dev/null +++ b/sentry-android-core/src/test/java/io/sentry/android/core/CurrentActivityIntegrationTest.kt @@ -0,0 +1,107 @@ +package io.sentry.android.core + +import android.app.Activity +import android.app.Application +import androidx.test.ext.junit.runners.AndroidJUnit4 +import io.sentry.IHub +import org.junit.runner.RunWith +import org.mockito.kotlin.any +import org.mockito.kotlin.mock +import org.mockito.kotlin.verify +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNull + +@RunWith(AndroidJUnit4::class) +class CurrentActivityIntegrationTest { + + private class Fixture { + val application = mock() + val activity = mock() + val hub = mock() + + val options = SentryAndroidOptions().apply { + dsn = "https://key@sentry.io/proj" + } + + fun getSut(): CurrentActivityIntegration { + val integration = CurrentActivityIntegration(application) + integration.register(hub, options) + return integration + } + } + + private lateinit var fixture: Fixture + + @BeforeTest + fun `set up`() { + fixture = Fixture() + } + + @Test + fun `when the integration is added registerActivityLifecycleCallbacks is called`() { + fixture.getSut() + verify(fixture.application).registerActivityLifecycleCallbacks(any()) + } + + @Test + fun `when the integration is closed unregisterActivityLifecycleCallbacks is called`() { + val sut = fixture.getSut() + sut.close() + + verify(fixture.application).unregisterActivityLifecycleCallbacks(any()) + } + + @Test + fun `when an activity is created the activity holder provides it`() { + val sut = fixture.getSut() + + sut.onActivityCreated(fixture.activity, null) + assertEquals(fixture.activity, CurrentActivityHolder.getInstance().activity) + } + + @Test + fun `when there is no active activity the holder does not provide an outdated one`() { + val sut = fixture.getSut() + + sut.onActivityCreated(fixture.activity, null) + sut.onActivityDestroyed(fixture.activity) + + assertNull(CurrentActivityHolder.getInstance().activity) + } + + @Test + fun `when a second activity is started it gets the current one`() { + val sut = fixture.getSut() + + sut.onActivityCreated(fixture.activity, null) + sut.onActivityStarted(fixture.activity) + sut.onActivityResumed(fixture.activity) + + val secondActivity = mock() + sut.onActivityCreated(secondActivity, null) + sut.onActivityStarted(secondActivity) + + assertEquals(secondActivity, CurrentActivityHolder.getInstance().activity) + } + + @Test + fun `destroying an old activity keeps the current one`() { + val sut = fixture.getSut() + + sut.onActivityCreated(fixture.activity, null) + sut.onActivityStarted(fixture.activity) + sut.onActivityResumed(fixture.activity) + + val secondActivity = mock() + sut.onActivityCreated(secondActivity, null) + sut.onActivityStarted(secondActivity) + + sut.onActivityPaused(fixture.activity) + sut.onActivityStopped(fixture.activity) + sut.onActivityDestroyed(fixture.activity) + + assertEquals(secondActivity, CurrentActivityHolder.getInstance().activity) + } +} diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ScreenshotEventProcessorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ScreenshotEventProcessorTest.kt index 3d81b176bf..2d208db720 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ScreenshotEventProcessorTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ScreenshotEventProcessorTest.kt @@ -1,7 +1,6 @@ package io.sentry.android.core import android.app.Activity -import android.app.Application import android.view.View import android.view.Window import androidx.test.ext.junit.runners.AndroidJUnit4 @@ -11,10 +10,7 @@ import io.sentry.MainEventProcessor import io.sentry.SentryEvent import io.sentry.TypeCheckHint.ANDROID_ACTIVITY import org.junit.runner.RunWith -import org.mockito.kotlin.any import org.mockito.kotlin.mock -import org.mockito.kotlin.never -import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import kotlin.test.BeforeTest import kotlin.test.Test @@ -27,7 +23,6 @@ import kotlin.test.assertTrue class ScreenshotEventProcessorTest { private class Fixture { - val application = mock() val buildInfo = mock() val activity = mock() val window = mock() @@ -49,7 +44,7 @@ class ScreenshotEventProcessorTest { fun getSut(attachScreenshot: Boolean = false): ScreenshotEventProcessor { options.isAttachScreenshot = attachScreenshot - return ScreenshotEventProcessor(application, options, buildInfo) + return ScreenshotEventProcessor(options, buildInfo) } } @@ -60,48 +55,12 @@ class ScreenshotEventProcessorTest { fixture = Fixture() } - @Test - fun `when adding screenshot event processor, registerActivityLifecycleCallbacks`() { - fixture.getSut() - - verify(fixture.application).registerActivityLifecycleCallbacks(any()) - } - - @Test - fun `when close is called and attach screenshot is enabled, unregisterActivityLifecycleCallbacks`() { - val sut = fixture.getSut(true) - - sut.close() - - verify(fixture.application).unregisterActivityLifecycleCallbacks(any()) - } - - @Test - fun `when close is called and attach screenshot is disabled, does not unregisterActivityLifecycleCallbacks`() { - val sut = fixture.getSut() - - sut.close() - - verify(fixture.application, never()).unregisterActivityLifecycleCallbacks(any()) - } - - @Test - fun `when process is called and attachScreenshot is disabled, unregisterActivityLifecycleCallbacks`() { - val sut = fixture.getSut() - val hint = Hint() - - val event = fixture.mainProcessor.process(getEvent(), hint) - sut.process(event, hint) - - verify(fixture.application).unregisterActivityLifecycleCallbacks(any()) - } - @Test fun `when process is called and attachScreenshot is disabled, does nothing`() { val sut = fixture.getSut() val hint = Hint() - sut.onActivityCreated(fixture.activity, null) + CurrentActivityHolder.getInstance().setActivity(fixture.activity) val event = fixture.mainProcessor.process(getEvent(), hint) sut.process(event, hint) @@ -114,7 +73,7 @@ class ScreenshotEventProcessorTest { val sut = fixture.getSut(true) val hint = Hint() - sut.onActivityCreated(fixture.activity, null) + CurrentActivityHolder.getInstance().setActivity(fixture.activity) val event = fixture.mainProcessor.process(SentryEvent(), hint) sut.process(event, hint) @@ -139,7 +98,7 @@ class ScreenshotEventProcessorTest { val hint = Hint() whenever(fixture.activity.isFinishing).thenReturn(true) - sut.onActivityCreated(fixture.activity, null) + CurrentActivityHolder.getInstance().setActivity(fixture.activity) val event = fixture.mainProcessor.process(getEvent(), hint) sut.process(event, hint) @@ -154,7 +113,7 @@ class ScreenshotEventProcessorTest { whenever(fixture.rootView.width).thenReturn(0) whenever(fixture.rootView.height).thenReturn(0) - sut.onActivityCreated(fixture.activity, null) + CurrentActivityHolder.getInstance().setActivity(fixture.activity) val event = fixture.mainProcessor.process(getEvent(), hint) sut.process(event, hint) @@ -167,7 +126,7 @@ class ScreenshotEventProcessorTest { val sut = fixture.getSut(true) val hint = Hint() - sut.onActivityCreated(fixture.activity, null) + CurrentActivityHolder.getInstance().setActivity(fixture.activity) val event = fixture.mainProcessor.process(getEvent(), hint) sut.process(event, hint) @@ -185,8 +144,8 @@ class ScreenshotEventProcessorTest { val sut = fixture.getSut(true) val hint = Hint() - sut.onActivityCreated(fixture.activity, null) - sut.onActivityDestroyed(fixture.activity) + CurrentActivityHolder.getInstance().setActivity(fixture.activity) + CurrentActivityHolder.getInstance().clearActivity() val event = fixture.mainProcessor.process(getEvent(), hint) sut.process(event, hint) From 5b6d756df7b1f79a035c8a267aaa1f701d42f96c Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Fri, 30 Dec 2022 11:09:27 +0100 Subject: [PATCH 06/24] Fix failing compose tests --- sentry-android-core/build.gradle.kts | 1 + 1 file changed, 1 insertion(+) diff --git a/sentry-android-core/build.gradle.kts b/sentry-android-core/build.gradle.kts index de6ce20596..d7478786fe 100644 --- a/sentry-android-core/build.gradle.kts +++ b/sentry-android-core/build.gradle.kts @@ -110,6 +110,7 @@ dependencies { testImplementation(projects.sentryAndroidTimber) testImplementation(projects.sentryComposeHelper) testImplementation(projects.sentryAndroidNdk) + testRuntimeOnly(Config.Libs.composeUi) testRuntimeOnly(Config.Libs.timber) testRuntimeOnly(Config.Libs.fragment) } From 0fd2993063c050cc31e47f994fa3a22b552e0630 Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Tue, 3 Jan 2023 07:47:27 +0100 Subject: [PATCH 07/24] Add missing integration tests for view hierarchy --- .../src/main/java/io/sentry/Attachment.java | 2 +- .../src/test/java/io/sentry/AttachmentTest.kt | 13 +++++++ .../test/java/io/sentry/SentryClientTest.kt | 38 ++++++++++++++++++- .../src/test/java/io/sentry/hints/HintTest.kt | 20 ++++++++++ 4 files changed, 71 insertions(+), 2 deletions(-) diff --git a/sentry/src/main/java/io/sentry/Attachment.java b/sentry/src/main/java/io/sentry/Attachment.java index d6099eacb9..01506095af 100644 --- a/sentry/src/main/java/io/sentry/Attachment.java +++ b/sentry/src/main/java/io/sentry/Attachment.java @@ -289,7 +289,7 @@ boolean isAddToTransactions() { return new Attachment( viewHierarchyBytes, "view-hierarchy.json", - "'application/json", + "application/json", DEFAULT_ATTACHMENT_TYPE, // TODO replace with VIEW_HIERARCHY_ATTACHMENT_TYPE, false); } diff --git a/sentry/src/test/java/io/sentry/AttachmentTest.kt b/sentry/src/test/java/io/sentry/AttachmentTest.kt index c2887fe94f..341eba4b19 100644 --- a/sentry/src/test/java/io/sentry/AttachmentTest.kt +++ b/sentry/src/test/java/io/sentry/AttachmentTest.kt @@ -112,4 +112,17 @@ class AttachmentTest { assertEquals(false, attachment.isAddToTransactions) assertEquals(bytes, attachment.bytes) } + + @Test + fun `creates attachment from view hierarchy`() { + val bytes = byteArrayOf() + val attachment = Attachment.fromViewHierarchy(bytes) + + assertEquals("view-hierarchy.json", attachment.filename) + assertEquals("application/json", attachment.contentType) + assertEquals(false, attachment.isAddToTransactions) + // TODO replace with event.view_hierarchy + assertEquals("event.attachment", attachment.attachmentType) + assertEquals(bytes, attachment.bytes) + } } diff --git a/sentry/src/test/java/io/sentry/SentryClientTest.kt b/sentry/src/test/java/io/sentry/SentryClientTest.kt index e1f5a88f1b..64aa7316b5 100644 --- a/sentry/src/test/java/io/sentry/SentryClientTest.kt +++ b/sentry/src/test/java/io/sentry/SentryClientTest.kt @@ -1420,6 +1420,42 @@ class SentryClientTest { ) } + @Test + fun `view hierarchy is added to the envelope from the hint`() { + val sut = fixture.getSut() + val attachment = Attachment.fromViewHierarchy(byteArrayOf()) + val hint = Hint().also { it.viewHierarchy = attachment } + + sut.captureEvent(SentryEvent(), hint) + + verify(fixture.transport).send( + check { envelope -> + val viewHierarchy = envelope.items.last() + assertNotNull(viewHierarchy) { + assertEquals(attachment.filename, viewHierarchy.header.fileName) + } + }, + anyOrNull() + ) + } + + @Test + fun `view hierarchy is dropped from hint via before send`() { + fixture.sentryOptions.beforeSend = CustomBeforeSendCallback() + val sut = fixture.getSut() + val attachment = Attachment.fromViewHierarchy(byteArrayOf()) + val hint = Hint().also { it.viewHierarchy = attachment } + + sut.captureEvent(SentryEvent(), hint) + + verify(fixture.transport).send( + check { envelope -> + assertEquals(1, envelope.items.count()) + }, + anyOrNull() + ) + } + @Test fun `capturing an error updates session and sends event + session`() { val sut = fixture.getSut() @@ -1978,7 +2014,7 @@ class SentryClientTest { class CustomBeforeSendCallback : SentryOptions.BeforeSendCallback { override fun execute(event: SentryEvent, hint: Hint): SentryEvent? { hint.screenshot = null - + hint.viewHierarchy = null return event } } diff --git a/sentry/src/test/java/io/sentry/hints/HintTest.kt b/sentry/src/test/java/io/sentry/hints/HintTest.kt index fba9dd848a..054baedcae 100644 --- a/sentry/src/test/java/io/sentry/hints/HintTest.kt +++ b/sentry/src/test/java/io/sentry/hints/HintTest.kt @@ -208,6 +208,7 @@ class HintTest { hint.set(userAttribute, "test label") hint.addAttachment(newAttachment("test attachment")) hint.screenshot = newAttachment("2") + hint.viewHierarchy = newAttachment("3") hint.clear() @@ -215,6 +216,25 @@ class HintTest { assertNull(hint.get(userAttribute)) assertEquals(1, hint.attachments.size) assertNotNull(hint.screenshot) + assertNotNull(hint.viewHierarchy) + } + + @Test + fun `can create hint with a screenshot`() { + val hint = Hint() + val attachment = newAttachment("test1") + hint.screenshot = attachment + + assertNotNull(hint.screenshot) + } + + @Test + fun `can create hint with a view hierarchy`() { + val hint = Hint() + val attachment = newAttachment("test1") + hint.viewHierarchy = attachment + + assertNotNull(hint.viewHierarchy) } companion object { From 95b543f4171650c2a7b8de2b09e9f5335efbae00 Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Tue, 3 Jan 2023 07:48:43 +0100 Subject: [PATCH 08/24] Fix Changelog --- CHANGELOG.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 77100048f2..898cfe5d2b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +### Features + +- Add Android View Hierarchy support ([#2440](https://github.com/getsentry/sentry-java/pull/2440)) + ## 6.11.0 ### Features @@ -7,7 +13,6 @@ - Disable Android concurrent profiling ([#2434](https://github.com/getsentry/sentry-java/pull/2434)) - Add logging for OpenTelemetry integration ([#2425](https://github.com/getsentry/sentry-java/pull/2425)) - Auto add `OpenTelemetryLinkErrorEventProcessor` for Spring Boot ([#2429](https://github.com/getsentry/sentry-java/pull/2429)) -- Add Android View Hierarchy support ([#2440](https://github.com/getsentry/sentry-java/pull/2440)) ### Fixes From ed189f876165a32a53e8f71757bc17b02f0097b7 Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Tue, 3 Jan 2023 16:03:50 +0100 Subject: [PATCH 09/24] Adapt modifiers / UTF-8 charset handling Based on PR comments --- .../core/CurrentActivityIntegration.java | 11 ++++---- .../core/ScreenshotEventProcessor.java | 6 ++--- .../core/ViewHierarchyEventProcessor.java | 27 ++++++++++++------- .../core/internal/gestures/ViewUtils.java | 2 +- .../internal/modules/ModulesLoader.java | 7 ++--- 5 files changed, 31 insertions(+), 22 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/CurrentActivityIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/CurrentActivityIntegration.java index da8ce117b1..b4c5f1ed02 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/CurrentActivityIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/CurrentActivityIntegration.java @@ -4,14 +4,15 @@ import android.app.Application; import android.os.Bundle; import androidx.annotation.NonNull; -import androidx.annotation.Nullable; import io.sentry.IHub; import io.sentry.Integration; import io.sentry.SentryOptions; +import io.sentry.util.Objects; import java.io.Closeable; import java.io.IOException; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; @ApiStatus.Internal public final class CurrentActivityIntegration @@ -19,8 +20,8 @@ public final class CurrentActivityIntegration private final @NotNull Application application; - public CurrentActivityIntegration(@NotNull Application application) { - this.application = application; + public CurrentActivityIntegration(final @NotNull Application application) { + this.application = Objects.requireNonNull(application, "Application is required"); } @Override @@ -67,13 +68,13 @@ public void close() throws IOException { CurrentActivityHolder.getInstance().clearActivity(); } - private void cleanCurrentActivity(@NonNull Activity activity) { + private void cleanCurrentActivity(final @NotNull Activity activity) { if (CurrentActivityHolder.getInstance().getActivity() == activity) { CurrentActivityHolder.getInstance().clearActivity(); } } - private void setCurrentActivity(@NonNull Activity activity) { + private void setCurrentActivity(final @NotNull Activity activity) { CurrentActivityHolder.getInstance().setActivity(activity); } } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ScreenshotEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/ScreenshotEventProcessor.java index 3bb6d91d8c..550634d309 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ScreenshotEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ScreenshotEventProcessor.java @@ -13,6 +13,7 @@ import io.sentry.util.Objects; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; /** * ScreenshotEventProcessor responsible for taking a screenshot of the screen when an error is @@ -32,9 +33,8 @@ public ScreenshotEventProcessor( Objects.requireNonNull(buildInfoProvider, "BuildInfoProvider is required"); } - @SuppressWarnings("NullAway") @Override - public @NotNull SentryEvent process(final @NotNull SentryEvent event, @NotNull Hint hint) { + public @NotNull SentryEvent process(final @NotNull SentryEvent event, final @NotNull Hint hint) { if (!event.isErrored()) { return event; } @@ -43,7 +43,7 @@ public ScreenshotEventProcessor( return event; } - final Activity activity = CurrentActivityHolder.getInstance().getActivity(); + final @Nullable Activity activity = CurrentActivityHolder.getInstance().getActivity(); if (activity == null || HintUtils.isFromHybridSdk(hint)) { return event; } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java index affed0bb88..a8eda88c4b 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java @@ -3,6 +3,7 @@ import android.app.Activity; import android.view.View; import android.view.ViewGroup; +import android.view.Window; import io.sentry.Attachment; import io.sentry.EventProcessor; import io.sentry.Hint; @@ -27,6 +28,9 @@ @ApiStatus.Internal public final class ViewHierarchyEventProcessor implements EventProcessor { + @SuppressWarnings("CharsetObjectCanBeUsed") + private static final @NotNull Charset UTF_8 = Charset.forName("UTF-8"); + private final @NotNull SentryAndroidOptions options; public ViewHierarchyEventProcessor(final @NotNull SentryAndroidOptions options) { @@ -34,7 +38,7 @@ public ViewHierarchyEventProcessor(final @NotNull SentryAndroidOptions options) } @Override - public @NotNull SentryEvent process(final @NotNull SentryEvent event, @NotNull Hint hint) { + public @NotNull SentryEvent process(final @NotNull SentryEvent event, final @NotNull Hint hint) { if (!event.isErrored()) { return event; } @@ -51,11 +55,8 @@ public ViewHierarchyEventProcessor(final @NotNull SentryAndroidOptions options) try { final @NotNull ViewHierarchy viewHierarchy = snapshotViewHierarchy(activity); - @SuppressWarnings("CharsetObjectCanBeUsed") - final Charset UTF8 = Charset.forName("UTF-8"); - try (final ByteArrayOutputStream stream = new ByteArrayOutputStream(); - final Writer writer = new BufferedWriter(new OutputStreamWriter(stream, UTF8))) { + final Writer writer = new BufferedWriter(new OutputStreamWriter(stream, UTF_8))) { options.getSerializer().serialize(viewHierarchy, writer); @@ -70,11 +71,16 @@ public ViewHierarchyEventProcessor(final @NotNull SentryAndroidOptions options) } @NotNull - public static ViewHierarchy snapshotViewHierarchy(Activity activity) { + public static ViewHierarchy snapshotViewHierarchy(@NotNull final Activity activity) { final List windows = new ArrayList<>(); final ViewHierarchy viewHierarchy = new ViewHierarchy("android_view_system", windows); - final @Nullable View decorView = activity.getWindow().peekDecorView(); + final @Nullable Window window = activity.getWindow(); + if (window == null) { + return viewHierarchy; + } + + final @Nullable View decorView = window.peekDecorView(); if (decorView == null) { return viewHierarchy; } @@ -86,7 +92,8 @@ public static ViewHierarchy snapshotViewHierarchy(Activity activity) { return viewHierarchy; } - private static void addChildren(@NotNull View view, @NotNull ViewHierarchyNode parentNode) { + private static void addChildren( + @NotNull final View view, @NotNull final ViewHierarchyNode parentNode) { if (!(view instanceof ViewGroup)) { return; } @@ -110,7 +117,7 @@ private static void addChildren(@NotNull View view, @NotNull ViewHierarchyNode p } @NotNull - private static ViewHierarchyNode viewToNode(final View view) { + private static ViewHierarchyNode viewToNode(@NotNull final View view) { @NotNull final ViewHierarchyNode node = new ViewHierarchyNode(); @Nullable String className = view.getClass().getCanonicalName(); @@ -122,7 +129,7 @@ private static ViewHierarchyNode viewToNode(final View view) { try { final String identifier = ViewUtils.getResourceId(view); node.setIdentifier(identifier); - } catch (Exception e) { + } catch (Throwable e) { // ignored } node.setX((double) view.getX()); diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/ViewUtils.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/ViewUtils.java index 348d552559..cb55d92214 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/ViewUtils.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/ViewUtils.java @@ -14,6 +14,7 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +@ApiStatus.Internal public final class ViewUtils { /** @@ -86,7 +87,6 @@ static String getResourceIdWithFallback(final @NotNull View view) { * @return human-readable view id * @throws Resources.NotFoundException in case the view id was not found */ - @ApiStatus.Internal public static String getResourceId(final @NotNull View view) throws Resources.NotFoundException { final int viewId = view.getId(); final Resources resources = view.getContext().getResources(); diff --git a/sentry/src/main/java/io/sentry/internal/modules/ModulesLoader.java b/sentry/src/main/java/io/sentry/internal/modules/ModulesLoader.java index 553b9279ba..8b6bc9ba37 100644 --- a/sentry/src/main/java/io/sentry/internal/modules/ModulesLoader.java +++ b/sentry/src/main/java/io/sentry/internal/modules/ModulesLoader.java @@ -16,6 +16,9 @@ @ApiStatus.Internal public abstract class ModulesLoader implements IModulesLoader { + @SuppressWarnings("CharsetObjectCanBeUsed") + private static final Charset UTF_8 = Charset.forName("UTF-8"); + public static final String EXTERNAL_MODULES_FILENAME = "sentry-external-modules.txt"; protected final @NotNull ILogger logger; private @Nullable Map cachedModules = null; @@ -35,11 +38,9 @@ public ModulesLoader(final @NotNull ILogger logger) { protected abstract Map loadModules(); - @SuppressWarnings("CharsetObjectCanBeUsed") protected Map parseStream(final @NotNull InputStream stream) { final Map modules = new TreeMap<>(); - try (final BufferedReader reader = - new BufferedReader(new InputStreamReader(stream, Charset.forName("UTF-8")))) { + try (final BufferedReader reader = new BufferedReader(new InputStreamReader(stream, UTF_8))) { String module = reader.readLine(); while (module != null) { int sep = module.lastIndexOf(':'); From c67d13238ef5fd244c5d48a3187ef9a7bf8dfcb1 Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Tue, 3 Jan 2023 16:52:57 +0100 Subject: [PATCH 10/24] Safeguard serialization of view hierarchy --- .../api/sentry-android-core.api | 2 +- .../core/ViewHierarchyEventProcessor.java | 59 +++++++++---- .../core/ViewHierarchyEventProcessorTest.kt | 85 ++++++++++++++++--- 3 files changed, 118 insertions(+), 28 deletions(-) diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index 174590b3d3..e29b172bbb 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -245,7 +245,7 @@ public final class io/sentry/android/core/UserInteractionIntegration : android/a public final class io/sentry/android/core/ViewHierarchyEventProcessor : io/sentry/EventProcessor { public fun (Lio/sentry/android/core/SentryAndroidOptions;)V public fun process (Lio/sentry/SentryEvent;Lio/sentry/Hint;)Lio/sentry/SentryEvent; - public static fun snapshotViewHierarchy (Landroid/app/Activity;)Lio/sentry/protocol/ViewHierarchy; + public static fun snapshotViewHierarchy (Landroid/view/View;)Lio/sentry/protocol/ViewHierarchy; } public final class io/sentry/android/core/cache/AndroidEnvelopeCache : io/sentry/cache/EnvelopeCache { diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java index a8eda88c4b..39186ede15 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java @@ -4,6 +4,7 @@ import android.view.View; import android.view.ViewGroup; import android.view.Window; +import androidx.annotation.NonNull; import io.sentry.Attachment; import io.sentry.EventProcessor; import io.sentry.Hint; @@ -20,6 +21,8 @@ import java.nio.charset.Charset; import java.util.ArrayList; import java.util.List; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -31,6 +34,8 @@ public final class ViewHierarchyEventProcessor implements EventProcessor { @SuppressWarnings("CharsetObjectCanBeUsed") private static final @NotNull Charset UTF_8 = Charset.forName("UTF-8"); + private static final long TIMEOUT_PROCESSING_MILLIS = 1000; + private final @NotNull SentryAndroidOptions options; public ViewHierarchyEventProcessor(final @NotNull SentryAndroidOptions options) { @@ -44,7 +49,7 @@ public ViewHierarchyEventProcessor(final @NotNull SentryAndroidOptions options) } if (!options.isAttachViewHierarchy()) { - this.options.getLogger().log(SentryLevel.DEBUG, "attachViewHierarchy is disabled."); + options.getLogger().log(SentryLevel.DEBUG, "attachViewHierarchy is disabled."); return event; } @@ -53,8 +58,40 @@ public ViewHierarchyEventProcessor(final @NotNull SentryAndroidOptions options) return event; } + final @Nullable Window window = activity.getWindow(); + if (window == null) { + return event; + } + + final @Nullable View decorView = window.peekDecorView(); + if (decorView == null) { + return event; + } + + try { + final @NotNull ViewHierarchy viewHierarchy = snapshotViewHierarchy(decorView); + final @NotNull Future future = + options + .getExecutorService() + .submit( + () -> { + try { + serializeViewHierarchy(viewHierarchy, hint); + } catch (Throwable e) { + options + .getLogger() + .log(SentryLevel.ERROR, "Failed trying to serialize view hierarchy.", e); + } + }); + future.get(TIMEOUT_PROCESSING_MILLIS, TimeUnit.MILLISECONDS); + } catch (Throwable t) { + options.getLogger().log(SentryLevel.ERROR, "Failed to process view hierarchy.", t); + } + return event; + } + + private void serializeViewHierarchy(@NonNull ViewHierarchy viewHierarchy, @NonNull Hint hint) { try { - final @NotNull ViewHierarchy viewHierarchy = snapshotViewHierarchy(activity); try (final ByteArrayOutputStream stream = new ByteArrayOutputStream(); final Writer writer = new BufferedWriter(new OutputStreamWriter(stream, UTF_8))) { @@ -66,28 +103,16 @@ public ViewHierarchyEventProcessor(final @NotNull SentryAndroidOptions options) } catch (Throwable t) { options.getLogger().log(SentryLevel.ERROR, "Could not snapshot ViewHierarchy", t); } - - return event; } @NotNull - public static ViewHierarchy snapshotViewHierarchy(@NotNull final Activity activity) { + public static ViewHierarchy snapshotViewHierarchy(@NotNull final View view) { final List windows = new ArrayList<>(); final ViewHierarchy viewHierarchy = new ViewHierarchy("android_view_system", windows); - final @Nullable Window window = activity.getWindow(); - if (window == null) { - return viewHierarchy; - } - - final @Nullable View decorView = window.peekDecorView(); - if (decorView == null) { - return viewHierarchy; - } - - final @NotNull ViewHierarchyNode decorNode = viewToNode(decorView); + final @NotNull ViewHierarchyNode decorNode = viewToNode(view); windows.add(decorNode); - addChildren(decorView, decorNode); + addChildren(view, decorNode); return viewHierarchy; } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ViewHierarchyEventProcessorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ViewHierarchyEventProcessorTest.kt index e7626b669c..8fc704a568 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ViewHierarchyEventProcessorTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ViewHierarchyEventProcessorTest.kt @@ -6,12 +6,16 @@ import android.view.ViewGroup import android.view.Window import androidx.test.ext.junit.runners.AndroidJUnit4 import io.sentry.Hint +import io.sentry.ISentryExecutorService import io.sentry.SentryEvent import io.sentry.protocol.SentryException import org.junit.runner.RunWith import org.mockito.kotlin.mock import org.mockito.kotlin.whenever -import java.lang.IllegalStateException +import java.util.concurrent.Callable +import java.util.concurrent.Executors +import java.util.concurrent.Future +import java.util.concurrent.TimeUnit import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals @@ -100,7 +104,7 @@ class ViewHierarchyEventProcessorTest { } @Test - fun `when there's no current activity the viewhierarchy is null`() { + fun `when there's no current activity the view hierarchy is null`() { CurrentActivityHolder.getInstance().clearActivity() val (event, hint) = fixture.process( @@ -115,7 +119,37 @@ class ViewHierarchyEventProcessorTest { } @Test - fun `when retrieving the viewHierarchy crashes no view hierarchy is collected`() { + fun `when there's no current window the view hierarchy is null`() { + whenever(fixture.activity.window).thenReturn(null) + + val (event, hint) = fixture.process( + true, + SentryEvent().apply { + exceptions = listOf(SentryException()) + } + ) + + assertNotNull(event) + assertNull(hint.viewHierarchy) + } + + @Test + fun `when there's no current decor view the view hierarchy is null`() { + whenever(fixture.window.peekDecorView()).thenReturn(null) + + val (event, hint) = fixture.process( + true, + SentryEvent().apply { + exceptions = listOf(SentryException()) + } + ) + + assertNotNull(event) + assertNull(hint.viewHierarchy) + } + + @Test + fun `when retrieving the view hierarchy crashes no view hierarchy is collected`() { whenever(fixture.view.width).thenThrow(IllegalStateException("invalid ui state")) val (event, hint) = fixture.process( true, @@ -143,13 +177,7 @@ class ViewHierarchyEventProcessorTest { ) ) - val activity = mock() - val window = mock() - whenever(window.decorView).thenReturn(content) - whenever(window.peekDecorView()).thenReturn(content) - whenever(activity.window).thenReturn(window) - - val viewHierarchy = ViewHierarchyEventProcessor.snapshotViewHierarchy(activity) + val viewHierarchy = ViewHierarchyEventProcessor.snapshotViewHierarchy(content) assertEquals("android_view_system", viewHierarchy.renderingSystem) assertEquals(1, viewHierarchy.windows!!.size) @@ -180,6 +208,43 @@ class ViewHierarchyEventProcessorTest { } } + @Test + fun `if serialization of view hierarchy takes too long, it does not get attached`() { + fixture.options.executorService = object : ISentryExecutorService { + val service = Executors.newSingleThreadScheduledExecutor() + override fun submit(runnable: Runnable): Future<*> { + service.submit { + Thread.sleep(2000L) + } + return service.submit(runnable) + } + + override fun submit(callable: Callable): Future { + service.submit { + Thread.sleep(2000L) + } + return service.submit(callable) + } + + override fun schedule(runnable: Runnable, delayMillis: Long): Future<*> { + return service.schedule(runnable, delayMillis, TimeUnit.MILLISECONDS) + } + + override fun close(timeoutMillis: Long) { + service.shutdown() + } + } + val (event, hint) = fixture.process( + true, + SentryEvent().apply { + exceptions = listOf(SentryException()) + } + ) + + assertNotNull(event) + assertNull(hint.viewHierarchy) + } + private fun mockedView( x: Float, y: Float, From 6f6a909e3a3eb3be4204f1635ce1f58a9c87fcc4 Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Wed, 4 Jan 2023 07:40:14 +0100 Subject: [PATCH 11/24] Update sentry/src/main/java/io/sentry/Hint.java Co-authored-by: Roman Zavarnitsyn --- sentry/src/main/java/io/sentry/Hint.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry/src/main/java/io/sentry/Hint.java b/sentry/src/main/java/io/sentry/Hint.java index 689860b3e5..ab945c5605 100644 --- a/sentry/src/main/java/io/sentry/Hint.java +++ b/sentry/src/main/java/io/sentry/Hint.java @@ -118,7 +118,7 @@ public void setScreenshot(@Nullable Attachment screenshot) { return screenshot; } - public void setViewHierarchy(@Nullable Attachment viewHierarchy) { + public void setViewHierarchy(final @Nullable Attachment viewHierarchy) { this.viewHierarchy = viewHierarchy; } From 1574530bc9e64d3c2c20162f9f66b0a09ca80ab4 Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Wed, 4 Jan 2023 07:40:33 +0100 Subject: [PATCH 12/24] Update sentry/src/main/java/io/sentry/protocol/ViewHierarchy.java Co-authored-by: Roman Zavarnitsyn --- sentry/src/main/java/io/sentry/protocol/ViewHierarchy.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry/src/main/java/io/sentry/protocol/ViewHierarchy.java b/sentry/src/main/java/io/sentry/protocol/ViewHierarchy.java index 12e5bf7931..9a24ccc835 100644 --- a/sentry/src/main/java/io/sentry/protocol/ViewHierarchy.java +++ b/sentry/src/main/java/io/sentry/protocol/ViewHierarchy.java @@ -26,7 +26,7 @@ public static final class JsonKeys { private @Nullable Map unknown; public ViewHierarchy( - @Nullable String renderingSystem, @Nullable List windows) { + final @Nullable String renderingSystem, final @Nullable List windows) { this.renderingSystem = renderingSystem; this.windows = windows; } From b4463cfb9c5e713621560114e39ae1c7dbd161f0 Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Wed, 4 Jan 2023 07:40:42 +0100 Subject: [PATCH 13/24] Update CHANGELOG.md Co-authored-by: Roman Zavarnitsyn --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 898cfe5d2b..369ac87043 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ ### Features -- Add Android View Hierarchy support ([#2440](https://github.com/getsentry/sentry-java/pull/2440)) +- Attach View Hierarchy to the errored/crashed events ([#2440](https://github.com/getsentry/sentry-java/pull/2440)) ## 6.11.0 From cbffc5c869ffc8a6fd22921420ca3763540074be Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Wed, 4 Jan 2023 07:55:02 +0100 Subject: [PATCH 14/24] Improve structure based on PR comments --- .../android/core/AndroidOptionsInitializer.java | 3 +-- .../android/core/ViewHierarchyEventProcessor.java | 11 +++++++---- .../android/core/AndroidOptionsInitializerTest.kt | 8 ++++++++ 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java index a5c06981e2..2faf2729ac 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java @@ -134,7 +134,7 @@ static void initializeIntegrationsAndProcessors( options.addEventProcessor( new DefaultAndroidEventProcessor(context, buildInfoProvider, options)); options.addEventProcessor(new PerformanceAndroidEventProcessor(options, activityFramesTracker)); - + options.addEventProcessor(new ViewHierarchyEventProcessor(options)); options.setTransportGate(new AndroidTransportGate(context, options.getLogger())); final SentryFrameMetricsCollector frameMetricsCollector = new SentryFrameMetricsCollector(context, options, buildInfoProvider); @@ -223,7 +223,6 @@ private static void installDefaultIntegrations( SentryLevel.WARNING, "ActivityLifecycle, FragmentLifecycle and UserInteraction Integrations need an Application class to be installed."); } - options.addEventProcessor(new ViewHierarchyEventProcessor(options)); if (isTimberAvailable) { options.addIntegration(new SentryTimberIntegration()); diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java index 39186ede15..09ca686381 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java @@ -55,15 +55,18 @@ public ViewHierarchyEventProcessor(final @NotNull SentryAndroidOptions options) final @Nullable Activity activity = CurrentActivityHolder.getInstance().getActivity(); if (activity == null) { + options.getLogger().log(SentryLevel.INFO, "Missing activity for view hierarchy snapshot."); return event; } final @Nullable Window window = activity.getWindow(); if (window == null) { + options.getLogger().log(SentryLevel.INFO, "Missing window for view hierarchy snapshot."); return event; } final @Nullable View decorView = window.peekDecorView(); + options.getLogger().log(SentryLevel.INFO, "Missing decor view for view hierarchy snapshot."); if (decorView == null) { return event; } @@ -107,12 +110,12 @@ private void serializeViewHierarchy(@NonNull ViewHierarchy viewHierarchy, @NonNu @NotNull public static ViewHierarchy snapshotViewHierarchy(@NotNull final View view) { - final List windows = new ArrayList<>(); + final List windows = new ArrayList<>(1); final ViewHierarchy viewHierarchy = new ViewHierarchy("android_view_system", windows); - final @NotNull ViewHierarchyNode decorNode = viewToNode(view); - windows.add(decorNode); - addChildren(view, decorNode); + final @NotNull ViewHierarchyNode node = viewToNode(view); + windows.add(node); + addChildren(view, node); return viewHierarchy; } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt index eba2fd4ae2..f9dce2ca21 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt @@ -166,6 +166,14 @@ class AndroidOptionsInitializerTest { assertNotNull(actual) } + @Test + fun `ViewHierarchyEventProcessor added to processors list`() { + fixture.initSut() + val actual = + fixture.sentryOptions.eventProcessors.any { it is ViewHierarchyEventProcessor } + assertNotNull(actual) + } + @Test fun `envelopesDir should be set at initialization`() { fixture.initSut() From 9f65ca67dcb13f9cd30d7e2e7028bb5e5dc7b8ea Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Thu, 5 Jan 2023 09:06:48 +0100 Subject: [PATCH 15/24] Add bytesFactory field to attachment Useful to postphone serialization of attachment payload to the point when it's needed --- .../core/ViewHierarchyEventProcessor.java | 47 +++++++------------ .../core/ViewHierarchyEventProcessorTest.kt | 42 ----------------- sentry/api/sentry.api | 4 +- .../src/main/java/io/sentry/Attachment.java | 46 ++++++++++++++++-- .../java/io/sentry/SentryEnvelopeItem.java | 33 ++++++++----- .../src/test/java/io/sentry/AttachmentTest.kt | 7 +-- .../test/java/io/sentry/SentryClientTest.kt | 4 +- .../java/io/sentry/SentryEnvelopeItemTest.kt | 25 ++++++++++ 8 files changed, 116 insertions(+), 92 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java index 09ca686381..f3dbaca324 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java @@ -21,8 +21,6 @@ import java.nio.charset.Charset; import java.util.ArrayList; import java.util.List; -import java.util.concurrent.Future; -import java.util.concurrent.TimeUnit; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -73,39 +71,30 @@ public ViewHierarchyEventProcessor(final @NotNull SentryAndroidOptions options) try { final @NotNull ViewHierarchy viewHierarchy = snapshotViewHierarchy(decorView); - final @NotNull Future future = - options - .getExecutorService() - .submit( - () -> { - try { - serializeViewHierarchy(viewHierarchy, hint); - } catch (Throwable e) { - options - .getLogger() - .log(SentryLevel.ERROR, "Failed trying to serialize view hierarchy.", e); - } - }); - future.get(TIMEOUT_PROCESSING_MILLIS, TimeUnit.MILLISECONDS); + attachViewHierarchy(viewHierarchy, hint); } catch (Throwable t) { options.getLogger().log(SentryLevel.ERROR, "Failed to process view hierarchy.", t); } return event; } - private void serializeViewHierarchy(@NonNull ViewHierarchy viewHierarchy, @NonNull Hint hint) { - try { - try (final ByteArrayOutputStream stream = new ByteArrayOutputStream(); - final Writer writer = new BufferedWriter(new OutputStreamWriter(stream, UTF_8))) { - - options.getSerializer().serialize(viewHierarchy, writer); - - final Attachment attachment = Attachment.fromViewHierarchy(stream.toByteArray()); - hint.setViewHierarchy(attachment); - } - } catch (Throwable t) { - options.getLogger().log(SentryLevel.ERROR, "Could not snapshot ViewHierarchy", t); - } + private void attachViewHierarchy(@NonNull ViewHierarchy viewHierarchy, @NonNull Hint hint) { + hint.setViewHierarchy( + Attachment.fromViewHierarchy( + () -> { + try { + try (final ByteArrayOutputStream stream = new ByteArrayOutputStream(); + final Writer writer = + new BufferedWriter(new OutputStreamWriter(stream, UTF_8))) { + + options.getSerializer().serialize(viewHierarchy, writer); + return stream.toByteArray(); + } + } catch (Throwable t) { + options.getLogger().log(SentryLevel.ERROR, "Could not serialize ViewHierarchy", t); + throw t; + } + })); } @NotNull diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ViewHierarchyEventProcessorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ViewHierarchyEventProcessorTest.kt index 8fc704a568..76b56cf740 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ViewHierarchyEventProcessorTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ViewHierarchyEventProcessorTest.kt @@ -6,16 +6,11 @@ import android.view.ViewGroup import android.view.Window import androidx.test.ext.junit.runners.AndroidJUnit4 import io.sentry.Hint -import io.sentry.ISentryExecutorService import io.sentry.SentryEvent import io.sentry.protocol.SentryException import org.junit.runner.RunWith import org.mockito.kotlin.mock import org.mockito.kotlin.whenever -import java.util.concurrent.Callable -import java.util.concurrent.Executors -import java.util.concurrent.Future -import java.util.concurrent.TimeUnit import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals @@ -208,43 +203,6 @@ class ViewHierarchyEventProcessorTest { } } - @Test - fun `if serialization of view hierarchy takes too long, it does not get attached`() { - fixture.options.executorService = object : ISentryExecutorService { - val service = Executors.newSingleThreadScheduledExecutor() - override fun submit(runnable: Runnable): Future<*> { - service.submit { - Thread.sleep(2000L) - } - return service.submit(runnable) - } - - override fun submit(callable: Callable): Future { - service.submit { - Thread.sleep(2000L) - } - return service.submit(callable) - } - - override fun schedule(runnable: Runnable, delayMillis: Long): Future<*> { - return service.schedule(runnable, delayMillis, TimeUnit.MILLISECONDS) - } - - override fun close(timeoutMillis: Long) { - service.shutdown() - } - } - val (event, hint) = fixture.process( - true, - SentryEvent().apply { - exceptions = listOf(SentryException()) - } - ) - - assertNotNull(event) - assertNull(hint.viewHierarchy) - } - private fun mockedView( x: Float, y: Float, diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index d6185a7711..a13c63cbcd 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -10,14 +10,16 @@ public final class io/sentry/Attachment { public fun (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Z)V public fun (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Z)V public fun (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ZLjava/lang/String;)V + public fun (Ljava/util/concurrent/Callable;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Z)V public fun ([BLjava/lang/String;)V public fun ([BLjava/lang/String;Ljava/lang/String;)V public fun ([BLjava/lang/String;Ljava/lang/String;Ljava/lang/String;Z)V public fun ([BLjava/lang/String;Ljava/lang/String;Z)V public static fun fromScreenshot ([B)Lio/sentry/Attachment; - public static fun fromViewHierarchy ([B)Lio/sentry/Attachment; + public static fun fromViewHierarchy (Ljava/util/concurrent/Callable;)Lio/sentry/Attachment; public fun getAttachmentType ()Ljava/lang/String; public fun getBytes ()[B + public fun getBytesFactory ()Ljava/util/concurrent/Callable; public fun getContentType ()Ljava/lang/String; public fun getFilename ()Ljava/lang/String; public fun getPathname ()Ljava/lang/String; diff --git a/sentry/src/main/java/io/sentry/Attachment.java b/sentry/src/main/java/io/sentry/Attachment.java index 01506095af..fe7cdb58d6 100644 --- a/sentry/src/main/java/io/sentry/Attachment.java +++ b/sentry/src/main/java/io/sentry/Attachment.java @@ -1,6 +1,7 @@ package io.sentry; import java.io.File; +import java.util.concurrent.Callable; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -8,6 +9,7 @@ public final class Attachment { private @Nullable byte[] bytes; + private final @Nullable Callable bytesFactory; private @Nullable String pathname; private final @NotNull String filename; private final @Nullable String contentType; @@ -80,6 +82,32 @@ public Attachment( final @Nullable String attachmentType, final boolean addToTransactions) { this.bytes = bytes; + this.bytesFactory = null; + this.filename = filename; + this.contentType = contentType; + this.attachmentType = attachmentType; + this.addToTransactions = addToTransactions; + } + + /** + * Initializes an Attachment with bytes factory, a filename, a content type, and + * addToTransactions. + * + * @param bytesFactory The bytes factory providing the data when being called + * @param filename The name of the attachment to display in Sentry. + * @param contentType The content type of the attachment. + * @param attachmentType the attachment type. + * @param addToTransactions true if the SDK should add this attachment to every + * {@link ITransaction} or set to false if it shouldn't. + */ + public Attachment( + final @NotNull Callable bytesFactory, + final @NotNull String filename, + final @Nullable String contentType, + final @Nullable String attachmentType, + final boolean addToTransactions) { + this.bytes = null; + this.bytesFactory = bytesFactory; this.filename = filename; this.contentType = contentType; this.attachmentType = attachmentType; @@ -156,6 +184,7 @@ public Attachment( final boolean addToTransactions) { this.pathname = pathname; this.filename = filename; + this.bytesFactory = null; this.contentType = contentType; this.attachmentType = attachmentType; this.addToTransactions = addToTransactions; @@ -181,6 +210,7 @@ public Attachment( final boolean addToTransactions) { this.pathname = pathname; this.filename = filename; + this.bytesFactory = null; this.contentType = contentType; this.addToTransactions = addToTransactions; } @@ -208,6 +238,7 @@ public Attachment( final @Nullable String attachmentType) { this.pathname = pathname; this.filename = filename; + this.bytesFactory = null; this.contentType = contentType; this.addToTransactions = addToTransactions; this.attachmentType = attachmentType; @@ -222,6 +253,15 @@ public Attachment( return bytes; } + /** + * Provides the bytes of the attachment. + * + * @return the bytes factory responsible for providing the bytes. + */ + public @Nullable Callable getBytesFactory() { + return bytesFactory; + } + /** * Gets the pathname string of the attachment. * @@ -282,12 +322,12 @@ boolean isAddToTransactions() { /** * Creates a new View Hierarchy Attachment * - * @param viewHierarchyBytes the serialized View Hierarchy + * @param bytesFactory the serialized View Hierarchy * @return the Attachment */ - public static @NotNull Attachment fromViewHierarchy(final byte[] viewHierarchyBytes) { + public static @NotNull Attachment fromViewHierarchy(final Callable bytesFactory) { return new Attachment( - viewHierarchyBytes, + bytesFactory, "view-hierarchy.json", "application/json", DEFAULT_ATTACHMENT_TYPE, // TODO replace with VIEW_HIERARCHY_ATTACHMENT_TYPE, diff --git a/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java b/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java index 56fe30ebdf..7ee132f541 100644 --- a/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java +++ b/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java @@ -171,25 +171,20 @@ public static SentryEnvelopeItem fromAttachment( new CachedItem( () -> { if (attachment.getBytes() != null) { - if (attachment.getBytes().length > maxAttachmentSize) { - throw new SentryEnvelopeException( - String.format( - "Dropping attachment with filename '%s', because the " - + "size of the passed bytes with %d bytes is bigger " - + "than the maximum allowed attachment size of " - + "%d bytes.", - attachment.getFilename(), - attachment.getBytes().length, - maxAttachmentSize)); - } + ensureAttachmentSizeLimit( + attachment.getBytes().length, maxAttachmentSize, attachment.getFilename()); return attachment.getBytes(); + } else if (attachment.getBytesFactory() != null) { + final byte[] data = attachment.getBytesFactory().call(); + ensureAttachmentSizeLimit(data.length, maxAttachmentSize, attachment.getFilename()); + return data; } else if (attachment.getPathname() != null) { return readBytesFromFile(attachment.getPathname(), maxAttachmentSize); } throw new SentryEnvelopeException( String.format( "Couldn't attach the attachment %s.\n" - + "Please check that either bytes or a path is set.", + + "Please check that either bytes, bytesFactory or a path is set.", attachment.getFilename())); }); @@ -205,6 +200,20 @@ public static SentryEnvelopeItem fromAttachment( return new SentryEnvelopeItem(itemHeader, () -> cachedItem.getBytes()); } + private static void ensureAttachmentSizeLimit( + final long size, final long maxAttachmentSize, final @NotNull String filename) + throws SentryEnvelopeException { + if (size > maxAttachmentSize) { + throw new SentryEnvelopeException( + String.format( + "Dropping attachment with filename '%s', because the " + + "size of the passed bytes with %d bytes is bigger " + + "than the maximum allowed attachment size of " + + "%d bytes.", + filename, size, maxAttachmentSize)); + } + } + public static @NotNull SentryEnvelopeItem fromProfilingTrace( final @NotNull ProfilingTraceData profilingTraceData, final long maxTraceFileSize, diff --git a/sentry/src/test/java/io/sentry/AttachmentTest.kt b/sentry/src/test/java/io/sentry/AttachmentTest.kt index 341eba4b19..2400c9fbd9 100644 --- a/sentry/src/test/java/io/sentry/AttachmentTest.kt +++ b/sentry/src/test/java/io/sentry/AttachmentTest.kt @@ -115,14 +115,15 @@ class AttachmentTest { @Test fun `creates attachment from view hierarchy`() { - val bytes = byteArrayOf() - val attachment = Attachment.fromViewHierarchy(bytes) + val bytes = byteArrayOf(1, 2, 3) + val attachment = Attachment.fromViewHierarchy { bytes } assertEquals("view-hierarchy.json", attachment.filename) assertEquals("application/json", attachment.contentType) assertEquals(false, attachment.isAddToTransactions) // TODO replace with event.view_hierarchy assertEquals("event.attachment", attachment.attachmentType) - assertEquals(bytes, attachment.bytes) + assertNull(attachment.bytes) + assertEquals(bytes, attachment.bytesFactory!!.call()) } } diff --git a/sentry/src/test/java/io/sentry/SentryClientTest.kt b/sentry/src/test/java/io/sentry/SentryClientTest.kt index 64aa7316b5..cf9eb52085 100644 --- a/sentry/src/test/java/io/sentry/SentryClientTest.kt +++ b/sentry/src/test/java/io/sentry/SentryClientTest.kt @@ -1423,7 +1423,7 @@ class SentryClientTest { @Test fun `view hierarchy is added to the envelope from the hint`() { val sut = fixture.getSut() - val attachment = Attachment.fromViewHierarchy(byteArrayOf()) + val attachment = Attachment.fromViewHierarchy { byteArrayOf() } val hint = Hint().also { it.viewHierarchy = attachment } sut.captureEvent(SentryEvent(), hint) @@ -1443,7 +1443,7 @@ class SentryClientTest { fun `view hierarchy is dropped from hint via before send`() { fixture.sentryOptions.beforeSend = CustomBeforeSendCallback() val sut = fixture.getSut() - val attachment = Attachment.fromViewHierarchy(byteArrayOf()) + val attachment = Attachment.fromViewHierarchy { byteArrayOf() } val hint = Hint().also { it.viewHierarchy = attachment } sut.captureEvent(SentryEvent(), hint) diff --git a/sentry/src/test/java/io/sentry/SentryEnvelopeItemTest.kt b/sentry/src/test/java/io/sentry/SentryEnvelopeItemTest.kt index 896445554f..40b4e090c7 100644 --- a/sentry/src/test/java/io/sentry/SentryEnvelopeItemTest.kt +++ b/sentry/src/test/java/io/sentry/SentryEnvelopeItemTest.kt @@ -58,6 +58,15 @@ class SentryEnvelopeItemTest { assertAttachment(attachment, fixture.bytesAllowed, item) } + @Test + fun `fromAttachment with bytes factory`() { + val attachment = Attachment({ fixture.bytesAllowed }, fixture.filename, "text/plain", null, false) + + val item = SentryEnvelopeItem.fromAttachment(attachment, fixture.maxAttachmentSize) + + assertAttachment(attachment, fixture.bytesAllowed, item) + } + @Test fun `fromAttachment with attachmentType`() { val attachment = Attachment(fixture.pathname, fixture.filename, "", true, "event.minidump") @@ -191,6 +200,22 @@ class SentryEnvelopeItemTest { ) } + @Test + fun `fromAttachment with bytes factory too big`() { + val attachment = Attachment({ fixture.bytesTooBig }, fixture.filename, "text/plain", null, false) + val exception = assertFailsWith { + SentryEnvelopeItem.fromAttachment(attachment, fixture.maxAttachmentSize).data + } + + assertEquals( + "Dropping attachment with filename '${fixture.filename}', because the " + + "size of the passed bytes with ${fixture.bytesTooBig.size} bytes is bigger " + + "than the maximum allowed attachment size of " + + "${fixture.maxAttachmentSize} bytes.", + exception.message + ) + } + @Test fun `fromAttachment with file too big`() { val file = File(fixture.pathname) From 040e1a0f01fea3fc36d76be718f8ce56776a58da Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Tue, 10 Jan 2023 11:21:35 +0100 Subject: [PATCH 16/24] Use visibility attribute on Android --- .../core/ViewHierarchyEventProcessor.java | 18 ++++++++++-- .../core/ViewHierarchyEventProcessorTest.kt | 8 +++--- sentry/api/sentry.api | 6 ++-- .../io/sentry/protocol/ViewHierarchyNode.java | 20 ++++++------- .../ViewHierarchyNodeSerializationTest.kt | 28 +++++++++---------- .../resources/json/view_hierarchy_node.json | 2 +- 6 files changed, 46 insertions(+), 36 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java index f3dbaca324..26c4405d64 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java @@ -32,8 +32,6 @@ public final class ViewHierarchyEventProcessor implements EventProcessor { @SuppressWarnings("CharsetObjectCanBeUsed") private static final @NotNull Charset UTF_8 = Charset.forName("UTF-8"); - private static final long TIMEOUT_PROCESSING_MILLIS = 1000; - private final @NotNull SentryAndroidOptions options; public ViewHierarchyEventProcessor(final @NotNull SentryAndroidOptions options) { @@ -154,7 +152,21 @@ private static ViewHierarchyNode viewToNode(@NotNull final View view) { node.setWidth((double) view.getWidth()); node.setHeight((double) view.getHeight()); node.setAlpha((double) view.getAlpha()); - node.setVisible(view.getVisibility() == View.VISIBLE); + + switch (view.getVisibility()) { + case View.VISIBLE: + node.setVisibility("visible"); + break; + case View.INVISIBLE: + node.setVisibility("invisible"); + break; + case View.GONE: + node.setVisibility("gone"); + break; + default: + // ignored + break; + } return node; } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ViewHierarchyEventProcessorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ViewHierarchyEventProcessorTest.kt index 76b56cf740..4fd03b761b 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ViewHierarchyEventProcessorTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ViewHierarchyEventProcessorTest.kt @@ -167,7 +167,7 @@ class ViewHierarchyEventProcessorTest { 1f, View.VISIBLE, listOf( - mockedView(10.0f, 11.0f, 100, 101, 0.5f, View.VISIBLE), + mockedView(10.0f, 11.0f, 100, 101, 0.5f, View.GONE), mockedView(20.0f, 21.0f, 200, 201, 1f, View.INVISIBLE) ) ) @@ -181,7 +181,7 @@ class ViewHierarchyEventProcessorTest { assertEquals(400.0, contentNode.height) assertEquals(0.0, contentNode.x) assertEquals(1.0, contentNode.y) - assertEquals(true, contentNode.visible) + assertEquals("visible", contentNode.visibility) assertEquals(2, contentNode.children!!.size) contentNode.children!![0].apply { @@ -189,7 +189,7 @@ class ViewHierarchyEventProcessorTest { assertEquals(101.0, height) assertEquals(10.0, x) assertEquals(11.0, y) - assertEquals(true, visible) + assertEquals("gone", visibility) assertEquals(null, children) } @@ -198,7 +198,7 @@ class ViewHierarchyEventProcessorTest { assertEquals(201.0, height) assertEquals(20.0, x) assertEquals(21.0, y) - assertEquals(false, visible) + assertEquals("invisible", visibility) assertEquals(null, children) } } diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index a13c63cbcd..7a3000dbcc 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -3409,7 +3409,7 @@ public final class io/sentry/protocol/ViewHierarchyNode : io/sentry/JsonSerializ public fun getTag ()Ljava/lang/String; public fun getType ()Ljava/lang/String; public fun getUnknown ()Ljava/util/Map; - public fun getVisible ()Ljava/lang/Boolean; + public fun getVisibility ()Ljava/lang/String; public fun getWidth ()Ljava/lang/Double; public fun getX ()Ljava/lang/Double; public fun getY ()Ljava/lang/Double; @@ -3422,7 +3422,7 @@ public final class io/sentry/protocol/ViewHierarchyNode : io/sentry/JsonSerializ public fun setTag (Ljava/lang/String;)V public fun setType (Ljava/lang/String;)V public fun setUnknown (Ljava/util/Map;)V - public fun setVisible (Ljava/lang/Boolean;)V + public fun setVisibility (Ljava/lang/String;)V public fun setWidth (Ljava/lang/Double;)V public fun setX (Ljava/lang/Double;)V public fun setY (Ljava/lang/Double;)V @@ -3442,7 +3442,7 @@ public final class io/sentry/protocol/ViewHierarchyNode$JsonKeys { public static final field RENDERING_SYSTEM Ljava/lang/String; public static final field TAG Ljava/lang/String; public static final field TYPE Ljava/lang/String; - public static final field VISIBLE Ljava/lang/String; + public static final field VISIBILITY Ljava/lang/String; public static final field WIDTH Ljava/lang/String; public static final field X Ljava/lang/String; public static final field Y Ljava/lang/String; diff --git a/sentry/src/main/java/io/sentry/protocol/ViewHierarchyNode.java b/sentry/src/main/java/io/sentry/protocol/ViewHierarchyNode.java index fcc4353edd..d751b27a80 100644 --- a/sentry/src/main/java/io/sentry/protocol/ViewHierarchyNode.java +++ b/sentry/src/main/java/io/sentry/protocol/ViewHierarchyNode.java @@ -25,7 +25,7 @@ public static final class JsonKeys { public static final String HEIGHT = "height"; public static final String X = "x"; public static final String Y = "y"; - public static final String VISIBLE = "visible"; + public static final String VISIBILITY = "visibility"; public static final String ALPHA = "alpha"; public static final String CHILDREN = "children"; } @@ -38,7 +38,7 @@ public static final class JsonKeys { private @Nullable Double height; private @Nullable Double x; private @Nullable Double y; - private @Nullable Boolean visible; + private @Nullable String visibility; private @Nullable Double alpha; private @Nullable List children; private @Nullable Map unknown; @@ -77,8 +77,8 @@ public void setY(final @Nullable Double y) { this.y = y; } - public void setVisible(final @Nullable Boolean visible) { - this.visible = visible; + public void setVisibility(final @Nullable String visibility) { + this.visibility = visibility; } public void setAlpha(final @Nullable Double alpha) { @@ -130,8 +130,8 @@ public Double getY() { } @Nullable - public Boolean getVisible() { - return visible; + public String getVisibility() { + return visibility; } @Nullable @@ -172,8 +172,8 @@ public void serialize(@NotNull JsonObjectWriter writer, @NotNull ILogger logger) if (y != null) { writer.name(JsonKeys.Y).value(y); } - if (visible != null) { - writer.name(JsonKeys.VISIBLE).value(visible); + if (visibility != null) { + writer.name(JsonKeys.VISIBILITY).value(visibility); } if (alpha != null) { writer.name(JsonKeys.ALPHA).value(alpha); @@ -237,8 +237,8 @@ public static final class Deserializer implements JsonDeserializer() fun getSut() = ViewHierarchyNode().apply { - setType("com.example.ui.FancyButton") - setIdentifier("button_logout") - setChildren( - listOf( - ViewHierarchyNode().apply { - setRenderingSystem("compose") - setType("Clickable") - } - ) + type = "com.example.ui.FancyButton" + identifier = "button_logout" + children = listOf( + ViewHierarchyNode().apply { + renderingSystem = "compose" + type = "Clickable" + } ) - setWidth(100.0) - setHeight(200.0) - setX(0.0) - setY(2.0) - setVisible(true) - setAlpha(1.0) + width = 100.0 + height = 200.0 + x = 0.0 + y = 2.0 + visibility = "visible" + alpha = 1.0 unknown = mapOf( "extra_property" to 42 ) diff --git a/sentry/src/test/resources/json/view_hierarchy_node.json b/sentry/src/test/resources/json/view_hierarchy_node.json index adb6daafc6..d7978618a1 100644 --- a/sentry/src/test/resources/json/view_hierarchy_node.json +++ b/sentry/src/test/resources/json/view_hierarchy_node.json @@ -5,7 +5,7 @@ "height": 200.0, "x": 0.0, "y": 2.0, - "visible": true, + "visibility": "visible", "alpha": 1.0, "children": [ { From 52af78f902ce525c9ff2c8f4278c5cff7775823b Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Tue, 10 Jan 2023 11:21:55 +0100 Subject: [PATCH 17/24] Move ScreenshotEventProcessor init --- .../java/io/sentry/android/core/AndroidOptionsInitializer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java index 2faf2729ac..7046539efe 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java @@ -134,6 +134,7 @@ static void initializeIntegrationsAndProcessors( options.addEventProcessor( new DefaultAndroidEventProcessor(context, buildInfoProvider, options)); options.addEventProcessor(new PerformanceAndroidEventProcessor(options, activityFramesTracker)); + options.addEventProcessor(new ScreenshotEventProcessor(options, buildInfoProvider)); options.addEventProcessor(new ViewHierarchyEventProcessor(options)); options.setTransportGate(new AndroidTransportGate(context, options.getLogger())); final SentryFrameMetricsCollector frameMetricsCollector = @@ -215,7 +216,6 @@ private static void installDefaultIntegrations( if (isFragmentAvailable) { options.addIntegration(new FragmentLifecycleIntegration((Application) context, true, true)); } - options.addEventProcessor(new ScreenshotEventProcessor(options, buildInfoProvider)); } else { options .getLogger() From 6077f2895d2dc9be354912f24c88eb773a2a3436 Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Tue, 10 Jan 2023 11:22:16 +0100 Subject: [PATCH 18/24] Set proper attachment type for view hierarchy --- sentry/src/main/java/io/sentry/Attachment.java | 5 +++-- sentry/src/test/java/io/sentry/AttachmentTest.kt | 3 +-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/sentry/src/main/java/io/sentry/Attachment.java b/sentry/src/main/java/io/sentry/Attachment.java index fe7cdb58d6..62331b06a5 100644 --- a/sentry/src/main/java/io/sentry/Attachment.java +++ b/sentry/src/main/java/io/sentry/Attachment.java @@ -20,7 +20,8 @@ public final class Attachment { /** A standard attachment without special meaning */ private static final String DEFAULT_ATTACHMENT_TYPE = "event.attachment"; - // private static final String VIEW_HIERARCHY_ATTACHMENT_TYPE = "event.view_hierarchy"; + + private static final String VIEW_HIERARCHY_ATTACHMENT_TYPE = "event.view_hierarchy"; /** * Initializes an Attachment with bytes and a filename. Sets addToTransaction to false @@ -330,7 +331,7 @@ boolean isAddToTransactions() { bytesFactory, "view-hierarchy.json", "application/json", - DEFAULT_ATTACHMENT_TYPE, // TODO replace with VIEW_HIERARCHY_ATTACHMENT_TYPE, + VIEW_HIERARCHY_ATTACHMENT_TYPE, false); } } diff --git a/sentry/src/test/java/io/sentry/AttachmentTest.kt b/sentry/src/test/java/io/sentry/AttachmentTest.kt index 2400c9fbd9..65838016a9 100644 --- a/sentry/src/test/java/io/sentry/AttachmentTest.kt +++ b/sentry/src/test/java/io/sentry/AttachmentTest.kt @@ -121,8 +121,7 @@ class AttachmentTest { assertEquals("view-hierarchy.json", attachment.filename) assertEquals("application/json", attachment.contentType) assertEquals(false, attachment.isAddToTransactions) - // TODO replace with event.view_hierarchy - assertEquals("event.attachment", attachment.attachmentType) + assertEquals("event.view_hierarchy", attachment.attachmentType) assertNull(attachment.bytes) assertEquals(bytes, attachment.bytesFactory!!.call()) } From 2c0efab655ca01fdab2f5ee7398fee34e97b5ac2 Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Tue, 10 Jan 2023 11:23:15 +0100 Subject: [PATCH 19/24] Re-use attachment.getBytes() result --- sentry/src/main/java/io/sentry/SentryEnvelopeItem.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java b/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java index 7ee132f541..00b1fd5d27 100644 --- a/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java +++ b/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java @@ -171,9 +171,9 @@ public static SentryEnvelopeItem fromAttachment( new CachedItem( () -> { if (attachment.getBytes() != null) { - ensureAttachmentSizeLimit( - attachment.getBytes().length, maxAttachmentSize, attachment.getFilename()); - return attachment.getBytes(); + final byte[] data = attachment.getBytes(); + ensureAttachmentSizeLimit(data.length, maxAttachmentSize, attachment.getFilename()); + return data; } else if (attachment.getBytesFactory() != null) { final byte[] data = attachment.getBytesFactory().call(); ensureAttachmentSizeLimit(data.length, maxAttachmentSize, attachment.getFilename()); From 7ce3c982aca8d0b87fdfc3a3e931c4b4d5b1f017 Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Tue, 10 Jan 2023 16:31:51 +0100 Subject: [PATCH 20/24] Move View Hierarchy serialization from processor to SentryEnvelopeItem --- .../core/ViewHierarchyEventProcessor.java | 28 +------- sentry/api/sentry.api | 8 +-- .../src/main/java/io/sentry/Attachment.java | 28 ++++---- .../src/main/java/io/sentry/SentryClient.java | 6 +- .../java/io/sentry/SentryEnvelopeItem.java | 27 ++++++-- .../src/test/java/io/sentry/AttachmentTest.kt | 7 +- .../test/java/io/sentry/JsonSerializerTest.kt | 9 +-- .../test/java/io/sentry/SentryClientTest.kt | 5 +- .../java/io/sentry/SentryEnvelopeItemTest.kt | 65 +++++++++++++------ .../sentry/clientreport/ClientReportTest.kt | 3 +- .../io/sentry/transport/RateLimiterTest.kt | 4 +- 11 files changed, 108 insertions(+), 82 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java index 26c4405d64..39fa5de65d 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java @@ -4,7 +4,6 @@ import android.view.View; import android.view.ViewGroup; import android.view.Window; -import androidx.annotation.NonNull; import io.sentry.Attachment; import io.sentry.EventProcessor; import io.sentry.Hint; @@ -14,10 +13,6 @@ import io.sentry.protocol.ViewHierarchy; import io.sentry.protocol.ViewHierarchyNode; import io.sentry.util.Objects; -import java.io.BufferedWriter; -import java.io.ByteArrayOutputStream; -import java.io.OutputStreamWriter; -import java.io.Writer; import java.nio.charset.Charset; import java.util.ArrayList; import java.util.List; @@ -62,39 +57,20 @@ public ViewHierarchyEventProcessor(final @NotNull SentryAndroidOptions options) } final @Nullable View decorView = window.peekDecorView(); - options.getLogger().log(SentryLevel.INFO, "Missing decor view for view hierarchy snapshot."); if (decorView == null) { + options.getLogger().log(SentryLevel.INFO, "Missing decor view for view hierarchy snapshot."); return event; } try { final @NotNull ViewHierarchy viewHierarchy = snapshotViewHierarchy(decorView); - attachViewHierarchy(viewHierarchy, hint); + hint.setViewHierarchy(Attachment.fromViewHierarchy(viewHierarchy)); } catch (Throwable t) { options.getLogger().log(SentryLevel.ERROR, "Failed to process view hierarchy.", t); } return event; } - private void attachViewHierarchy(@NonNull ViewHierarchy viewHierarchy, @NonNull Hint hint) { - hint.setViewHierarchy( - Attachment.fromViewHierarchy( - () -> { - try { - try (final ByteArrayOutputStream stream = new ByteArrayOutputStream(); - final Writer writer = - new BufferedWriter(new OutputStreamWriter(stream, UTF_8))) { - - options.getSerializer().serialize(viewHierarchy, writer); - return stream.toByteArray(); - } - } catch (Throwable t) { - options.getLogger().log(SentryLevel.ERROR, "Could not serialize ViewHierarchy", t); - throw t; - } - })); - } - @NotNull public static ViewHierarchy snapshotViewHierarchy(@NotNull final View view) { final List windows = new ArrayList<>(1); diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 7a3000dbcc..11f25477bb 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -4,25 +4,25 @@ public final class io/sentry/AsyncHttpTransportFactory : io/sentry/ITransportFac } public final class io/sentry/Attachment { + public fun (Lio/sentry/JsonSerializable;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Z)V public fun (Ljava/lang/String;)V public fun (Ljava/lang/String;Ljava/lang/String;)V public fun (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)V public fun (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Z)V public fun (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Z)V public fun (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ZLjava/lang/String;)V - public fun (Ljava/util/concurrent/Callable;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Z)V public fun ([BLjava/lang/String;)V public fun ([BLjava/lang/String;Ljava/lang/String;)V public fun ([BLjava/lang/String;Ljava/lang/String;Ljava/lang/String;Z)V public fun ([BLjava/lang/String;Ljava/lang/String;Z)V public static fun fromScreenshot ([B)Lio/sentry/Attachment; - public static fun fromViewHierarchy (Ljava/util/concurrent/Callable;)Lio/sentry/Attachment; + public static fun fromViewHierarchy (Lio/sentry/protocol/ViewHierarchy;)Lio/sentry/Attachment; public fun getAttachmentType ()Ljava/lang/String; public fun getBytes ()[B - public fun getBytesFactory ()Ljava/util/concurrent/Callable; public fun getContentType ()Ljava/lang/String; public fun getFilename ()Ljava/lang/String; public fun getPathname ()Ljava/lang/String; + public fun getSerializable ()Lio/sentry/JsonSerializable; } public final class io/sentry/Baggage { @@ -1278,7 +1278,7 @@ public final class io/sentry/SentryEnvelopeHeader$JsonKeys { } public final class io/sentry/SentryEnvelopeItem { - public static fun fromAttachment (Lio/sentry/Attachment;J)Lio/sentry/SentryEnvelopeItem; + public static fun fromAttachment (Lio/sentry/ISerializer;Lio/sentry/ILogger;Lio/sentry/Attachment;J)Lio/sentry/SentryEnvelopeItem; public static fun fromClientReport (Lio/sentry/ISerializer;Lio/sentry/clientreport/ClientReport;)Lio/sentry/SentryEnvelopeItem; public static fun fromEvent (Lio/sentry/ISerializer;Lio/sentry/SentryBaseEvent;)Lio/sentry/SentryEnvelopeItem; public static fun fromProfilingTrace (Lio/sentry/ProfilingTraceData;JLio/sentry/ISerializer;)Lio/sentry/SentryEnvelopeItem; diff --git a/sentry/src/main/java/io/sentry/Attachment.java b/sentry/src/main/java/io/sentry/Attachment.java index 62331b06a5..4f2e12fe40 100644 --- a/sentry/src/main/java/io/sentry/Attachment.java +++ b/sentry/src/main/java/io/sentry/Attachment.java @@ -1,7 +1,7 @@ package io.sentry; +import io.sentry.protocol.ViewHierarchy; import java.io.File; -import java.util.concurrent.Callable; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -9,7 +9,7 @@ public final class Attachment { private @Nullable byte[] bytes; - private final @Nullable Callable bytesFactory; + private final @Nullable JsonSerializable serializable; private @Nullable String pathname; private final @NotNull String filename; private final @Nullable String contentType; @@ -83,7 +83,7 @@ public Attachment( final @Nullable String attachmentType, final boolean addToTransactions) { this.bytes = bytes; - this.bytesFactory = null; + this.serializable = null; this.filename = filename; this.contentType = contentType; this.attachmentType = attachmentType; @@ -94,7 +94,7 @@ public Attachment( * Initializes an Attachment with bytes factory, a filename, a content type, and * addToTransactions. * - * @param bytesFactory The bytes factory providing the data when being called + * @param serializable A json serializable holding the attachment payload * @param filename The name of the attachment to display in Sentry. * @param contentType The content type of the attachment. * @param attachmentType the attachment type. @@ -102,13 +102,13 @@ public Attachment( * {@link ITransaction} or set to false if it shouldn't. */ public Attachment( - final @NotNull Callable bytesFactory, + final @NotNull JsonSerializable serializable, final @NotNull String filename, final @Nullable String contentType, final @Nullable String attachmentType, final boolean addToTransactions) { this.bytes = null; - this.bytesFactory = bytesFactory; + this.serializable = serializable; this.filename = filename; this.contentType = contentType; this.attachmentType = attachmentType; @@ -185,7 +185,7 @@ public Attachment( final boolean addToTransactions) { this.pathname = pathname; this.filename = filename; - this.bytesFactory = null; + this.serializable = null; this.contentType = contentType; this.attachmentType = attachmentType; this.addToTransactions = addToTransactions; @@ -211,7 +211,7 @@ public Attachment( final boolean addToTransactions) { this.pathname = pathname; this.filename = filename; - this.bytesFactory = null; + this.serializable = null; this.contentType = contentType; this.addToTransactions = addToTransactions; } @@ -239,7 +239,7 @@ public Attachment( final @Nullable String attachmentType) { this.pathname = pathname; this.filename = filename; - this.bytesFactory = null; + this.serializable = null; this.contentType = contentType; this.addToTransactions = addToTransactions; this.attachmentType = attachmentType; @@ -259,8 +259,8 @@ public Attachment( * * @return the bytes factory responsible for providing the bytes. */ - public @Nullable Callable getBytesFactory() { - return bytesFactory; + public @Nullable JsonSerializable getSerializable() { + return serializable; } /** @@ -323,12 +323,12 @@ boolean isAddToTransactions() { /** * Creates a new View Hierarchy Attachment * - * @param bytesFactory the serialized View Hierarchy + * @param viewHierarchy the View Hierarchy * @return the Attachment */ - public static @NotNull Attachment fromViewHierarchy(final Callable bytesFactory) { + public static @NotNull Attachment fromViewHierarchy(final ViewHierarchy viewHierarchy) { return new Attachment( - bytesFactory, + viewHierarchy, "view-hierarchy.json", "application/json", VIEW_HIERARCHY_ATTACHMENT_TYPE, diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index 1827a57fdc..48567f186f 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -278,7 +278,11 @@ private boolean shouldSendSessionUpdateForDroppedEvent( if (attachments != null) { for (final Attachment attachment : attachments) { final SentryEnvelopeItem attachmentItem = - SentryEnvelopeItem.fromAttachment(attachment, options.getMaxAttachmentSize()); + SentryEnvelopeItem.fromAttachment( + options.getSerializer(), + options.getLogger(), + attachment, + options.getMaxAttachmentSize()); envelopeItems.add(attachmentItem); } } diff --git a/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java b/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java index 00b1fd5d27..431d27942c 100644 --- a/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java +++ b/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java @@ -165,7 +165,10 @@ public static SentryEnvelopeItem fromUserFeedback( } public static SentryEnvelopeItem fromAttachment( - final @NotNull Attachment attachment, final long maxAttachmentSize) { + final @NotNull ISerializer serializer, + final @NotNull ILogger logger, + final @NotNull Attachment attachment, + final long maxAttachmentSize) { final CachedItem cachedItem = new CachedItem( @@ -174,10 +177,24 @@ public static SentryEnvelopeItem fromAttachment( final byte[] data = attachment.getBytes(); ensureAttachmentSizeLimit(data.length, maxAttachmentSize, attachment.getFilename()); return data; - } else if (attachment.getBytesFactory() != null) { - final byte[] data = attachment.getBytesFactory().call(); - ensureAttachmentSizeLimit(data.length, maxAttachmentSize, attachment.getFilename()); - return data; + } else if (attachment.getSerializable() != null) { + final JsonSerializable serializable = attachment.getSerializable(); + try { + try (final ByteArrayOutputStream stream = new ByteArrayOutputStream(); + final Writer writer = + new BufferedWriter(new OutputStreamWriter(stream, UTF_8))) { + + serializer.serialize(serializable, writer); + + final byte[] data = stream.toByteArray(); + ensureAttachmentSizeLimit( + data.length, maxAttachmentSize, attachment.getFilename()); + return data; + } + } catch (Throwable t) { + logger.log(SentryLevel.ERROR, "Could not serialize attachment serializable", t); + throw t; + } } else if (attachment.getPathname() != null) { return readBytesFromFile(attachment.getPathname(), maxAttachmentSize); } diff --git a/sentry/src/test/java/io/sentry/AttachmentTest.kt b/sentry/src/test/java/io/sentry/AttachmentTest.kt index 65838016a9..7ce47b4e1c 100644 --- a/sentry/src/test/java/io/sentry/AttachmentTest.kt +++ b/sentry/src/test/java/io/sentry/AttachmentTest.kt @@ -1,5 +1,6 @@ package io.sentry +import io.sentry.protocol.ViewHierarchy import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse @@ -115,14 +116,14 @@ class AttachmentTest { @Test fun `creates attachment from view hierarchy`() { - val bytes = byteArrayOf(1, 2, 3) - val attachment = Attachment.fromViewHierarchy { bytes } + val hierarchy = ViewHierarchy("android", emptyList()) + val attachment = Attachment.fromViewHierarchy(hierarchy) assertEquals("view-hierarchy.json", attachment.filename) assertEquals("application/json", attachment.contentType) assertEquals(false, attachment.isAddToTransactions) assertEquals("event.view_hierarchy", attachment.attachmentType) assertNull(attachment.bytes) - assertEquals(bytes, attachment.bytesFactory!!.call()) + assertEquals(hierarchy, attachment.serializable) } } diff --git a/sentry/src/test/java/io/sentry/JsonSerializerTest.kt b/sentry/src/test/java/io/sentry/JsonSerializerTest.kt index 944fb24f73..af24bcf042 100644 --- a/sentry/src/test/java/io/sentry/JsonSerializerTest.kt +++ b/sentry/src/test/java/io/sentry/JsonSerializerTest.kt @@ -45,14 +45,15 @@ class JsonSerializerTest { val serializer: ISerializer val hub = mock() val traceFile = Files.createTempFile("test", "here").toFile() + val options = SentryOptions() init { - val options = SentryOptions() options.dsn = "https://key@sentry.io/proj" options.setLogger(logger) - options.setDebug(true) + options.isDebug = true whenever(hub.options).thenReturn(options) serializer = JsonSerializer(options) + options.setSerializer(serializer) options.setEnvelopeReader(EnvelopeReader(serializer)) } } @@ -946,9 +947,9 @@ class JsonSerializerTest { val message = "hello" val attachment = Attachment(message.toByteArray(), "bytes.txt") - val validAttachmentItem = SentryEnvelopeItem.fromAttachment(attachment, 5) + val validAttachmentItem = SentryEnvelopeItem.fromAttachment(fixture.serializer, fixture.options.logger, attachment, 5) - val invalidAttachmentItem = SentryEnvelopeItem.fromAttachment(Attachment("no"), 5) + val invalidAttachmentItem = SentryEnvelopeItem.fromAttachment(fixture.serializer, fixture.options.logger, Attachment("no"), 5) val envelope = SentryEnvelope(header, listOf(invalidAttachmentItem, validAttachmentItem)) val actualJson = serializeToString(envelope) diff --git a/sentry/src/test/java/io/sentry/SentryClientTest.kt b/sentry/src/test/java/io/sentry/SentryClientTest.kt index cf9eb52085..e9543f5493 100644 --- a/sentry/src/test/java/io/sentry/SentryClientTest.kt +++ b/sentry/src/test/java/io/sentry/SentryClientTest.kt @@ -15,6 +15,7 @@ import io.sentry.protocol.SentryException import io.sentry.protocol.SentryId import io.sentry.protocol.SentryTransaction import io.sentry.protocol.User +import io.sentry.protocol.ViewHierarchy import io.sentry.test.callMethod import io.sentry.transport.ITransport import io.sentry.transport.ITransportGate @@ -1423,7 +1424,7 @@ class SentryClientTest { @Test fun `view hierarchy is added to the envelope from the hint`() { val sut = fixture.getSut() - val attachment = Attachment.fromViewHierarchy { byteArrayOf() } + val attachment = Attachment.fromViewHierarchy(ViewHierarchy("android_view_system", emptyList())) val hint = Hint().also { it.viewHierarchy = attachment } sut.captureEvent(SentryEvent(), hint) @@ -1443,7 +1444,7 @@ class SentryClientTest { fun `view hierarchy is dropped from hint via before send`() { fixture.sentryOptions.beforeSend = CustomBeforeSendCallback() val sut = fixture.getSut() - val attachment = Attachment.fromViewHierarchy { byteArrayOf() } + val attachment = Attachment.fromViewHierarchy(ViewHierarchy("android_view_system", emptyList())) val hint = Hint().also { it.viewHierarchy = attachment } sut.captureEvent(SentryEvent(), hint) diff --git a/sentry/src/test/java/io/sentry/SentryEnvelopeItemTest.kt b/sentry/src/test/java/io/sentry/SentryEnvelopeItemTest.kt index 40b4e090c7..7b76678624 100644 --- a/sentry/src/test/java/io/sentry/SentryEnvelopeItemTest.kt +++ b/sentry/src/test/java/io/sentry/SentryEnvelopeItemTest.kt @@ -2,13 +2,18 @@ package io.sentry import io.sentry.exception.SentryEnvelopeException import io.sentry.protocol.User +import io.sentry.protocol.ViewHierarchy import io.sentry.test.injectForField import io.sentry.vendor.Base64 import org.junit.Assert.assertArrayEquals import org.mockito.kotlin.mock import org.mockito.kotlin.verify import org.mockito.kotlin.whenever +import java.io.BufferedWriter +import java.io.ByteArrayOutputStream import java.io.File +import java.io.OutputStreamWriter +import java.nio.charset.Charset import kotlin.test.AfterTest import kotlin.test.Test import kotlin.test.assertEquals @@ -20,7 +25,8 @@ import kotlin.test.assertNull class SentryEnvelopeItemTest { private class Fixture { - val serializer = JsonSerializer(SentryOptions()) + val options = SentryOptions() + val serializer = JsonSerializer(options) val pathname = "hello.txt" val filename = pathname val bytes = "hello".toByteArray() @@ -53,25 +59,28 @@ class SentryEnvelopeItemTest { fun `fromAttachment with bytes`() { val attachment = Attachment(fixture.bytesAllowed, fixture.filename) - val item = SentryEnvelopeItem.fromAttachment(attachment, fixture.maxAttachmentSize) + val item = SentryEnvelopeItem.fromAttachment(fixture.serializer, fixture.options.logger, attachment, fixture.maxAttachmentSize) assertAttachment(attachment, fixture.bytesAllowed, item) } @Test - fun `fromAttachment with bytes factory`() { - val attachment = Attachment({ fixture.bytesAllowed }, fixture.filename, "text/plain", null, false) + fun `fromAttachment with Serializable`() { + val viewHierarchy = ViewHierarchy("android", emptyList()) + val viewHierarchySerialized = serialize(viewHierarchy) - val item = SentryEnvelopeItem.fromAttachment(attachment, fixture.maxAttachmentSize) + val attachment = Attachment(viewHierarchy, fixture.filename, "text/plain", null, false) - assertAttachment(attachment, fixture.bytesAllowed, item) + val item = SentryEnvelopeItem.fromAttachment(fixture.serializer, fixture.options.logger, attachment, fixture.maxAttachmentSize) + + assertAttachment(attachment, viewHierarchySerialized, item) } @Test fun `fromAttachment with attachmentType`() { val attachment = Attachment(fixture.pathname, fixture.filename, "", true, "event.minidump") - val item = SentryEnvelopeItem.fromAttachment(attachment, fixture.maxAttachmentSize) + val item = SentryEnvelopeItem.fromAttachment(fixture.serializer, fixture.options.logger, attachment, fixture.maxAttachmentSize) assertEquals("event.minidump", item.header.attachmentType) } @@ -82,7 +91,7 @@ class SentryEnvelopeItemTest { file.writeBytes(fixture.bytesAllowed) val attachment = Attachment(file.path) - val item = SentryEnvelopeItem.fromAttachment(attachment, fixture.maxAttachmentSize) + val item = SentryEnvelopeItem.fromAttachment(fixture.serializer, fixture.options.logger, attachment, fixture.maxAttachmentSize) assertAttachment(attachment, fixture.bytesAllowed, item) } @@ -94,7 +103,7 @@ class SentryEnvelopeItemTest { file.writeBytes(twoMB) val attachment = Attachment(file.absolutePath) - val item = SentryEnvelopeItem.fromAttachment(attachment, fixture.maxAttachmentSize) + val item = SentryEnvelopeItem.fromAttachment(fixture.serializer, fixture.options.logger, attachment, fixture.maxAttachmentSize) assertAttachment(attachment, twoMB, item) } @@ -103,7 +112,7 @@ class SentryEnvelopeItemTest { fun `fromAttachment with non existent file`() { val attachment = Attachment("I don't exist", "file.txt") - val item = SentryEnvelopeItem.fromAttachment(attachment, fixture.maxAttachmentSize) + val item = SentryEnvelopeItem.fromAttachment(fixture.serializer, fixture.options.logger, attachment, fixture.maxAttachmentSize) assertFailsWith( "Reading the attachment ${attachment.pathname} failed, because the file located at " + @@ -123,7 +132,7 @@ class SentryEnvelopeItemTest { if (changedFileReadPermission) { val attachment = Attachment(file.path, "file.txt") - val item = SentryEnvelopeItem.fromAttachment(attachment, fixture.maxAttachmentSize) + val item = SentryEnvelopeItem.fromAttachment(fixture.serializer, fixture.options.logger, attachment, fixture.maxAttachmentSize) assertFailsWith( "Reading the attachment ${attachment.pathname} failed, " + @@ -146,7 +155,7 @@ class SentryEnvelopeItemTest { val securityManager = DenyReadFileSecurityManager(fixture.pathname) System.setSecurityManager(securityManager) - val item = SentryEnvelopeItem.fromAttachment(attachment, fixture.maxAttachmentSize) + val item = SentryEnvelopeItem.fromAttachment(fixture.serializer, fixture.options.logger, attachment, fixture.maxAttachmentSize) assertFailsWith("Reading the attachment ${attachment.pathname} failed.") { item.data @@ -165,7 +174,7 @@ class SentryEnvelopeItemTest { // reflection instead. attachment.injectForField("pathname", null) - val item = SentryEnvelopeItem.fromAttachment(attachment, fixture.maxAttachmentSize) + val item = SentryEnvelopeItem.fromAttachment(fixture.serializer, fixture.options.logger, attachment, fixture.maxAttachmentSize) assertFailsWith( "Couldn't attach the attachment ${attachment.filename}.\n" + @@ -180,7 +189,7 @@ class SentryEnvelopeItemTest { val image = this::class.java.classLoader.getResource("Tongariro.jpg")!! val attachment = Attachment(image.path) - val item = SentryEnvelopeItem.fromAttachment(attachment, fixture.maxAttachmentSize) + val item = SentryEnvelopeItem.fromAttachment(fixture.serializer, fixture.options.logger, attachment, fixture.maxAttachmentSize) assertAttachment(attachment, image.readBytes(), item) } @@ -188,7 +197,7 @@ class SentryEnvelopeItemTest { fun `fromAttachment with bytes too big`() { val attachment = Attachment(fixture.bytesTooBig, fixture.filename) val exception = assertFailsWith { - SentryEnvelopeItem.fromAttachment(attachment, fixture.maxAttachmentSize).data + SentryEnvelopeItem.fromAttachment(fixture.serializer, fixture.options.logger, attachment, fixture.maxAttachmentSize).data } assertEquals( @@ -201,15 +210,22 @@ class SentryEnvelopeItemTest { } @Test - fun `fromAttachment with bytes factory too big`() { - val attachment = Attachment({ fixture.bytesTooBig }, fixture.filename, "text/plain", null, false) + fun `fromAttachment with serializable too big`() { + val serializable = JsonSerializable { writer, _ -> + writer.beginObject() + writer.name("payload").value(String(fixture.bytesTooBig)) + writer.endObject() + } + val serializedBytes = serialize(serializable) + + val attachment = Attachment(serializable, fixture.filename, "text/plain", null, false) val exception = assertFailsWith { - SentryEnvelopeItem.fromAttachment(attachment, fixture.maxAttachmentSize).data + SentryEnvelopeItem.fromAttachment(fixture.serializer, fixture.options.logger, attachment, fixture.maxAttachmentSize).data } assertEquals( "Dropping attachment with filename '${fixture.filename}', because the " + - "size of the passed bytes with ${fixture.bytesTooBig.size} bytes is bigger " + + "size of the passed bytes with ${serializedBytes.size} bytes is bigger " + "than the maximum allowed attachment size of " + "${fixture.maxAttachmentSize} bytes.", exception.message @@ -223,7 +239,7 @@ class SentryEnvelopeItemTest { val attachment = Attachment(file.path) val exception = assertFailsWith { - SentryEnvelopeItem.fromAttachment(attachment, fixture.maxAttachmentSize).data + SentryEnvelopeItem.fromAttachment(fixture.serializer, fixture.options.logger, attachment, fixture.maxAttachmentSize).data } assertEquals( @@ -312,4 +328,13 @@ class SentryEnvelopeItemTest { assertEquals(attachment.filename, actualItem.header.fileName) assertArrayEquals(expectedBytes, actualItem.data) } + + private fun serialize(serializable: JsonSerializable): ByteArray { + ByteArrayOutputStream().use { stream -> + BufferedWriter(OutputStreamWriter(stream, Charset.forName("UTF-8"))).use { writer -> + fixture.serializer.serialize(serializable, writer) + return stream.toByteArray() + } + } + } } diff --git a/sentry/src/test/java/io/sentry/clientreport/ClientReportTest.kt b/sentry/src/test/java/io/sentry/clientreport/ClientReportTest.kt index 5593404fb8..4bca606ad6 100644 --- a/sentry/src/test/java/io/sentry/clientreport/ClientReportTest.kt +++ b/sentry/src/test/java/io/sentry/clientreport/ClientReportTest.kt @@ -5,6 +5,7 @@ import io.sentry.DataCategory import io.sentry.DateUtils import io.sentry.EventProcessor import io.sentry.Hint +import io.sentry.NoOpLogger import io.sentry.Sentry import io.sentry.SentryEnvelope import io.sentry.SentryEnvelopeHeader @@ -52,7 +53,7 @@ class ClientReportTest { SentryEnvelopeItem.fromEvent(opts.serializer, SentryEvent()), SentryEnvelopeItem.fromSession(opts.serializer, Session("dis", User(), "env", "0.0.1")), SentryEnvelopeItem.fromUserFeedback(opts.serializer, UserFeedback(SentryId(UUID.randomUUID()))), - SentryEnvelopeItem.fromAttachment(Attachment("{ \"number\": 10 }".toByteArray(), "log.json"), 1000) + SentryEnvelopeItem.fromAttachment(opts.serializer, NoOpLogger.getInstance(), Attachment("{ \"number\": 10 }".toByteArray(), "log.json"), 1000) ) clientReportRecorder.recordLostEnvelope(DiscardReason.NETWORK_ERROR, envelope) diff --git a/sentry/src/test/java/io/sentry/transport/RateLimiterTest.kt b/sentry/src/test/java/io/sentry/transport/RateLimiterTest.kt index e1cff28c96..fc1a133f6f 100644 --- a/sentry/src/test/java/io/sentry/transport/RateLimiterTest.kt +++ b/sentry/src/test/java/io/sentry/transport/RateLimiterTest.kt @@ -186,7 +186,7 @@ class RateLimiterTest { } ) val sessionItem = SentryEnvelopeItem.fromSession(fixture.serializer, Session("123", User(), "env", "release")) - val attachmentItem = SentryEnvelopeItem.fromAttachment(Attachment("{ \"number\": 10 }".toByteArray(), "log.json"), 1000) + val attachmentItem = SentryEnvelopeItem.fromAttachment(fixture.serializer, NoOpLogger.getInstance(), Attachment("{ \"number\": 10 }".toByteArray(), "log.json"), 1000) val envelope = SentryEnvelope(SentryEnvelopeHeader(), arrayListOf(eventItem, userFeedbackItem, sessionItem, attachmentItem)) @@ -218,7 +218,7 @@ class RateLimiterTest { } ) val sessionItem = SentryEnvelopeItem.fromSession(fixture.serializer, Session("123", User(), "env", "release")) - val attachmentItem = SentryEnvelopeItem.fromAttachment(Attachment("{ \"number\": 10 }".toByteArray(), "log.json"), 1000) + val attachmentItem = SentryEnvelopeItem.fromAttachment(fixture.serializer, NoOpLogger.getInstance(), Attachment("{ \"number\": 10 }".toByteArray(), "log.json"), 1000) val envelope = SentryEnvelope(SentryEnvelopeHeader(), arrayListOf(eventItem, userFeedbackItem, sessionItem, attachmentItem)) From bbe99b430481d7ead18325c98fb59a62a31d963e Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Wed, 11 Jan 2023 08:36:22 +0100 Subject: [PATCH 21/24] Remove unused field --- .../io/sentry/android/core/ViewHierarchyEventProcessor.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java index 39fa5de65d..95bc5fe6fe 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java @@ -13,7 +13,6 @@ import io.sentry.protocol.ViewHierarchy; import io.sentry.protocol.ViewHierarchyNode; import io.sentry.util.Objects; -import java.nio.charset.Charset; import java.util.ArrayList; import java.util.List; import org.jetbrains.annotations.ApiStatus; @@ -24,9 +23,6 @@ @ApiStatus.Internal public final class ViewHierarchyEventProcessor implements EventProcessor { - @SuppressWarnings("CharsetObjectCanBeUsed") - private static final @NotNull Charset UTF_8 = Charset.forName("UTF-8"); - private final @NotNull SentryAndroidOptions options; public ViewHierarchyEventProcessor(final @NotNull SentryAndroidOptions options) { From 3d22c9f78d1b000b52d623a69864cc75579a861d Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Wed, 11 Jan 2023 13:37:43 +0100 Subject: [PATCH 22/24] Fix don't lookup resource name when view id is not set Gets rid of "Invalid ID 0x" logcat output spam --- .../core/internal/gestures/ViewUtils.java | 8 +++++--- .../core/internal/gestures/ViewUtilsTest.kt | 17 +++++++++++++++++ 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/ViewUtils.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/ViewUtils.java index cb55d92214..269e72d03d 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/ViewUtils.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/ViewUtils.java @@ -89,11 +89,13 @@ static String getResourceIdWithFallback(final @NotNull View view) { */ public static String getResourceId(final @NotNull View view) throws Resources.NotFoundException { final int viewId = view.getId(); + if (viewId == View.NO_ID) { + throw new Resources.NotFoundException(); + } final Resources resources = view.getContext().getResources(); - String resourceId = ""; if (resources != null) { - resourceId = resources.getResourceEntryName(viewId); + return resources.getResourceEntryName(viewId); } - return resourceId; + return ""; } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/ViewUtilsTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/ViewUtilsTest.kt index 57ff124510..51bea191e0 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/ViewUtilsTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/ViewUtilsTest.kt @@ -7,6 +7,8 @@ import org.mockito.kotlin.any import org.mockito.kotlin.doReturn import org.mockito.kotlin.doThrow import org.mockito.kotlin.mock +import org.mockito.kotlin.never +import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import kotlin.test.Test import kotlin.test.assertEquals @@ -44,6 +46,21 @@ class ViewUtilsTest { assertFailsWith { ViewUtils.getResourceId(view) } } + @Test + fun `when view has no id set, resource name is not looked up `() { + val context = mock() + val resources = mock() + whenever(context.resources).thenReturn(resources) + + val view = mock { + whenever(it.id).doReturn(View.NO_ID) + whenever(it.context).thenReturn(context) + } + + assertFailsWith { ViewUtils.getResourceId(view) } + verify(context, never()).resources + } + @Test fun `getResourceIdWithFallback falls back to hexadecimal id when resource not found`() { val view = mock { From 331fb966acd501005d4adfb276778dc5a3bf7b07 Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Thu, 12 Jan 2023 09:48:53 +0100 Subject: [PATCH 23/24] Fix typo --- sentry/src/main/java/io/sentry/SentryEnvelopeItem.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java b/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java index 431d27942c..2d5767718f 100644 --- a/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java +++ b/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java @@ -201,7 +201,7 @@ public static SentryEnvelopeItem fromAttachment( throw new SentryEnvelopeException( String.format( "Couldn't attach the attachment %s.\n" - + "Please check that either bytes, bytesFactory or a path is set.", + + "Please check that either bytes, serializable or a path is set.", attachment.getFilename())); }); From c94745dc2719e115fa2bff8a8486e1e044f4107c Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Thu, 12 Jan 2023 10:03:13 +0100 Subject: [PATCH 24/24] Fix do not look up resource names for generated view ids Gets rid of "Invalid ID" logcat error output --- .../core/internal/gestures/ViewUtils.java | 6 +++++- .../core/internal/gestures/ViewUtilsTest.kt | 16 ++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/ViewUtils.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/ViewUtils.java index 269e72d03d..6e7dab2ef5 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/ViewUtils.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/ViewUtils.java @@ -89,7 +89,7 @@ static String getResourceIdWithFallback(final @NotNull View view) { */ public static String getResourceId(final @NotNull View view) throws Resources.NotFoundException { final int viewId = view.getId(); - if (viewId == View.NO_ID) { + if (viewId == View.NO_ID || isViewIdGenerated(viewId)) { throw new Resources.NotFoundException(); } final Resources resources = view.getContext().getResources(); @@ -98,4 +98,8 @@ public static String getResourceId(final @NotNull View view) throws Resources.No } return ""; } + + private static boolean isViewIdGenerated(int id) { + return (id & 0xFF000000) == 0 && (id & 0x00FFFFFF) != 0; + } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/ViewUtilsTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/ViewUtilsTest.kt index 51bea191e0..05f7abc478 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/ViewUtilsTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/ViewUtilsTest.kt @@ -61,6 +61,22 @@ class ViewUtilsTest { verify(context, never()).resources } + @Test + fun `when view id is generated, resource name is not looked up `() { + val context = mock() + val resources = mock() + whenever(context.resources).thenReturn(resources) + + val view = mock { + // View.generateViewId() starts with 1 + whenever(it.id).doReturn(1) + whenever(it.context).thenReturn(context) + } + + assertFailsWith { ViewUtils.getResourceId(view) } + verify(context, never()).resources + } + @Test fun `getResourceIdWithFallback falls back to hexadecimal id when resource not found`() { val view = mock {