From a22f63e42c1831b933bb696bac1674c0f477ee01 Mon Sep 17 00:00:00 2001 From: davotoula Date: Mon, 4 May 2026 10:15:22 +0200 Subject: [PATCH 1/3] refactor(log): convert interpolated Log calls to lambda overload Avoid eager string interpolation when the log level is filtered out at runtime. Covers Log.d/i calls and Log.w/e calls that did not pass a throwable. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../amethyst/service/call/CallMediaManager.kt | 2 +- .../service/call/WebRtcCallSession.kt | 14 +++++------ .../amethyst/commons/marmot/MarmotManager.kt | 14 +++++------ .../KeyPackageRotationManager.kt | 12 +++++----- .../nip64Chess/ChessStateReconstructor.kt | 24 +++++++++---------- .../quartz/nip64Chess/LiveChessGameState.kt | 22 ++++++++--------- .../quartz/nipBEBle/relay/BleMeshManager.kt | 4 ++-- .../quartz/nipBEBle/relay/BleNostrClient.kt | 2 +- .../quartz/nipBEBle/relay/BleNostrServer.kt | 2 +- 9 files changed, 48 insertions(+), 48 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/call/CallMediaManager.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/call/CallMediaManager.kt index 3b29f79bd..4949c8055 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/call/CallMediaManager.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/call/CallMediaManager.kt @@ -180,7 +180,7 @@ class CallMediaManager( } override fun onCameraSwitchError(error: String?) { - Log.e(TAG, "Camera switch failed: $error") + Log.e(TAG) { "Camera switch failed: $error" } } }, ) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/call/WebRtcCallSession.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/call/WebRtcCallSession.kt index d9d338811..9ef641c4a 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/call/WebRtcCallSession.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/call/WebRtcCallSession.kt @@ -211,7 +211,7 @@ class WebRtcCallSession( } override fun onCreateFailure(error: String?) { - Log.e(TAG, "Create offer failed: $error") + Log.e(TAG) { "Create offer failed: $error" } error?.let { onError("Create offer failed: $it") } } @@ -245,7 +245,7 @@ class WebRtcCallSession( } override fun onCreateFailure(error: String?) { - Log.e(TAG, "Create answer failed: $error") + Log.e(TAG) { "Create answer failed: $error" } error?.let { onError("Create answer failed: $it") } } @@ -278,7 +278,7 @@ class WebRtcCallSession( } override fun onSetFailure(error: String?) { - Log.e(TAG, "setRemoteDescription FAILED: $error (type=${sdp.type})") + Log.e(TAG) { "setRemoteDescription FAILED: $error (type=${sdp.type})" } error?.let { onError("SDP error: $it") } } }, @@ -314,7 +314,7 @@ class WebRtcCallSession( } override fun onCreateFailure(error: String?) { - Log.e(TAG, "ICE restart offer creation failed: $error") + Log.e(TAG) { "ICE restart offer creation failed: $error" } onError("Connection failed - check network") onDisconnected() } @@ -364,7 +364,7 @@ class WebRtcCallSession( } override fun onSetFailure(error: String?) { - Log.e(TAG, "Rollback FAILED: $error") + Log.e(TAG) { "Rollback FAILED: $error" } error?.let { onError("Rollback failed: $it") } } }, @@ -386,7 +386,7 @@ class WebRtcCallSession( } override fun onCreateFailure(error: String?) { - Log.e(TAG, "$label onCreateFailure: $error") + Log.e(TAG) { "$label onCreateFailure: $error" } error?.let { onError("SDP error: $it") } } @@ -395,7 +395,7 @@ class WebRtcCallSession( } override fun onSetFailure(error: String?) { - Log.e(TAG, "$label onSetFailure: $error") + Log.e(TAG) { "$label onSetFailure: $error" } error?.let { onError("SDP error: $it") } } } diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt index c7003f277..855579b98 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt @@ -142,7 +142,7 @@ class MarmotManager( if (result is WelcomeResult.Joined) { subscriptionManager.subscribeGroup(result.nostrGroupId) - Log.d("MarmotManager", "Joined group ${result.nostrGroupId}") + Log.d("MarmotManager") { "Joined group ${result.nostrGroupId}" } } return result @@ -370,17 +370,17 @@ class MarmotManager( try { groupManager.clearAllState() } catch (e: Exception) { - Log.w("MarmotManager", "resetAllState(): groupManager.clearAllState failed: ${e.message}") + Log.w("MarmotManager") { "resetAllState(): groupManager.clearAllState failed: ${e.message}" } } try { keyPackageRotationManager.clearAllState() } catch (e: Exception) { - Log.w("MarmotManager", "resetAllState(): keyPackageRotationManager.clearAllState failed: ${e.message}") + Log.w("MarmotManager") { "resetAllState(): keyPackageRotationManager.clearAllState failed: ${e.message}" } } try { subscriptionManager.clear() } catch (e: Exception) { - Log.w("MarmotManager", "resetAllState(): subscriptionManager.clear failed: ${e.message}") + Log.w("MarmotManager") { "resetAllState(): subscriptionManager.clear failed: ${e.message}" } } } @@ -405,7 +405,7 @@ class MarmotManager( try { messageStore?.delete(nostrGroupId) } catch (e: Exception) { - Log.w("MarmotManager", "Failed to delete persisted messages for $nostrGroupId: ${e.message}") + Log.w("MarmotManager") { "Failed to delete persisted messages for $nostrGroupId: ${e.message}" } } return outboundEvent } @@ -423,7 +423,7 @@ class MarmotManager( try { messageStore?.appendMessage(nostrGroupId, innerEventJson) } catch (e: Exception) { - Log.w("MarmotManager", "Failed to persist Marmot message for $nostrGroupId: ${e.message}") + Log.w("MarmotManager") { "Failed to persist Marmot message for $nostrGroupId: ${e.message}" } } } @@ -435,7 +435,7 @@ class MarmotManager( try { messageStore?.loadMessages(nostrGroupId) ?: emptyList() } catch (e: Exception) { - Log.w("MarmotManager", "Failed to load persisted messages for $nostrGroupId: ${e.message}") + Log.w("MarmotManager") { "Failed to load persisted messages for $nostrGroupId: ${e.message}" } emptyList() } diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mip00KeyPackages/KeyPackageRotationManager.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mip00KeyPackages/KeyPackageRotationManager.kt index 5edd30945..5061b856e 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mip00KeyPackages/KeyPackageRotationManager.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mip00KeyPackages/KeyPackageRotationManager.kt @@ -103,7 +103,7 @@ class KeyPackageRotationManager( try { store.load() } catch (e: Exception) { - Log.w("KeyPackageRotationManager", "Failed to load persisted KeyPackages: ${e.message}") + Log.w("KeyPackageRotationManager") { "Failed to load persisted KeyPackages: ${e.message}" } null } ?: return try { @@ -117,7 +117,7 @@ class KeyPackageRotationManager( try { store.delete() } catch (e: Exception) { - Log.w("KeyPackageRotationManager", "Failed to delete legacy snapshot: ${e.message}") + Log.w("KeyPackageRotationManager") { "Failed to delete legacy snapshot: ${e.message}" } } return } @@ -133,7 +133,7 @@ class KeyPackageRotationManager( try { store.delete() } catch (e: Exception) { - Log.w("KeyPackageRotationManager", "Failed to delete stale snapshot: ${e.message}") + Log.w("KeyPackageRotationManager") { "Failed to delete stale snapshot: ${e.message}" } } return } @@ -154,7 +154,7 @@ class KeyPackageRotationManager( "${decoded.namedSlotDTags.size} named slot d-tag(s)" } } catch (e: Exception) { - Log.w("KeyPackageRotationManager", "Failed to decode persisted KeyPackages: ${e.message}") + Log.w("KeyPackageRotationManager") { "Failed to decode persisted KeyPackages: ${e.message}" } } } @@ -174,7 +174,7 @@ class KeyPackageRotationManager( try { store.delete() } catch (e: Exception) { - Log.w("KeyPackageRotationManager", "clearAllState(): failed to delete snapshot: ${e.message}") + Log.w("KeyPackageRotationManager") { "clearAllState(): failed to delete snapshot: ${e.message}" } } } @@ -280,7 +280,7 @@ class KeyPackageRotationManager( try { store.save(snapshotBytesUnlocked()) } catch (e: Exception) { - Log.w("KeyPackageRotationManager", "Failed to persist KeyPackages: ${e.message}") + Log.w("KeyPackageRotationManager") { "Failed to persist KeyPackages: ${e.message}" } } } diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip64Chess/ChessStateReconstructor.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip64Chess/ChessStateReconstructor.kt index 297ca80d6..4f730db33 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip64Chess/ChessStateReconstructor.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip64Chess/ChessStateReconstructor.kt @@ -60,7 +60,7 @@ object ChessStateReconstructor { events: JesterGameEvents, viewerPubkey: String, ): ReconstructionResult { - Log.d("chessdebug", "[Reconstructor] reconstruct() called: viewerPubkey=${viewerPubkey.take(8)}, startEvent=${events.startEvent?.id?.take(8)}, moveCount=${events.moves.size}") + Log.d("chessdebug") { "[Reconstructor] reconstruct() called: viewerPubkey=${viewerPubkey.take(8)}, startEvent=${events.startEvent?.id?.take(8)}, moveCount=${events.moves.size}" } // Step 1: Get start event (required for player info) val startEvent = @@ -75,7 +75,7 @@ object ChessStateReconstructor { val challengerColor = startEvent.playerColor() ?: Color.WHITE val challengedPubkey = startEvent.opponentPubkey() - Log.d("chessdebug", "[Reconstructor] startEventId=${startEventId.take(8)}, challenger=${challengerPubkey.take(8)}, challengerColor=$challengerColor, challengedPubkey=${challengedPubkey?.take(8)}") + Log.d("chessdebug") { "[Reconstructor] startEventId=${startEventId.take(8)}, challenger=${challengerPubkey.take(8)}, challengerColor=$challengerColor, challengedPubkey=${challengedPubkey?.take(8)}" } // In Jester, we determine opponent from the p-tag or from move events // For open challenges, we need to find who made the first move @@ -85,7 +85,7 @@ object ChessStateReconstructor { ?.pubKey val actualOpponent = challengedPubkey ?: opponentFromMoves - Log.d("chessdebug", "[Reconstructor] opponentFromMoves=${opponentFromMoves?.take(8)}, actualOpponent=${actualOpponent?.take(8)}") + Log.d("chessdebug") { "[Reconstructor] opponentFromMoves=${opponentFromMoves?.take(8)}, actualOpponent=${actualOpponent?.take(8)}" } // Determine players based on challenger's color choice val (whitePubkey, blackPubkey) = @@ -117,7 +117,7 @@ object ChessStateReconstructor { ViewerRole.SPECTATOR -> blackPubkey ?: "" // For spectators, "opponent" is black } - Log.d("chessdebug", "[Reconstructor] white=${whitePubkey?.take(8)}, black=${blackPubkey?.take(8)}, viewerRole=$viewerRole, playerColor=$playerColor") + Log.d("chessdebug") { "[Reconstructor] white=${whitePubkey?.take(8)}, black=${blackPubkey?.take(8)}, viewerRole=$viewerRole, playerColor=$playerColor" } // Check if game is pending (no moves yet AND viewer is the challenger) // If viewer accepted someone else's challenge, the game is NOT pending @@ -129,9 +129,9 @@ object ChessStateReconstructor { val latestMove = events.latestMove() val history = latestMove?.history() ?: emptyList() - Log.d("chessdebug", "[Reconstructor] latestMove=${latestMove?.id?.take(8)}, historySize=${history.size}, isPendingChallenge=$isPendingChallenge") + Log.d("chessdebug") { "[Reconstructor] latestMove=${latestMove?.id?.take(8)}, historySize=${history.size}, isPendingChallenge=$isPendingChallenge" } if (history.isNotEmpty()) { - Log.d("chessdebug", "[Reconstructor] history: ${history.joinToString(" ")}") + Log.d("chessdebug") { "[Reconstructor] history: ${history.joinToString(" ")}" } } // Track applied moves @@ -147,10 +147,10 @@ object ChessStateReconstructor { } else { // Move failed - game might be desynced isDesynced = true - Log.d("chessdebug", "[Reconstructor] DESYNC: move #$moveNumber '$san' failed: ${result.error}") + Log.d("chessdebug") { "[Reconstructor] DESYNC: move #$moveNumber '$san' failed: ${result.error}" } // Try to recover by loading the FEN from latest move if available latestMove?.fen()?.let { fen -> - Log.d("chessdebug", "[Reconstructor] Recovering with FEN: $fen") + Log.d("chessdebug") { "[Reconstructor] Recovering with FEN: $fen" } engine.loadFen(fen) } break @@ -162,7 +162,7 @@ object ChessStateReconstructor { val currentFen = engine.getFen() if (!fenPositionsMatch(currentFen, expectedFen)) { isDesynced = true - Log.d("chessdebug", "[Reconstructor] FEN MISMATCH: current=$currentFen, expected=$expectedFen") + Log.d("chessdebug") { "[Reconstructor] FEN MISMATCH: current=$currentFen, expected=$expectedFen" } engine.loadFen(expectedFen) } } @@ -173,13 +173,13 @@ object ChessStateReconstructor { when { gameResult != null -> { val result = parseGameResult(gameResult) - Log.d("chessdebug", "[Reconstructor] Game finished from result tag: $gameResult -> $result") + Log.d("chessdebug") { "[Reconstructor] Game finished from result tag: $gameResult -> $result" } GameStatus.Finished(result) } engine.isCheckmate() -> { val winner = engine.getSideToMove().opposite() - Log.d("chessdebug", "[Reconstructor] Checkmate detected, winner=$winner") + Log.d("chessdebug") { "[Reconstructor] Checkmate detected, winner=$winner" } GameStatus.Finished( if (winner == Color.WHITE) GameResult.WHITE_WINS else GameResult.BLACK_WINS, ) @@ -219,7 +219,7 @@ object ChessStateReconstructor { timeControl = null, // Not supported in Jester ) - Log.d("chessdebug", "[Reconstructor] Result: status=$gameStatus, appliedMoves=${appliedMoveNumbers.size}, isDesynced=$isDesynced, headEvent=${headEventId.take(8)}") + Log.d("chessdebug") { "[Reconstructor] Result: status=$gameStatus, appliedMoves=${appliedMoveNumbers.size}, isDesynced=$isDesynced, headEvent=${headEventId.take(8)}" } return ReconstructionResult.Success(state, engine) } diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip64Chess/LiveChessGameState.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip64Chess/LiveChessGameState.kt index 7a49956ab..2d8167483 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip64Chess/LiveChessGameState.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip64Chess/LiveChessGameState.kt @@ -126,15 +126,15 @@ class LiveChessGameState( to: String, promotion: PieceType? = null, ): ChessMoveEvent? { - Log.d("chessdebug", "[LiveGame] makeMove: game=${startEventId.take(8)}, from=$from, to=$to, promotion=$promotion, isPlayerTurn=${isPlayerTurn()}, status=${_gameStatus.value}") + Log.d("chessdebug") { "[LiveGame] makeMove: game=${startEventId.take(8)}, from=$from, to=$to, promotion=$promotion, isPlayerTurn=${isPlayerTurn()}, status=${_gameStatus.value}" } if (!isPlayerTurn()) { - Log.d("chessdebug", "[LiveGame] makeMove REJECTED: not player's turn (sideToMove=${engine.getSideToMove()}, playerColor=$playerColor)") + Log.d("chessdebug") { "[LiveGame] makeMove REJECTED: not player's turn (sideToMove=${engine.getSideToMove()}, playerColor=$playerColor)" } _lastError.value = "Not your turn" return null } if (_gameStatus.value != GameStatus.InProgress) { - Log.d("chessdebug", "[LiveGame] makeMove REJECTED: game not in progress (${_gameStatus.value})") + Log.d("chessdebug") { "[LiveGame] makeMove REJECTED: game not in progress (${_gameStatus.value})" } _lastError.value = "Game is not in progress" return null } @@ -175,7 +175,7 @@ class LiveChessGameState( * @param newHeadEventId The ID of the newly published move event */ fun updateHeadEventId(newHeadEventId: String) { - Log.d("chessdebug", "[LiveGame] updateHeadEventId: game=${startEventId.take(8)}, old=${_headEventId.value.take(8)}, new=${newHeadEventId.take(8)}") + Log.d("chessdebug") { "[LiveGame] updateHeadEventId: game=${startEventId.take(8)}, old=${_headEventId.value.take(8)}, new=${newHeadEventId.take(8)}" } _headEventId.value = newHeadEventId } @@ -209,10 +209,10 @@ class LiveChessGameState( fen: String, moveNumber: Int? = null, ): Boolean { - Log.d("chessdebug", "[LiveGame] applyOpponentMove: game=${startEventId.take(8)}, san=$san, moveNumber=$moveNumber, expectedMove=${_moveHistory.value.size + 1}") + Log.d("chessdebug") { "[LiveGame] applyOpponentMove: game=${startEventId.take(8)}, san=$san, moveNumber=$moveNumber, expectedMove=${_moveHistory.value.size + 1}" } // Check for duplicate move if (moveNumber != null && receivedMoveNumbers.contains(moveNumber)) { - Log.d("chessdebug", "[LiveGame] applyOpponentMove: DUPLICATE move #$moveNumber, ignoring") + Log.d("chessdebug") { "[LiveGame] applyOpponentMove: DUPLICATE move #$moveNumber, ignoring" } // Already processed this move, ignore return true } @@ -220,7 +220,7 @@ class LiveChessGameState( // Check for out-of-order move val expectedMoveNumber = _moveHistory.value.size + 1 if (moveNumber != null && moveNumber > expectedMoveNumber) { - Log.d("chessdebug", "[LiveGame] applyOpponentMove: OUT-OF-ORDER move #$moveNumber (expected #$expectedMoveNumber), queuing") + Log.d("chessdebug") { "[LiveGame] applyOpponentMove: OUT-OF-ORDER move #$moveNumber (expected #$expectedMoveNumber), queuing" } // Store for later processing pendingMoves[moveNumber] = san to fen return true @@ -246,13 +246,13 @@ class LiveChessGameState( if (currentFen != fen) { // Positions don't match - desync detected _isDesynced.value = true - Log.d("chessdebug", "[LiveGame] applyOpponentMove: FEN MISMATCH after $san - current=$currentFen, expected=$fen") + Log.d("chessdebug") { "[LiveGame] applyOpponentMove: FEN MISMATCH after $san - current=$currentFen, expected=$fen" } _lastError.value = "Position mismatch - syncing to opponent's position" // Load the opponent's FEN to stay in sync engine.loadFen(fen) } else { _isDesynced.value = false - Log.d("chessdebug", "[LiveGame] applyOpponentMove: SUCCESS - $san applied, totalMoves=${_moveHistory.value.size + 1}") + Log.d("chessdebug") { "[LiveGame] applyOpponentMove: SUCCESS - $san applied, totalMoves=${_moveHistory.value.size + 1}" } } _currentPosition.value = engine.getPosition() @@ -271,7 +271,7 @@ class LiveChessGameState( return true } else { - Log.d("chessdebug", "[LiveGame] applyOpponentMove: INVALID move $san - ${result.error}") + Log.d("chessdebug") { "[LiveGame] applyOpponentMove: INVALID move $san - ${result.error}" } _lastError.value = "Invalid opponent move: $san" return false } @@ -524,7 +524,7 @@ class LiveChessGameState( * Used when loading a game from cache that already has an end event. */ fun markAsFinished(result: GameResult) { - Log.d("chessdebug", "[LiveGame] markAsFinished: game=${startEventId.take(8)}, result=$result") + Log.d("chessdebug") { "[LiveGame] markAsFinished: game=${startEventId.take(8)}, result=$result" } _gameStatus.value = GameStatus.Finished(result) } diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nipBEBle/relay/BleMeshManager.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nipBEBle/relay/BleMeshManager.kt index f2f4a6574..9e33fb9b9 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nipBEBle/relay/BleMeshManager.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nipBEBle/relay/BleMeshManager.kt @@ -155,7 +155,7 @@ class BleMeshManager( val role = assignRole(transport.deviceUuid, peerUuid) val peer = BlePeer(peerUuid, role, platformHandle) - Log.d("BleMeshManager", "Discovered peer $peerUuid, my role: $role") + Log.d("BleMeshManager") { "Discovered peer $peerUuid, my role: $role" } when (role) { BleRole.CLIENT -> { @@ -239,7 +239,7 @@ class BleMeshManager( peer: BlePeer?, error: String, ) { - Log.e("BleMeshManager", "BLE error for peer ${peer?.deviceUuid}: $error") + Log.e("BleMeshManager") { "BLE error for peer ${peer?.deviceUuid}: $error" } listener.onError(peer, error) } diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nipBEBle/relay/BleNostrClient.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nipBEBle/relay/BleNostrClient.kt index 2c75d9576..1a0771d2e 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nipBEBle/relay/BleNostrClient.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nipBEBle/relay/BleNostrClient.kt @@ -195,7 +195,7 @@ class BleNostrClient( if (success) { currentChunkIndex++ } else { - Log.e("BleNostrClient", "Failed to write chunk $currentChunkIndex to ${peer.deviceUuid}") + Log.e("BleNostrClient") { "Failed to write chunk $currentChunkIndex to ${peer.deviceUuid}" } } } } diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nipBEBle/relay/BleNostrServer.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nipBEBle/relay/BleNostrServer.kt index 5a8d6d39b..6b74d3e4b 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nipBEBle/relay/BleNostrServer.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nipBEBle/relay/BleNostrServer.kt @@ -172,7 +172,7 @@ class BleNostrServer( if (success) { currentChunkIndex++ } else { - Log.e("BleNostrServer", "Failed to notify chunk $currentChunkIndex to ${peer.deviceUuid}") + Log.e("BleNostrServer") { "Failed to notify chunk $currentChunkIndex to ${peer.deviceUuid}" } } } } From 31dee7fe3813219f1ab806df4d179bf6c504d5eb Mon Sep 17 00:00:00 2001 From: davotoula Date: Mon, 4 May 2026 10:52:39 +0200 Subject: [PATCH 2/3] fix(log): pass throwable to Log.w in Marmot catch blocks Previously the catch-block warnings interpolated ${e.message} but dropped the throwable, losing the stack trace and showing nothing for exceptions whose message is null. Switch to the (tag, msg, throwable) overload so the cause and stack trace are logged. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../amethyst/commons/marmot/MarmotManager.kt | 12 ++++++------ .../mip00KeyPackages/KeyPackageRotationManager.kt | 12 ++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt index 855579b98..54a6851d4 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt @@ -370,17 +370,17 @@ class MarmotManager( try { groupManager.clearAllState() } catch (e: Exception) { - Log.w("MarmotManager") { "resetAllState(): groupManager.clearAllState failed: ${e.message}" } + Log.w("MarmotManager", "resetAllState(): groupManager.clearAllState failed", e) } try { keyPackageRotationManager.clearAllState() } catch (e: Exception) { - Log.w("MarmotManager") { "resetAllState(): keyPackageRotationManager.clearAllState failed: ${e.message}" } + Log.w("MarmotManager", "resetAllState(): keyPackageRotationManager.clearAllState failed", e) } try { subscriptionManager.clear() } catch (e: Exception) { - Log.w("MarmotManager") { "resetAllState(): subscriptionManager.clear failed: ${e.message}" } + Log.w("MarmotManager", "resetAllState(): subscriptionManager.clear failed", e) } } @@ -405,7 +405,7 @@ class MarmotManager( try { messageStore?.delete(nostrGroupId) } catch (e: Exception) { - Log.w("MarmotManager") { "Failed to delete persisted messages for $nostrGroupId: ${e.message}" } + Log.w("MarmotManager", "Failed to delete persisted messages for $nostrGroupId", e) } return outboundEvent } @@ -423,7 +423,7 @@ class MarmotManager( try { messageStore?.appendMessage(nostrGroupId, innerEventJson) } catch (e: Exception) { - Log.w("MarmotManager") { "Failed to persist Marmot message for $nostrGroupId: ${e.message}" } + Log.w("MarmotManager", "Failed to persist Marmot message for $nostrGroupId", e) } } @@ -435,7 +435,7 @@ class MarmotManager( try { messageStore?.loadMessages(nostrGroupId) ?: emptyList() } catch (e: Exception) { - Log.w("MarmotManager") { "Failed to load persisted messages for $nostrGroupId: ${e.message}" } + Log.w("MarmotManager", "Failed to load persisted messages for $nostrGroupId", e) emptyList() } diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mip00KeyPackages/KeyPackageRotationManager.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mip00KeyPackages/KeyPackageRotationManager.kt index 5061b856e..4265e6cf5 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mip00KeyPackages/KeyPackageRotationManager.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mip00KeyPackages/KeyPackageRotationManager.kt @@ -103,7 +103,7 @@ class KeyPackageRotationManager( try { store.load() } catch (e: Exception) { - Log.w("KeyPackageRotationManager") { "Failed to load persisted KeyPackages: ${e.message}" } + Log.w("KeyPackageRotationManager", "Failed to load persisted KeyPackages", e) null } ?: return try { @@ -117,7 +117,7 @@ class KeyPackageRotationManager( try { store.delete() } catch (e: Exception) { - Log.w("KeyPackageRotationManager") { "Failed to delete legacy snapshot: ${e.message}" } + Log.w("KeyPackageRotationManager", "Failed to delete legacy snapshot", e) } return } @@ -133,7 +133,7 @@ class KeyPackageRotationManager( try { store.delete() } catch (e: Exception) { - Log.w("KeyPackageRotationManager") { "Failed to delete stale snapshot: ${e.message}" } + Log.w("KeyPackageRotationManager", "Failed to delete stale snapshot", e) } return } @@ -154,7 +154,7 @@ class KeyPackageRotationManager( "${decoded.namedSlotDTags.size} named slot d-tag(s)" } } catch (e: Exception) { - Log.w("KeyPackageRotationManager") { "Failed to decode persisted KeyPackages: ${e.message}" } + Log.w("KeyPackageRotationManager", "Failed to decode persisted KeyPackages", e) } } @@ -174,7 +174,7 @@ class KeyPackageRotationManager( try { store.delete() } catch (e: Exception) { - Log.w("KeyPackageRotationManager") { "clearAllState(): failed to delete snapshot: ${e.message}" } + Log.w("KeyPackageRotationManager", "clearAllState(): failed to delete snapshot", e) } } @@ -280,7 +280,7 @@ class KeyPackageRotationManager( try { store.save(snapshotBytesUnlocked()) } catch (e: Exception) { - Log.w("KeyPackageRotationManager") { "Failed to persist KeyPackages: ${e.message}" } + Log.w("KeyPackageRotationManager", "Failed to persist KeyPackages", e) } } From 72fcdfb6d16dfc89c4f2e00ba016b775a0dd615a Mon Sep 17 00:00:00 2001 From: davotoula Date: Mon, 4 May 2026 12:47:22 +0200 Subject: [PATCH 3/3] docs(skill): extend find-non-lambda-logs to flag dropped throwables Add Step 3 to the audit: catch-block Log.w/e calls that interpolate ${e.message} but don't pass `e` lose the stack trace and log "...: null" when the exception's message is null. Document the throwable-overload fix alongside the existing lambda-overload conversion. Co-Authored-By: Claude Opus 4.7 (1M context) --- .claude/skills/find-non-lambda-logs/SKILL.md | 53 +++++++++++++++++--- 1 file changed, 46 insertions(+), 7 deletions(-) diff --git a/.claude/skills/find-non-lambda-logs/SKILL.md b/.claude/skills/find-non-lambda-logs/SKILL.md index 0b0009255..7b87d2a41 100644 --- a/.claude/skills/find-non-lambda-logs/SKILL.md +++ b/.claude/skills/find-non-lambda-logs/SKILL.md @@ -1,13 +1,16 @@ --- name: find-non-lambda-logs -description: Use when auditing or migrating Log calls to lambda overloads, after adding new logging, or checking for string interpolation in Log.d/i/w/e calls that waste allocations when the log level is filtered out +description: Use when auditing or migrating Log calls — flags both interpolated Log.d/i/w/e that should use the lambda overload (allocation hygiene) and catch-block Log.w/e that interpolate ${e.message} but drop the throwable (lost stack traces) --- # Find Non-Lambda Log Calls ## Overview -Locates `Log.d/i/w/e` calls that use string interpolation without the lambda overload, wasting string allocation when the log level is filtered out in release builds. +Two related logging hygiene issues: + +1. **Lambda overload missing.** `Log.d/i/w/e` calls that use string interpolation without the lambda overload waste string allocation when the log level is filtered out in release builds. +2. **Throwable dropped in catch blocks.** `Log.w/e` calls inside `catch (e: ...)` blocks that interpolate `${e.message}` but don't pass `e` lose the stack trace, and log nothing useful when `e.message` is null (NPE, IOException with no message, etc.). ## When to Use @@ -60,7 +63,22 @@ type: kotlin Then **manually exclude** lines where a throwable is passed as third argument (ending with `, e)`, `, throwable)`, etc.). Check the actual line — a catch block catching `e` doesn't mean `e` is passed to the Log call. -### Step 3: Verify no android.util.Log leakage +### Step 3: Find catch-block Log.w/e that drop the throwable + +Among the Step 2 hits, the calls that interpolate `${e.message}` (or `${t.message}`, `${throwable.message}`) but do not pass the exception itself are a separate bug — they lose the stack trace AND log a useless empty-ish line whenever the exception's message is null. + +Quick filter: + +``` +pattern: Log\.(w|e)\([^)]*\$\{(e|t|throwable|cause)\.message\}[^)]*\)$ +type: kotlin +``` + +Then for each hit, open the file and confirm the line is **inside a `catch (e: ...)` block** and **does not pass `e` (or the matching name) as a third argument**. False positives: extension functions / helpers that accept an `e: SomeError` parameter and forward it elsewhere. + +Both Step 2 and Step 3 may flag the same line — handle Step 3 first (different fix), then apply Step 2 to whatever remains. + +### Step 4: Verify no android.util.Log leakage ``` pattern: android\.util\.Log\.(d|i|w|e|v)\( @@ -69,7 +87,9 @@ type: kotlin These bypass the `Log.minLevel` filter entirely. Exclude `PlatformLog.android.kt` which is the wrapper implementation. -## Fix Pattern +## Fix Patterns + +### Lambda overload (Step 1 + Step 2) ```kotlin // Before @@ -79,8 +99,27 @@ Log.d("Tag", "Processing event ${event.id} from ${relay.url}") Log.d("Tag") { "Processing event ${event.id} from ${relay.url}" } ``` +### Throwable overload (Step 3) + +Switch to `(tag, msg, throwable)` — the lambda overload does **not** accept a throwable, so this case must use the eager-string form. Drop the redundant `${e.message}` from the message text since the throwable already carries it. + +```kotlin +// Before — stack trace lost, prints "...failed: null" if e.message is null +try { groupManager.clearAllState() } catch (e: Exception) { + Log.w("MarmotManager") { "clearAllState failed: ${e.message}" } +} + +// After — full stack trace logged +try { groupManager.clearAllState() } catch (e: Exception) { + Log.w("MarmotManager", "clearAllState failed", e) +} +``` + +Trade-off: the message string is allocated eagerly even when warn is filtered, but warn-level catch logs are rare-event paths so this cost is negligible compared to losing diagnostic detail. + ## Do NOT Convert -- Calls passing a `Throwable` parameter - the lambda overload `(tag) { message }` has no throwable parameter -- Static string calls with no `$` interpolation - no allocation benefit -- Commented-out log calls +- **To lambda:** calls passing a `Throwable` parameter — the lambda overload `(tag) { message }` has no throwable parameter. +- Static string calls with no `$` interpolation — no allocation benefit. +- Commented-out log calls. +- Informational/intentional log of `e.message` *outside* a catch block (rare; usually means the exception was already handled and only the message is meaningful).