Skip to content

RUM-700 prevent NPE in GestureListener#1634

Merged
xgouchet merged 1 commit into
developfrom
xgouchet/RUM-700/null_motion_events
Sep 22, 2023
Merged

RUM-700 prevent NPE in GestureListener#1634
xgouchet merged 1 commit into
developfrom
xgouchet/RUM-700/null_motion_events

Conversation

@xgouchet

Copy link
Copy Markdown
Contributor

What does this PR do?

Create a custom Java version of GestureDetector.OnGestureListener class to keep compatibility with older versions of Android

Motivation

Our telemetry is reporting a lot of exception similar to:

java.lang.NullPointerException: Parameter specified as non-null is null: method kotlin.jvm.internal.Intrinsics.checkNotNullParameter, parameter startDownEvent
	at com.datadog.android.rum.internal.instrumentation.gestures.GesturesListener.onScroll(Unknown Source:2)
	at android.view.GestureDetector.onTouchEvent(GestureDetector.java:791)

On Android pre API 32, the onScroll method could be called with a null startDownEvent value. Starting in API 33, this parameter is marked as @NonNull, meaning that our Kotlin implementation must accept non-null parameters (trying to force the type as nullable MotionEvent? cause a compilation error.

This PR uses a custom Java override where the annotation is replaced with @Nullable to make Kotlin happy.

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • 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)

@xgouchet
xgouchet requested a review from a team as a code owner September 21, 2023 16:55

@0xnm 0xnm 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.

nice trick!

@codecov-commenter

codecov-commenter commented Sep 22, 2023

Copy link
Copy Markdown

Codecov Report

Merging #1634 (68a49de) into develop (8b37d30) will decrease coverage by 0.03%.
The diff coverage is 40.00%.

@@             Coverage Diff             @@
##           develop    #1634      +/-   ##
===========================================
- Coverage    83.77%   83.74%   -0.03%     
===========================================
  Files          455      456       +1     
  Lines        15691    15691              
  Branches      2334     2334              
===========================================
- Hits         13144    13140       -4     
- Misses        1922     1933      +11     
+ Partials       625      618       -7     
Files Changed Coverage Δ
...nstrumentation/gestures/GestureListenerCompat.java 33.33% <33.33%> (ø)
...ernal/instrumentation/gestures/GesturesListener.kt 92.36% <50.00%> (-0.69%) ⬇️

... and 20 files with indirect coverage changes

import androidx.annotation.NonNull;
import androidx.annotation.Nullable;

public abstract class GestureListenerCompat implements GestureDetector.OnGestureListener {

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.

I know you opened a ticket at Google about this and I also updated that one, did they ever reply to you ? Nice trick anyway.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No never got an answer ;D

@xgouchet
xgouchet merged commit c23314d into develop Sep 22, 2023
@xgouchet
xgouchet deleted the xgouchet/RUM-700/null_motion_events branch September 22, 2023 07:34
@xgouchet xgouchet added this to the 2.2.0 milestone Dec 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants