Skip to content

FIX [#642, RUM-3855]: Android Runtime Error in v2.3.1 for ReadableArray.asFirstPartyHosts() - #644

Merged
marco-saia-datadog merged 1 commit into
developfrom
marcosaia/fix/issue-642-hashmap-to-readablemap
Apr 4, 2024
Merged

FIX [#642, RUM-3855]: Android Runtime Error in v2.3.1 for ReadableArray.asFirstPartyHosts()#644
marco-saia-datadog merged 1 commit into
developfrom
marcosaia/fix/issue-642-hashmap-to-readablemap

Conversation

@marco-saia-datadog

@marco-saia-datadogmarco-saia-datadog commented Mar 28, 2024•
edited
Loading

Copy link
Copy Markdown
Member

What does this PR do?

ISSUE: #642

INTRODUCED WITH: v2.3.0 in PR #625

Fixes a wrong cast which leads to a Runtime Error.

Details

The tests rely on JavaOnlyMap, but its method toHashMap just wraps the map using a HashMap, without converting nested maps and arrays.

The ReadableNativeMap, which is the one used in a real use case, has a different implementation of toHashMap , which recursively converts the nested objects in either stdlib HashMap(s) or ArrayList(s).

The error shows up because .asFirstPartyHosts() performs an unsafe cast to ReadableMap for the nested objects inside of the array, but the actual type is HashMap from kotlin stdlib.

This PR introduces new conversion functions to ensure that the logic is consistent for different implementations of ReadableArray and ReadableMap.

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests
  • Make sure you discussed the feature or bugfix with the maintaining team in an Issue
  • Make sure each commit and the PR mention the Issue number (cf the CONTRIBUTING doc)

@marco-saia-datadog
marco-saia-datadogforce-pushed the marcosaia/fix/issue-642-hashmap-to-readablemap branch 2 times, most recently from 6a8e09c to 83d323cCompareMarch 28, 2024 13:40
@marco-saia-datadog
marco-saia-datadog marked this pull request as ready for review March 28, 2024 15:32
@marco-saia-datadog
marco-saia-datadog requested a review from a team as a code ownerMarch 28, 2024 15:32
0xnm
0xnm requested changes Mar 28, 2024
View reviewed changes
Comment threadpackages/core/android/src/main/kotlin/com/datadog/reactnative/DdSdkBridgeExt.kt Outdated
Comment threadpackages/core/android/src/main/kotlin/com/datadog/reactnative/DdSdkBridgeExt.kt Outdated
Comment threadpackages/core/android/src/main/kotlin/com/datadog/reactnative/DdSdkBridgeExt.kt Outdated
Comment threadpackages/core/android/src/main/kotlin/com/datadog/reactnative/DdSdkBridgeExt.kt Outdated
Comment threadpackages/core/android/src/main/kotlin/com/datadog/reactnative/DdSdkBridgeExt.kt
Comment threadpackages/core/android/src/main/kotlin/com/datadog/reactnative/DdSdkBridgeExt.kt Outdated
Comment threadpackages/core/android/src/main/kotlin/com/datadog/reactnative/DdSdkConfigurationExt.kt Outdated
Comment threadpackages/core/android/src/main/kotlin/com/datadog/reactnative/DdSdkConfigurationExt.kt Outdated
Comment threadpackages/core/android/src/test/kotlin/com/datadog/reactnative/DdSdkBridgeExtTest.kt Outdated
Comment threadpackages/core/android/src/test/kotlin/com/datadog/reactnative/DdSdkBridgeExtTest.kt Outdated
@marco-saia-datadog
marco-saia-datadogforce-pushed the marcosaia/fix/issue-642-hashmap-to-readablemap branch from f6ae714 to 800e4c7CompareApril 3, 2024 13:06
@marco-saia-datadogmarco-saia-datadog mentioned this pull request Apr 3, 2024
Closed
0xnm
0xnm reviewed Apr 3, 2024
View reviewed changes
Comment threadpackages/core/android/src/main/kotlin/com/datadog/reactnative/DdSdkBridgeExt.kt Outdated
Comment threadpackages/core/android/src/main/kotlin/com/datadog/reactnative/DdSdkBridgeExt.kt
Comment threadpackages/core/android/src/main/kotlin/com/datadog/reactnative/DdSdkBridgeExt.kt Outdated
Comment threadpackages/core/android/src/main/kotlin/com/datadog/reactnative/DdSdkBridgeExt.kt Outdated
Comment threadpackages/core/android/src/main/kotlin/com/datadog/reactnative/DdSdkBridgeExt.kt Outdated
Comment threadpackages/core/android/src/main/kotlin/com/datadog/reactnative/DdSdkConfigurationExt.kt
Comment threadpackages/core/android/src/test/kotlin/com/datadog/reactnative/DdSdkBridgeExtTest.kt
Comment threadpackages/core/android/src/test/kotlin/com/datadog/reactnative/DdSdkBridgeExtTest.kt Outdated
Comment threadpackages/core/android/src/test/kotlin/com/datadog/reactnative/DdSdkBridgeExtTest.kt Outdated
Comment threadpackages/core/android/src/test/kotlin/com/datadog/reactnative/DdSdkBridgeExtTest.kt Outdated
@marco-saia-datadog
marco-saia-datadogforce-pushed the marcosaia/fix/issue-642-hashmap-to-readablemap branch 2 times, most recently from 0db299c to c955537CompareApril 4, 2024 10:34
@marco-saia-datadog
marco-saia-datadogforce-pushed the marcosaia/fix/issue-642-hashmap-to-readablemap branch from c955537 to dc35cfaCompareApril 4, 2024 10:40
0xnm
0xnm previously approved these changes Apr 4, 2024
View reviewed changes

@0xnm0xnm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm! I hope that we add also integration test to check full configuration conversion through the layers.

Comment threadpackages/core/android/src/test/kotlin/com/datadog/reactnative/DdSdkBridgeExtTest.kt Outdated
Comment threadpackages/core/android/src/test/kotlin/com/datadog/reactnative/DdSdkBridgeExtTest.kt Outdated
@marco-saia-datadog
marco-saia-datadogforce-pushed the marcosaia/fix/issue-642-hashmap-to-readablemap branch from dc35cfa to 80bd61aCompareApril 4, 2024 11:43
0xnm
0xnm approved these changes Apr 4, 2024
View reviewed changes
@marco-saia-datadog
marco-saia-datadogforce-pushed the marcosaia/fix/issue-642-hashmap-to-readablemap branch from 80bd61a to ebd6552CompareApril 4, 2024 12:06
@marco-saia-datadog
marco-saia-datadog merged commit 52be8fb into developApr 4, 2024
@marco-saia-datadog
marco-saia-datadog deleted the marcosaia/fix/issue-642-hashmap-to-readablemap branch April 4, 2024 12:26
@marco-saia-datadogmarco-saia-datadog changed the title FIX #642: Android Runtime Error in v2.3.1 for ReadableArray.asFirstPartyHosts()FIX [#642, RUM-3857]: Android Runtime Error in v2.3.1 for ReadableArray.asFirstPartyHosts()Apr 4, 2024
@marco-saia-datadogmarco-saia-datadog changed the title FIX [#642, RUM-3857]: Android Runtime Error in v2.3.1 for ReadableArray.asFirstPartyHosts()FIX [#642, RUM-3855]: Android Runtime Error in v2.3.1 for ReadableArray.asFirstPartyHosts()Apr 4, 2024
This was referenced Apr 11, 2024
Closed
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@0xnm0xnm0xnm approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@marco-saia-datadog@0xnm