From 72fcdfb6d16dfc89c4f2e00ba016b775a0dd615a Mon Sep 17 00:00:00 2001
From: davotoula <david.kaspar@gmail.com>
Date: Mon, 4 May 2026 12:47:22 +0200
Subject: [PATCH] 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) <noreply@anthropic.com>
---
 .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 0b00092553..7b87d2a415 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).