newrelic / newrelic-android-agent

SDK to enable instrumentation of Android mobile apps in New Relic
Apache License 2.0
13 stars 12 forks source link

A `kotlin.collection.Map` passed as a parameter attributes into `NewRelic.recordBreadcrumb` will result in a runtime crash #275

Open jordanterry opened 3 weeks ago

jordanterry commented 3 weeks ago

Description

When a kotlin.collection.Map (derived from a function that returns an immutable map) is passed as a parameter to NewRelic.recordBreadcrumb a runtime exception is thrown.

Steps to Reproduce

Populated Immutable Map

Code:

NewRelic.recordBreadcrumb(
    "Breadcrumb",
     mapOf("key" to "value"),
)

Runtime Exception:

Exception in thread "main" java.lang.UnsupportedOperationException
    at java.base/java.util.AbstractMap.put(AbstractMap.java:209)
    at com.newrelic.agent.android.NewRelic.recordBreadcrumb(NewRelic.java:943)
        ...

Empty Immutable Map

Code:

NewRelic.recordBreadcrumb(
    "Breadcrumb",
    emptyMap()
)

Runtime Exception:

Exception in thread "main" java.lang.UnsupportedOperationException: Operation is not supported for read-only collection
    at kotlin.collections.EmptyMap.put(Maps.kt)
    at com.newrelic.agent.android.NewRelic.recordBreadcrumb(NewRelic.java:943)
        ...

Using mutableMapOf

If I try to use mutableMapOf I see an error in my IDE.

NewRelic.recordBreadcrumb(
    "Hi",
    mutableMapOf("key" to "value")
)

Screenshot 2024-09-04 at 11 39 22

Expected Behavior

I should be able to use any Map without IDE issues or runtime exceptions when recording a breadcrumb.

Relevant Logs / Console output

Logs provided above.

Your Environment

Kotlin + Android

ndesai-newrelic commented 3 weeks ago

@jordanterry we created internal bug ticket for this, we will fix it very soon.

ndesai-newrelic commented 3 weeks ago

@jordanterry Current Limitation Our SDK is implemented in Java, which does not natively support mutable maps as method parameters. Due to this language constraint, we are unable to add methods that accept mutable maps as parameters. Planned Fix We acknowledge that this limitation has led to crashes when users attempt to pass mapOf() or emptyMap() as arguments. We want to assure you that we are actively working on a solution to address this issue.

jordanterry commented 2 weeks ago

@ndesai-newrelic would it be possible to create a mutable map internally? The fix doesn't necessarily have to be on the public API.

Here a HashMap is created:

https://github.com/newrelic/newrelic-android-agent/blob/8cec5e81e22b6fd529033e4991d4204d6e7973af/agent/src/main/java/com/newrelic/agent/android/NewRelic.java#L970-L975

When I get a free moment I can open a PR if that helps?

ndesai-newrelic commented 2 weeks ago

@jordanterry that's already done, it will release soon.