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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix ensure session is updated using configureScope #2846

Merged
merged 5 commits into from
Jul 18, 2023
Merged

Fix ensure session is updated using configureScope #2846

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
259c871
Fix ensure session is updated using configureScope
markushi Jul 18, 2023
6110b1e
Add improvements based on PR comments
markushi Jul 18, 2023
dd366fd
Add Changelog
markushi Jul 18, 2023
c74e828
Fix docstring
markushi Jul 18, 2023
a93f2b7
Fix changelog order
markushi Jul 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
- Fix ANRv2 thread dump parsing for native-only threads ([#2839](https://github.com/getsentry/sentry-java/pull/2839))
- Derive `TracingContext` values from event for ANRv2 events ([#2839](https://github.com/getsentry/sentry-java/pull/2839))

### Features
markushi marked this conversation as resolved.
Show resolved Hide resolved
- (Internal) Extend APIs for hybrid SDKs ([#2814](https://github.com/getsentry/sentry-java/pull/2814), [#2846](https://github.com/getsentry/sentry-java/pull/2846))

## 6.25.2

### Fixes
Expand Down
92 changes: 50 additions & 42 deletions sentry-android-core/src/main/java/io/sentry/android/core/InternalSentrySdk.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ public final class InternalSentrySdk {
@Nullable
public static Scope getCurrentScope() {
final @NotNull AtomicReference<Scope> scopeRef = new AtomicReference<>();
HubAdapter.getInstance().withScope(scopeRef::set);
//noinspection Convert2MethodRef
HubAdapter.getInstance().withScope(scope -> scopeRef.set(scope));
return scopeRef.get();
}

Expand All @@ -60,15 +61,14 @@ public static Map<String, Object> serializeScope(
final @NotNull Context context,
final @NotNull SentryAndroidOptions options,
final @Nullable Scope scope) {

final @NotNull Map<String, Object> data = new HashMap<>();
if (scope == null) {
return data;
}

final @NotNull ILogger logger = options.getLogger();
final @NotNull ObjectWriter writer = new MapObjectWriter(data);

try {
final @NotNull ILogger logger = options.getLogger();
final @NotNull ObjectWriter writer = new MapObjectWriter(data);

final @NotNull DeviceInfoUtil deviceInfoUtil = DeviceInfoUtil.getInstance(context, options);
final @NotNull Device deviceInfo = deviceInfoUtil.collectDeviceInformation(true, true);
Expand Down Expand Up @@ -114,7 +114,7 @@ public static Map<String, Object> serializeScope(
writer.name("fingerprint").value(logger, scope.getFingerprint());
writer.name("level").value(logger, scope.getLevel());
writer.name("breadcrumbs").value(logger, scope.getBreadcrumbs());
} catch (Exception e) {
} catch (Throwable e) {
options.getLogger().log(SentryLevel.ERROR, "Could not serialize scope.", e);
return new HashMap<>();
}
Expand All @@ -124,54 +124,62 @@ public static Map<String, Object> serializeScope(

/**
* Captures the provided envelope. Compared to {@link IHub#captureEvent(SentryEvent)} this method
* - will not enrich events with additional data (e.g. scope) - will not execute beforeSend: it's
* up to the caller to take care of this - will not perform any sampling: it's up to the caller to
* take care of this - will enrich the envelope with a Session updates is applicable
* <br>
* - will not enrich events with additional data (e.g. scope)<br>
* - will not execute beforeSend: it's up to the caller to take care of this<br>
* - will not perform any sampling: it's up to the caller to take care of this<br>
* - will enrich the envelope with a Session update if applicable<br>
*
* @param envelopeData the serialized envelope data
* @return The Id (SentryId object) of the event
* @throws Exception In case the provided envelope could not be parsed / is invalid
* @return The Id (SentryId object) of the event, or null in case the envelope could not be
* captured
*/
public static SentryId captureEnvelope(final @NotNull byte[] envelopeData) throws Exception {
@Nullable
public static SentryId captureEnvelope(final @NotNull byte[] envelopeData) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The return should not be nullable, but rather SentryId.EMPTY_ID as every other capture method, so the caller does not have to handle NPEs.

final @NotNull IHub hub = HubAdapter.getInstance();
final @NotNull SentryOptions options = hub.getOptions();

final @NotNull ISerializer serializer = options.getSerializer();
final @Nullable SentryEnvelope envelope =
options.getEnvelopeReader().read(new ByteArrayInputStream(envelopeData));
if (envelope == null) {
throw new IllegalArgumentException("Envelope could not be read");
}
try {
final @NotNull ISerializer serializer = options.getSerializer();
final @Nullable SentryEnvelope envelope =
options.getEnvelopeReader().read(new ByteArrayInputStream(envelopeData));
if (envelope == null) {
return null;
}

final @NotNull List<SentryEnvelopeItem> envelopeItems = new ArrayList<>();
final @NotNull List<SentryEnvelopeItem> envelopeItems = new ArrayList<>();

// determine session state based on events inside envelope
@Nullable Session.State status = null;
boolean crashedOrErrored = false;
for (SentryEnvelopeItem item : envelope.getItems()) {
envelopeItems.add(item);
// determine session state based on events inside envelope
@Nullable Session.State status = null;
boolean crashedOrErrored = false;
for (SentryEnvelopeItem item : envelope.getItems()) {
envelopeItems.add(item);

final SentryEvent event = item.getEvent(serializer);
if (event != null) {
if (event.isCrashed()) {
status = Session.State.Crashed;
}
if (event.isCrashed() || event.isErrored()) {
crashedOrErrored = true;
final SentryEvent event = item.getEvent(serializer);
if (event != null) {
if (event.isCrashed()) {
status = Session.State.Crashed;
}
if (event.isCrashed() || event.isErrored()) {
crashedOrErrored = true;
}
}
}
}

// update session and add it to envelope if necessary
final @Nullable Session session = updateSession(hub, options, status, crashedOrErrored);
if (session != null) {
final SentryEnvelopeItem sessionItem = SentryEnvelopeItem.fromSession(serializer, session);
envelopeItems.add(sessionItem);
}
// update session and add it to envelope if necessary
final @Nullable Session session = updateSession(hub, options, status, crashedOrErrored);
if (session != null) {
final SentryEnvelopeItem sessionItem = SentryEnvelopeItem.fromSession(serializer, session);
envelopeItems.add(sessionItem);
}

final SentryEnvelope repackagedEnvelope =
new SentryEnvelope(envelope.getHeader(), envelopeItems);
return hub.captureEnvelope(repackagedEnvelope);
final SentryEnvelope repackagedEnvelope =
new SentryEnvelope(envelope.getHeader(), envelopeItems);
return hub.captureEnvelope(repackagedEnvelope);
} catch (Throwable t) {
options.getLogger().log(SentryLevel.ERROR, "Failed to capture envelope", t);
}
return null;
}

@Nullable
Expand All @@ -181,7 +189,7 @@ private static Session updateSession(
final @Nullable Session.State status,
final boolean crashedOrErrored) {
final @NotNull AtomicReference<Session> sessionRef = new AtomicReference<>();
hub.withScope(
hub.configureScope(
scope -> {
final @Nullable Session session = scope.getSession();
if (session != null) {
Expand Down
18 changes: 11 additions & 7 deletions sentry-android-core/src/test/java/io/sentry/android/core/InternalSentrySdkTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@ import org.mockito.kotlin.whenever
import java.io.ByteArrayInputStream
import java.io.ByteArrayOutputStream
import java.io.InputStreamReader
import java.util.concurrent.atomic.AtomicReference
import kotlin.test.BeforeTest
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertFails
import kotlin.test.assertNotNull
import kotlin.test.assertNull
import kotlin.test.assertTrue
Expand Down Expand Up @@ -203,9 +203,7 @@ class InternalSentrySdkTest {

@Test
fun `captureEnvelope fails if payload is invalid`() {
assertFails {
InternalSentrySdk.captureEnvelope(ByteArray(8))
}
assertNull(InternalSentrySdk.captureEnvelope(ByteArray(8)))
}

@Test
Expand Down Expand Up @@ -251,20 +249,26 @@ class InternalSentrySdkTest {
}
)

// then the session should be included
val capturedEnvelope = fixture.capturedEnvelopes.first()
val capturedEnvelopeItems = capturedEnvelope.items.toList()

// and it should contain the original event / attachment
// then it should contain the original event + session
assertEquals(2, capturedEnvelopeItems.size)
assertEquals(SentryItemType.Event, capturedEnvelopeItems[0].header.type)
assertEquals(SentryItemType.Session, capturedEnvelopeItems[1].header.type)

// and then the sent session should be marked as crashed
val capturedSession = fixture.options.serializer.deserialize(
InputStreamReader(ByteArrayInputStream(capturedEnvelopeItems[1].data)),
Session::class.java
)!!

assertEquals(Session.State.Crashed, capturedSession.status)

// and the local session should be marked as crashed too
val scopeRef = AtomicReference<Scope>()
Sentry.configureScope { scope ->
scopeRef.set(scope)
}
assertEquals(Session.State.Crashed, scopeRef.get().session!!.status)
}
}