Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve exception grouping on Android for RuntimeExceptions #3184

Closed
wants to merge 5 commits into from
Closed

Improve exception grouping on Android for RuntimeExceptions #3184

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5b57ad1
Improve exception grouping
markushi Feb 1, 2024
28cdf1b
Merge branch 'main' into feat/improve-exception-grouping
markushi Feb 19, 2024
e410cc4
Reverse order for Android RuntimeExceptions
markushi Feb 19, 2024
c3c6e8c
Update Changelog
markushi Feb 19, 2024
8c4a254
Add more tests
markushi Feb 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions sentry/api/sentry.api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3798,18 +3798,24 @@ public final class io/sentry/protocol/Mechanism : io/sentry/JsonSerializable, io
public fun <init> (Ljava/lang/Thread;)V
public fun getData ()Ljava/util/Map;
public fun getDescription ()Ljava/lang/String;
public fun getExceptionId ()Ljava/lang/Integer;
public fun getHelpLink ()Ljava/lang/String;
public fun getIsExceptionGroup ()Ljava/lang/Boolean;
public fun getMeta ()Ljava/util/Map;
public fun getParentId ()Ljava/lang/Integer;
public fun getSynthetic ()Ljava/lang/Boolean;
public fun getType ()Ljava/lang/String;
public fun getUnknown ()Ljava/util/Map;
public fun isHandled ()Ljava/lang/Boolean;
public fun serialize (Lio/sentry/ObjectWriter;Lio/sentry/ILogger;)V
public fun setData (Ljava/util/Map;)V
public fun setDescription (Ljava/lang/String;)V
public fun setExceptionId (Ljava/lang/Integer;)V
public fun setHandled (Ljava/lang/Boolean;)V
public fun setHelpLink (Ljava/lang/String;)V
public fun setIsExceptionGroup (Ljava/lang/Boolean;)V
public fun setMeta (Ljava/util/Map;)V
public fun setParentId (Ljava/lang/Integer;)V
public fun setSynthetic (Ljava/lang/Boolean;)V
public fun setType (Ljava/lang/String;)V
public fun setUnknown (Ljava/util/Map;)V
Expand All @@ -3824,9 +3830,12 @@ public final class io/sentry/protocol/Mechanism$Deserializer : io/sentry/JsonDes
public final class io/sentry/protocol/Mechanism$JsonKeys {
public static final field DATA Ljava/lang/String;
public static final field DESCRIPTION Ljava/lang/String;
public static final field EXCEPTION_ID Ljava/lang/String;
public static final field HANDLED Ljava/lang/String;
public static final field HELP_LINK Ljava/lang/String;
public static final field IS_EXCEPTION_GROUP Ljava/lang/String;
public static final field META Ljava/lang/String;
public static final field PARENT_ID Ljava/lang/String;
public static final field SYNTHETIC Ljava/lang/String;
public static final field TYPE Ljava/lang/String;
public fun <init> ()V
Expand Down
19 changes: 18 additions & 1 deletion sentry/src/main/java/io/sentry/SentryExceptionFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,11 @@ Deque<SentryException> extractExceptionQueue(final @NotNull Throwable throwable)

Throwable currentThrowable = throwable;

boolean handled = false;
String type = null;

int currentExceptionId = 0;

// Stack the exceptions to send them in the reverse order
while (currentThrowable != null && circularityDetector.add(currentThrowable)) {
boolean snapshot = false;
Expand All @@ -151,14 +156,26 @@ Deque<SentryException> extractExceptionQueue(final @NotNull Throwable throwable)
ExceptionMechanismException exceptionMechanismThrowable =
(ExceptionMechanismException) currentThrowable;
exceptionMechanism = exceptionMechanismThrowable.getExceptionMechanism();
handled = Boolean.TRUE.equals(exceptionMechanism.isHandled());
type = exceptionMechanism.getType();

currentThrowable = exceptionMechanismThrowable.getThrowable();
thread = exceptionMechanismThrowable.getThread();
snapshot = exceptionMechanismThrowable.isSnapshot();
} else {
exceptionMechanism = null;
exceptionMechanism = new Mechanism();
exceptionMechanism.setHandled(handled);
exceptionMechanism.setType(type);

thread = Thread.currentThread();
}

exceptionMechanism.setExceptionId(currentExceptionId);
if (currentExceptionId > 0) {
exceptionMechanism.setParentId(currentExceptionId - 1);
}
currentExceptionId++;

final boolean includeSentryFrames =
exceptionMechanism != null && Boolean.FALSE.equals(exceptionMechanism.isHandled());
final List<SentryStackFrame> frames =
Expand Down
5 changes: 5 additions & 0 deletions sentry/src/main/java/io/sentry/UncaughtExceptionHandlerIntegration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,11 @@ static Throwable getUnhandledThrowable(
final Mechanism mechanism = new Mechanism();
mechanism.setHandled(false);
mechanism.setType("UncaughtExceptionHandler");

// probably should be Android only?
if (thrown instanceof RuntimeException) {
Copy link
Member

Choose a reason for hiding this comment

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

A RuntimeException by itself isn't an exception group, there's suppressed exceptions which we have an issue for (#542) but I'm guessing that has little to do with the issue we're trying to fix here.

mechanism.setIsExceptionGroup(true);
}
return new ExceptionMechanismException(mechanism, thrown, thread);
}

Expand Down
54 changes: 54 additions & 0 deletions sentry/src/main/java/io/sentry/protocol/Mechanism.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,12 @@ public final class Mechanism implements JsonUnknown, JsonSerializable {
*/
private @Nullable Boolean synthetic;

private @Nullable Boolean isExceptionGroup;

private @Nullable Integer parentId;

private @Nullable Integer exceptionId;

@SuppressWarnings("unused")
private @Nullable Map<String, Object> unknown;

Expand Down Expand Up @@ -140,6 +146,33 @@ public void setSynthetic(final @Nullable Boolean synthetic) {
this.synthetic = synthetic;
}

@Nullable
public Boolean getIsExceptionGroup() {
return isExceptionGroup;
}

public void setIsExceptionGroup(@Nullable final Boolean exceptionGroup) {
isExceptionGroup = exceptionGroup;
}

@Nullable
public Integer getParentId() {
return parentId;
}

public void setParentId(@Nullable final Integer parentId) {
this.parentId = parentId;
}

@Nullable
public Integer getExceptionId() {
return exceptionId;
}

public void setExceptionId(@Nullable Integer exceptionId) {
this.exceptionId = exceptionId;
}

// JsonKeys

public static final class JsonKeys {
Expand All @@ -150,6 +183,9 @@ public static final class JsonKeys {
public static final String META = "meta";
public static final String DATA = "data";
public static final String SYNTHETIC = "synthetic";
public static final String IS_EXCEPTION_GROUP = "is_exception_group";
public static final String EXCEPTION_ID = "exception_id";
public static final String PARENT_ID = "parent_id";
}

// JsonUnknown
Expand Down Expand Up @@ -191,6 +227,15 @@ public void serialize(final @NotNull ObjectWriter writer, final @NotNull ILogger
if (synthetic != null) {
writer.name(JsonKeys.SYNTHETIC).value(synthetic);
}
if (isExceptionGroup != null) {
writer.name(JsonKeys.IS_EXCEPTION_GROUP).value(isExceptionGroup);
}
if (exceptionId != null) {
writer.name(JsonKeys.EXCEPTION_ID).value(exceptionId);
}
if (parentId != null) {
writer.name(JsonKeys.PARENT_ID).value(parentId);
}
if (unknown != null) {
for (String key : unknown.keySet()) {
Object value = unknown.get(key);
Expand Down Expand Up @@ -238,6 +283,15 @@ public static final class Deserializer implements JsonDeserializer<Mechanism> {
case JsonKeys.SYNTHETIC:
mechanism.synthetic = reader.nextBooleanOrNull();
break;
case JsonKeys.IS_EXCEPTION_GROUP:
mechanism.isExceptionGroup = reader.nextBooleanOrNull();
break;
case JsonKeys.EXCEPTION_ID:
mechanism.exceptionId = reader.nextIntegerOrNull();
break;
case JsonKeys.PARENT_ID:
mechanism.parentId = reader.nextIntegerOrNull();
break;
default:
if (unknown == null) {
unknown = new HashMap<>();
Expand Down
Loading