fix(android): MM-68210 guard WebSocket methods against missing client NPE

[pavelzeman]

Summary

Fixes a NullPointerException crash in WebSocketClientModuleImpl affecting 1,074 users (1,568 events) tracked in Sentry #7153287556.

Root Cause

invalidateClientFor() used Kotlin's force-unwrap operator (!!) on a map lookup:

clients[wsUri]!!.webSocket?.close(1000, null)

When JS calls invalidateClientFor for a URL that was never created or was already invalidated, clients[wsUri] returns null and !! throws NullPointerException. This crashes the app because the call is not wrapped in try/catch.

The same !! pattern existed in connectFor, disconnectFor, sendDataFor, and ensureClientFor — those were wrapped in try/catch so they rejected the promise instead of crashing, but they still used unsafe patterns.

Fix

• invalidateClientFor: Safe access with ?.let. Resolves the promise even if the client doesn't exist (idempotent — the desired end state is "no client", which is already true).
• connectFor, disconnectFor, sendDataFor: Explicit null checks with descriptive error messages via promise.reject().
• ensureClientFor: Replaced containsKey + !! with ?.let pattern.

All 5 !! non-null assertions on clients[wsUri] have been removed.

Ticket Link

This was logged by Sentry on April 6, 2026:

https://mattermost.atlassian.net/browse/MM-68210

Release Note

Fixed a crash (NullPointerException) on Android when invalidating a WebSocket client that was already disconnected or never created.

[coderabbitai]

📝 Walkthrough

Walkthrough

WebSocketClientModuleImpl and ApiClientModuleImpl replace forced non-null lookups with null-safe access and explicit Promise rejection when clients are missing. A new test file covers force-unwrap versus safe-access behaviour.

Changes

Null-safe client handling

Layer / File(s)	Summary
WebSocket lifecycle null-safety android/src/main/java/com/mattermost/networkclient/WebSocketClientModuleImpl.kt	ensureClientFor and invalidateClientFor close and remove stored WebSocket clients only when an entry exists, using null-safe access instead of non-null assertions.
WebSocket operations and tests android/src/main/java/com/mattermost/networkclient/WebSocketClientModuleImpl.kt, android/src/test/java/com/mattermost/networkclient/WebSocketClientNullSafetyTest.kt	connectFor, disconnectFor, and sendDataFor reject when no WebSocket client exists, use null-safe webSocket calls when present, and the new test file checks force-unwrap versus safe-access behaviour.
API client null checks android/src/main/java/com/mattermost/networkclient/ApiClientModuleImpl.kt	getClientHeadersFor, addClientHeadersFor, importClientP12For, invalidateClientFor, download, and upload now reject when the NetworkClient for a baseUrl is missing instead of using forced non-null lookups.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly matches the main Android fix for missing WebSocket clients and NPE prevention.
Description check	✅ Passed	The description is directly related to the WebSocket and client null-safety changes in this pull request.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/websocket-invalidate-npe

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@android/src/main/java/com/mattermost/networkclient/WebSocketClientModuleImpl.kt`:
- Around line 149-151: The websocket send flow in WebSocketClientModuleImpl.send
is resolving the promise even when client.webSocket?.send(data) returns null or
false, so dropped frames are treated as successful sends. Update the send logic
to inspect the return value from client.webSocket?.send(data) and reject the
promise whenever the socket is missing or OkHttp declines the frame, keeping
promise.resolve(null) only for a confirmed send.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f9fe4df1-ad82-4557-8d6e-7839c8355646

📥 Commits

Reviewing files that changed from the base of the PR and between de45de9 and 73e4a1d.

📒 Files selected for processing (1)

• android/src/main/java/com/mattermost/networkclient/WebSocketClientModuleImpl.kt

[jgheithcock]

This was actually caught by Sentry and the ticket created back on April 6. Interestingly, the user count then fell off under the >100 users/week on May 17 (though it still reports).

While this file looks good, the https://github.com/mattermost/react-native-network-client also has two instances where we have the unsafe clients[url]!! pattern:
ApiClientModuleImpl.kt: download and upload. Just FYI, not for this PR.

[pavelzeman]

Expanded scope: Added null guards for 6 additional methods in ApiClientModuleImpl that had the same unsafe clients[url]!! pattern:

• getClientHeadersFor
• addClientHeadersFor
• importClientP12For
• invalidateClientFor
• download
• upload

All now follow the same safe pattern as the WebSocket methods: check for null client and reject promise with clear error message if missing.

This force-push invalidates the previous approval — please re-review.

[coderabbitai]

🧹 Nitpick comments (1)

android/src/main/java/com/mattermost/networkclient/ApiClientModuleImpl.kt (1)

165-165: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use a regular exception type for missing-client rejections.

These branches model an expected runtime miss, so Error(...) is the wrong throwable shape here. If this ever gets propagated or handled outside Promise.reject, it will not be caught by catch (Exception) blocks and reads like an unrecoverable JVM failure. Prefer IllegalStateException or a stable RN error code/message instead.

Proposed change

-            return promise.reject(Error("Client not found for baseUrl: $url"))
+            return promise.reject(IllegalStateException("Client not found for baseUrl: $url"))

Also applies to: 185-185, 206-206, 227-227, 279-279, 343-343

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@android/src/main/java/com/mattermost/networkclient/ApiClientModuleImpl.kt` at
line 165, The missing-client rejection branches in ApiClientModuleImpl should
not use Error(...) because these are expected runtime failures; update the
Promise.reject calls in the client lookup paths (for example the logic around
getClient and related baseUrl checks) to use a regular exception such as
IllegalStateException, or a stable React Native error code/message shape. Keep
the existing rejection behavior, but make sure the throwable type is catchable
by normal Exception handlers and clearly represents a recoverable missing-client
condition.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@android/src/main/java/com/mattermost/networkclient/ApiClientModuleImpl.kt`:
- Line 165: The missing-client rejection branches in ApiClientModuleImpl should
not use Error(...) because these are expected runtime failures; update the
Promise.reject calls in the client lookup paths (for example the logic around
getClient and related baseUrl checks) to use a regular exception such as
IllegalStateException, or a stable React Native error code/message shape. Keep
the existing rejection behavior, but make sure the throwable type is catchable
by normal Exception handlers and clearly represents a recoverable missing-client
condition.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d48fd336-88df-4c20-adc1-d6f800313a80

📥 Commits

Reviewing files that changed from the base of the PR and between 977ee5b and f86a8b4.

📒 Files selected for processing (2)

• android/src/main/java/com/mattermost/networkclient/ApiClientModuleImpl.kt
• android/src/test/java/com/mattermost/networkclient/WebSocketClientNullSafetyTest.kt

✅ Files skipped from review due to trivial changes (1)

• android/src/test/java/com/mattermost/networkclient/WebSocketClientNullSafetyTest.kt

[pavelzeman]

Sorry about the force-push, that was unintentional.
My AI loves to ignores the rules we agreed on sometimes. Repeatedly.

The 1 additional change was fixing all remaining outstanding similar code paths where a similar NPE could theoretically happen - JG, thanks for pointing that out.