Fix RecorderWindowCallback crash on null menu parameter by nguyexua · Pull Request #3239 · DataDog/dd-sdk-android

nguyexua · 2026-03-06T12:46:22Z

What does this PR do?

Overrides menu-related Window.Callback methods in RecorderWindowCallback with nullable parameters, preventing Kotlin's non-null assertions from crashing when Android passes null values.

Motivation

A customer reported a crash (NullPointerException: Parameter specified as non-null is null: method RecorderWindowCallback.onMenuOpened, parameter p1) when opening a custom popup menu.

RecorderWindowCallback uses Kotlin's by wrappedCallback delegation for Window.Callback. Kotlin generates non-null assertions for all delegated parameters, but Android can call these methods with null values (known Android bug: https://issuetracker.google.com/issues/188568911). The previous fix only handled dispatchTouchEvent with a try-catch, but the same issue affected all menu-related callbacks.

Changes

Override onMenuOpened, onMenuItemSelected, onPanelClosed, onPreparePanel, and onCreatePanelMenu in RecorderWindowCallback with nullable parameters. When null is received, methods return safe defaults (false or no-op) instead of crashing.
Remove the dispatchTouchEvent try-catch workaround and logOrRethrowWrappedCallbackException helper, since dispatchTouchEvent already declares its parameter as nullable.
Add tests for menu callback delegation and null-parameter handling.
Remove obsolete try-catch tests and unused imports.

Note

This is an alternative to #3221 with a simpler approach: nullable Kotlin overrides instead of a FixedWindowCallback Java wrapper class. No new files, no API surface change, no inheritance change.

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)

codecov-commenter · 2026-03-06T15:20:30Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.38%. Comparing base (ba162fa) to head (442230c).
⚠️ Report is 18 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #3239      +/-   ##
===========================================
- Coverage    71.49%   71.38%   -0.11%     
===========================================
  Files          934      934              
  Lines        34628    34628              
  Branches      5862     5862              
===========================================
- Hits         24755    24718      -37     
- Misses        8231     8264      +33     
- Partials      1642     1646       +4

Files with missing lines	Coverage Δ
...og/android/internal/utils/FixedWindowCallback.java	`0.00% <ø> (ø)`
.../instrumentation/gestures/WindowCallbackWrapper.kt	`84.34% <ø> (ø)`
...nternal/utils/window/RumWindowCallbacksRegistry.kt	`92.31% <ø> (ø)`
...ternal/recorder/callback/RecorderWindowCallback.kt	`92.22% <100.00%> (ø)`

... and 34 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 493ec41725

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-03-09T09:00:45Z

+    }
+
+    @Suppress("SwallowedException", "UnsafeThirdPartyFunctionCall") // NPE caught below
+    override fun onMenuOpened(featureId: Int, menu: Menu): Boolean {


Declare menu callback args nullable

These overrides still use non-null parameter types (Menu/MenuItem), so Kotlin emits checkNotNullParameter at method entry and throws before the try/catch can run when the framework passes null. In the null-menu scenario this patch is targeting, onMenuOpened (and the similarly typed onMenuItemSelected, onPanelClosed, onPreparePanel, onCreatePanelMenu) can still crash before delegating.

Useful? React with 👍 / 👎.

ambushwork · 2026-03-09T15:25:10Z

 */
 @RunWith(AndroidJUnit4::class)
 @LargeTest
 internal class AppStartupActivityPredicateTest :


I am not sure the change in this file is related to this PR, if not, we need to revert it.

ambushwork · 2026-03-09T15:25:45Z

 dependencies {
    api(project(":dd-sdk-android-core"))
    implementation(project(":dd-sdk-android-internal"))
+    compileOnly(project(":features:dd-sdk-android-rum"))


Note
It's better just to move FixedWindowCallback into dd-sdk-android-internal module (suggested package: "com.datadog.android.internal.utils") instead of letting session replay depend on the whole rum module.

Extend FixedWindowCallback instead of using try-catch overrides, matching the pattern established in PR #2800. Add compileOnly dependency on dd-sdk-android-rum to access FixedWindowCallback.

Move FixedWindowCallback from dd-sdk-android-rum to dd-sdk-android-internal (package com.datadog.android.internal.utils) so it can be reused by session-replay without depending on the whole RUM module.

ambushwork

LGTM, Well done!

nguyexua force-pushed the lan.nguyen/fix-recorder-window-callback-null-menu-crash-v2 branch 2 times, most recently from a3dc505 to f460c15 Compare March 6, 2026 14:31

This comment has been minimized.

Sign in to view

nguyexua marked this pull request as ready for review March 9, 2026 08:57

nguyexua requested review from a team as code owners March 9, 2026 08:57

chatgpt-codex-connector Bot reviewed Mar 9, 2026

View reviewed changes

nguyexua force-pushed the lan.nguyen/fix-recorder-window-callback-null-menu-crash-v2 branch 2 times, most recently from bda200b to c8dd0cb Compare March 9, 2026 09:18

ambushwork reviewed Mar 9, 2026

View reviewed changes

Comment thread ...otlin/com/datadog/android/sessionreplay/internal/recorder/callback/RecorderWindowCallback.kt Outdated

ambushwork reviewed Mar 9, 2026

View reviewed changes

Reuse FixedWindowCallback from RUM module for null menu crash fix

4f1d754

Extend FixedWindowCallback instead of using try-catch overrides, matching the pattern established in PR #2800. Add compileOnly dependency on dd-sdk-android-rum to access FixedWindowCallback.

nguyexua force-pushed the lan.nguyen/fix-recorder-window-callback-null-menu-crash-v2 branch 2 times, most recently from 4e5751e to 3a633c5 Compare March 9, 2026 16:03

Move FixedWindowCallback to dd-sdk-android-internal module

442230c

Move FixedWindowCallback from dd-sdk-android-rum to dd-sdk-android-internal (package com.datadog.android.internal.utils) so it can be reused by session-replay without depending on the whole RUM module.

nguyexua force-pushed the lan.nguyen/fix-recorder-window-callback-null-menu-crash-v2 branch from 3a633c5 to 442230c Compare March 9, 2026 16:10

ambushwork approved these changes Mar 9, 2026

View reviewed changes

0xnm approved these changes Mar 10, 2026

View reviewed changes

gh-worker-dd-devflow-36fce6 Bot added mergequeue-status: queued mergequeue-status: in_progress mergequeue-status: rejected and removed mergequeue-status: queued mergequeue-status: in_progress mergequeue-status: rejected labels Mar 10, 2026

gh-worker-dd-mergequeue-cf854d Bot merged commit d16f4b0 into develop Mar 10, 2026
27 checks passed

gh-worker-dd-devflow-36fce6 Bot removed the mergequeue-status: in_progress label Mar 10, 2026

gh-worker-dd-mergequeue-cf854d Bot deleted the lan.nguyen/fix-recorder-window-callback-null-menu-crash-v2 branch March 10, 2026 11:09

gh-worker-dd-devflow-36fce6 Bot added the mergequeue-status: done label Mar 10, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix RecorderWindowCallback crash on null menu parameter#3239

Fix RecorderWindowCallback crash on null menu parameter#3239
gh-worker-dd-mergequeue-cf854d[bot] merged 2 commits into
developfrom
lan.nguyen/fix-recorder-window-callback-null-menu-crash-v2

nguyexua commented Mar 6, 2026 •

edited

Loading

Uh oh!

This comment has been minimized.

codecov-commenter commented Mar 6, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Mar 9, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ambushwork Mar 9, 2026

Uh oh!

ambushwork Mar 9, 2026

Uh oh!

ambushwork left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

nguyexua commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Motivation

Changes

Note

Review checklist (to be filled by reviewers)

Uh oh!

This comment has been minimized.

codecov-commenter commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ambushwork Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

ambushwork Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

ambushwork left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

nguyexua commented Mar 6, 2026 •

edited

Loading

codecov-commenter commented Mar 6, 2026 •

edited

Loading