fix: Prevent NullPointerException in BleGattServer.sendResponse

[sentry]

Fixes COLUMBA-6Q. The issue was that: Android BLE stack passes null offset to GATT callback, causing NullPointerException during sendResponse unboxing.

• Hardcode offset to 0 in BluetoothGattServer.sendResponse to prevent NullPointerException on older Android devices where the BLE stack might provide a null-boxed Integer for offset.
• Add a generic catch (Exception) block to handleCharacteristicWriteRequest for improved error logging of unexpected issues.

This fix was generated by Seer in Sentry, triggered automatically. 👁️ Run ID: 12135876

Not quite right? Click here to continue debugging with Seer.

[greptile-apps]

Greptile Summary

This PR fixes a NullPointerException crash (COLUMBA-6Q) where certain older Android devices (e.g. Redmi Note 5A, Android 7.1.2) deliver a null-boxed Integer for the offset parameter through Parcel IPC, causing a crash inside BluetoothGattServer.sendResponse. The fix is correct and well-documented: since offset is always 0 for non-prepared characteristic writes, hardcoding 0 is safe. A generic catch (e: Exception) block is also added for broader error logging.

Key changes:

• offset replaced with 0 in both the success and failure sendResponse calls inside handleCharacteristicWriteRequest — correct and safe for standard (non-prepared) writes.
• A well-commented explanation of the device-specific BLE stack bug is included inline.
• A new catch (e: Exception) block is added for logging unexpected errors — however, this catch swallows CancellationException, which breaks Kotlin structured concurrency (see inline comment). This needs to be fixed before merging.
• Neither catch block sends a GATT_FAILURE response when responseNeeded is true, which may leave the central device hanging if an exception is raised before the response is sent.

Confidence Score: 3/5

• The core NPE fix is correct, but the new generic catch block introduces a structured concurrency bug that should be addressed before merging.
• The offset=0 hardcoding is a sound fix for the reported crash. However, the new catch (e: Exception) block silently swallows CancellationException, which can prevent coroutines from cancelling properly during BLE server teardown. This is a real regression introduced by the PR that warrants a fix.
• Pay close attention to the new catch (e: Exception) block in BleGattServer.kt (lines 839–841).

Important Files Changed

Filename	Overview
reticulum/src/main/java/com/lxmf/messenger/reticulum/ble/server/BleGattServer.kt	Hardcodes offset=0 in sendResponse (correct for non-prepared writes) and adds a generic Exception catch, but the generic catch swallows CancellationException, breaking coroutine structured concurrency.

Sequence Diagram

sequenceDiagram
    participant Central as BLE Central (Remote Device)
    participant OS as Android BLE Stack
    participant Server as BleGattServer
    participant GATT as BluetoothGattServer

    Central->>OS: ATT Write Request (offset may be null on buggy firmware)
    OS->>Server: onCharacteristicWriteRequest(device, requestId, ..., offset, value)
    Note over Server: offset may be null-boxed Integer on older Android
    Server->>Server: handleCharacteristicWriteRequest(...)
    alt responseNeeded == true
        Note over Server,GATT: Before fix: passes offset (NPE if null)<br/>After fix: hardcodes 0
        Server->>GATT: sendResponse(device, requestId, GATT_SUCCESS, 0, value)
        GATT->>Central: ATT Write Response
    end
    alt Exception thrown
        Note over Server: catch(SecurityException) → log only<br/>catch(Exception) → log only<br/>⚠️ CancellationException also caught here
        Note over Server: No GATT_FAILURE sent → Central may time out
    end

Loading

Prompt To Fix All With AI

This is a comment left during a code review.
Path: reticulum/src/main/java/com/lxmf/messenger/reticulum/ble/server/BleGattServer.kt
Line: 839-841

Comment:
**`CancellationException` is swallowed by the generic `catch (e: Exception)` block**

`CancellationException` extends `IllegalStateException → RuntimeException → Exception`, so this new catch will intercept coroutine cancellation signals. Because `handleCharacteristicWriteRequest` runs inside `withContext(Dispatchers.Main)`, cancelling the parent coroutine (e.g. during lifecycle teardown) will throw a `CancellationException` that gets silently swallowed here, logged as "Unexpected error", and the coroutine will appear to have completed normally. This breaks Kotlin structured concurrency and can prevent the BLE server from shutting down cleanly.

The fix is to re-throw `CancellationException` before logging:

```suggestion
        } catch (e: Exception) {
            if (e is kotlinx.coroutines.CancellationException) throw e
            Log.e(TAG, "Unexpected error in handleCharacteristicWriteRequest for ${device.address}", e)
        }
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: reticulum/src/main/java/com/lxmf/messenger/reticulum/ble/server/BleGattServer.kt
Line: 837-841

Comment:
**No `sendResponse` in error paths may leave the central hanging**

Both catch blocks (the pre-existing `SecurityException` one and the new `Exception` one) log the error but never call `gattServer?.sendResponse(device, requestId, GATT_FAILURE, 0, null)`. When `responseNeeded` is `true` and an exception is thrown before the response is sent, the requesting central device will time out waiting for an ATT response. On some BLE stacks this leads to connection-level errors or a full disconnect rather than a clean protocol failure.

Consider sending a `GATT_FAILURE` response in the catch blocks when `responseNeeded` is true, guarded by a nested try/catch to handle any permission error from the call itself.

How can I resolve this? If you propose a fix, please make it concise.

_{Last reviewed commit: "fix: Prevent NullPoi..."}

[greptile-apps]

CancellationException is swallowed by the generic catch (e: Exception) block

CancellationException extends IllegalStateException → RuntimeException → Exception, so this new catch will intercept coroutine cancellation signals. Because handleCharacteristicWriteRequest runs inside withContext(Dispatchers.Main), cancelling the parent coroutine (e.g. during lifecycle teardown) will throw a CancellationException that gets silently swallowed here, logged as "Unexpected error", and the coroutine will appear to have completed normally. This breaks Kotlin structured concurrency and can prevent the BLE server from shutting down cleanly.

The fix is to re-throw CancellationException before logging:

Suggested change

	} catch (e: Exception) {
	Log.e(TAG, "Unexpected error in handleCharacteristicWriteRequest for ${device.address}", e)
	}
	} catch (e: Exception) {
	if (e is kotlinx.coroutines.CancellationException) throw e
	Log.e(TAG, "Unexpected error in handleCharacteristicWriteRequest for ${device.address}", e)
	}

Prompt To Fix With AI

This is a comment left during a code review.
Path: reticulum/src/main/java/com/lxmf/messenger/reticulum/ble/server/BleGattServer.kt
Line: 839-841

Comment:
**`CancellationException` is swallowed by the generic `catch (e: Exception)` block**

`CancellationException` extends `IllegalStateException → RuntimeException → Exception`, so this new catch will intercept coroutine cancellation signals. Because `handleCharacteristicWriteRequest` runs inside `withContext(Dispatchers.Main)`, cancelling the parent coroutine (e.g. during lifecycle teardown) will throw a `CancellationException` that gets silently swallowed here, logged as "Unexpected error", and the coroutine will appear to have completed normally. This breaks Kotlin structured concurrency and can prevent the BLE server from shutting down cleanly.

The fix is to re-throw `CancellationException` before logging:

```suggestion
        } catch (e: Exception) {
            if (e is kotlinx.coroutines.CancellationException) throw e
            Log.e(TAG, "Unexpected error in handleCharacteristicWriteRequest for ${device.address}", e)
        }
```

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

No sendResponse in error paths may leave the central hanging

Both catch blocks (the pre-existing SecurityException one and the new Exception one) log the error but never call gattServer?.sendResponse(device, requestId, GATT_FAILURE, 0, null). When responseNeeded is true and an exception is thrown before the response is sent, the requesting central device will time out waiting for an ATT response. On some BLE stacks this leads to connection-level errors or a full disconnect rather than a clean protocol failure.

Consider sending a GATT_FAILURE response in the catch blocks when responseNeeded is true, guarded by a nested try/catch to handle any permission error from the call itself.

Prompt To Fix With AI

This is a comment left during a code review.
Path: reticulum/src/main/java/com/lxmf/messenger/reticulum/ble/server/BleGattServer.kt
Line: 837-841

Comment:
**No `sendResponse` in error paths may leave the central hanging**

Both catch blocks (the pre-existing `SecurityException` one and the new `Exception` one) log the error but never call `gattServer?.sendResponse(device, requestId, GATT_FAILURE, 0, null)`. When `responseNeeded` is `true` and an exception is thrown before the response is sent, the requesting central device will time out waiting for an ATT response. On some BLE stacks this leads to connection-level errors or a full disconnect rather than a clean protocol failure.

Consider sending a `GATT_FAILURE` response in the catch blocks when `responseNeeded` is true, guarded by a nested try/catch to handle any permission error from the call itself.

How can I resolve this? If you propose a fix, please make it concise.