Applying immutable map in SentryEvent#setExtras causes UnsupportedOperationException

[wzieba]

Platform:

• Android -> If yes, which Device API (and compileSdkVersion/targetSdkVersion/Build tools) version?
  Device API: 26
  compileSdkVersion: 30
  targetSdkVersion: 30
  buildTools: applied by Android Gradle Plugin

IDE:

• Android Studio -> If yes, which version?
  4.2 beta 3

Build system:

• Gradle -> If yes, which version?
  6.7.1

Android Gradle Plugin:

• Yes -> If yes, which version?
  4.2.0-beta03

Sentry Android Gradle Plugin:

• No

Proguard/R8:

• Disabled

Platform installed with:

• Maven Central

The version of the SDK:
4.3.0

---

I have the following issue:

When applying extra data by Sentry#setExtra and then applying extra data for a single event by SentryEvent#setExtras with immutable map (from Kotlin), I'm experiencing UnsupportedOperationException as SDK tries to edit the immutable map that is passed.

It crashes on the runtime as Java's method signatures will accept immutable Kotlin's map.

Steps to reproduce:
I've prepared a reproduction repository.

To check it, please go to SentryProxyTest. Both of the use cases are tested there.

You can also check the results of those tests on CI.
The test with mutable map passes while test with immutable map throws an exception

Actual result:
When applying extra with MutableMap everything works fine, the exception is not thrown. When applying extra with Map (which is immutable), UnsupportedOperationException is thrown.

Expected result:
Sentry SDK should accept both the mutable and immutable types. For immutable types, if editing is required, it should map a Map to mutable type.

---

[marandaneto]

@wzieba thanks for reporting and steps with repro, that's great.

I'm not entirely sure that we should project our setters with UnsupportedOperationException, the SDK itself does not use Collections.unmodifiableList nor Kotlin immutable types, let me gather some feedback internally before doing such changes.

We could check its type if UnmodifiableCollection and copy it to a nonimmutable type, I'll need to check whats the type for the Kotlin one though.

Obviously a workaround would be using only non-immutable types for now, or appending the values using SentryEvent#setExtra(key, value).

[marandaneto]

not sure if I should tag this as an enhancement or bug, I'll figure this out.

[mateuszkwiecinski]

I'll add my 2 cents: In general calling sentry-java classes from Kotlin seems to be a bit inconvenient and rather error prone. There are a lot of cases where it is not clear whether a field can be null, issues resulting in runtime crashes caused by immutability (as the one described in this ticket), or simply invonvenient constructor calls.

In my case I tried to write custom EventProcessor, and I had to struggle with multiple runtime crashes, before I reached a working solution.
I wish I could write

sentryOptions.addEventProcessor { event, _ ->
    event.debugMeta.images += DebugImage(type = "proguard", uuid = "xxx")
    event
}

instead of guessing what should I do if event is null, how to create valid DebugMeta and what's the difference between empty and null images list, always remembering I sometimes can't use immutable objects. I always have to look into source code and unterstand the data flow and infer nullability and mutability.

I don't know what's the proper way to address all concenrs, I just wanted to share you may want to treat this issue as making sentry-java more Kotlin friendly in general, not focusing on immutability in mentione setExtras case.

[marandaneto]

@mateuszkwiecinski I don't disagree, although some of the problems you've mentioned are already known and in the backlog.

eg for better null-safety using Kotlin, #1137 adding @nullable and @NotNullable annotations
eg for adding proguard, #1236

ideally, you'd not need to fiddle with these things, the SDK should do it automatically, in our protocol pretty much everything is nullable by default, we want to be as cheap as possible (memory footprint and JSON ser/deser, payload size over the wire etc).

thanks for the feedback.

[marandaneto]

after discussed internally, we'll work on that but along with other changes that improve the Kotlin experience overall, during Q2.

fwiw the App does not crash if applying an immutable list/map/etc to SentryEvent, it drops the event though and it fails silently (it logs out if debug is enabled).