v0.23.0: Offline crash decode generates no decoding

[Jason2866]

@coderabbitai analyze what is the reason for broken offline decoding introduced with PR #38

[Jason2866]

@coderabbitai analyze what is the reason for broken offline decoding introduced with PR #38

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Get PR `#38` details and diff
gh pr view 38 --repo Jason2866/ESP-Decoder --json title,body,files,commits
echo "---FILES CHANGED---"
gh pr diff 38 --repo Jason2866/ESP-Decoder

Length of output: 220775

---

🏁 Script executed:

#!/bin/bash
# Get the current crashDecoder.ts
fd -e ts crashDecoder --exec cat {}

echo "=== Check how decode() result is used in crashDecoder.ts ==="
rg -n "decodeCrash|decodeCoredump|decode(" src/crashDecoder.ts 2>/dev/null || true

echo "=== Check stacktraceLines filter in xtensa.js ==="
cat src/vendor/trbr/decode/xtensa.js | grep -A5 "stacktraceLines"

echo "=== Check decode() return shape for coredump ==="
grep -n "fixDecodeResult\|threadDecodeResult\|result\.map" src/vendor/trbr/decode/decode.js | head -30

echo "=== Check index.js exports ==="
cat src/vendor/trbr/index.js

echo "=== Check what crashDecoder imports from vendor/trbr ==="
grep -n "^import" src/crashDecoder.ts | head -20

Length of output: 89480

---

@Jason2866 Here’s what changed with PR #38 and why offline crash decode can show “no decoding” in v0.23.0:

What changed

• We now vendored trbr and call trbr.decode() for selection/offline decode whenever a toolPath exists and is executable (src/crashDecoder.ts).
• The Xtensa decoder in trbr only keeps frames that were successfully resolved via GDB/addr2line:
  • In src/vendor/trbr/decode/xtensa.js the frames are filtered with .filter(isGDBLine), i.e., anything that isn’t a parsed GDB line is dropped. I verified this in the current tree: the snippet shows stacktraceLines: addrLines.map(({ location }) => location).filter(isGDBLine).

Why this breaks “offline” in some setups

• If a toolPath is present but unusable (wrong-arch GDB, incompatible toolset, or can’t resolve against the provided ELF), trbr.decode() can complete without throwing, but yield zero stacktrace lines (because of the isGDBLine filter).
• In src/crashDecoder.ts, after calling trbr.decode(), we only log a warning when 0 frames are returned and still return the empty result; we do not fall back to our raw text-only decoder in this case.
• The pre-trbr text heuristics (Backtrace: pairs or >>>stack>>> blocks) that don’t need GDB are now gated behind addr2line availability; if a toolPath exists but can’t actually resolve, those heuristics won’t run either, and we end up with an empty stacktrace.

So the practical regression is: “tool present but not actually working” → trbr returns 0 frames → we don’t fall back → UI shows nothing useful.

Concrete code pointers

• Frame filtering that hides non-GDB lines:
  • src/vendor/trbr/decode/xtensa.js
    stacktraceLines: addrLines.map(({ location }) => location).filter(isGDBLine)
• No-fallback-on-empty in our wrapper:
  • src/crashDecoder.ts, after:
    const result = await decode(params, crashEvent.rawText, decodeOptions);
    const decRes = Array.isArray(result) ? result[0]?.result ?? result[0] : result;
    // We only write a warning when (decRes?.stacktraceLines?.length ?? 0) === 0 and still return convertDecodeResult(result,...).

Minimal, safe fix

• In src/crashDecoder.ts, add a fallback when trbr returns no frames:
  • If (decRes?.stacktraceLines?.length ?? 0) === 0, return createRawDecode(crashEvent.rawText) instead (set toolsMissing = true so the user sees the warning). This restores the previous behavior for “present-but-broken” toolchains without touching the happy path.

Optional robustness tweak

• Broaden isToolAccessError(...) to also treat common GDB messages as tool failures (“not in executable format”, “file format not recognized”, etc.). That will steer more bad-tool cases into the raw fallback automatically.

Why I’m confident this is the root cause

• The xtensa decoder explicitly filters to parsed GDB frames only; with a misconfigured toolPath that doesn’t actually resolve, trbr returns 0 frames by design.
• Our wrapper currently only falls back to raw decoding when the tool is missing/not-executable (or throws and matches a narrow error predicate) — not when we get a “successful but empty” result. That exact gap was introduced with the refactor in PR #38.

If you want, I can prep a small PR that:

• Adds the empty-result fallback (5–10 lines in decodeCrash),
• Extends isToolAccessError(...) with the two GDB messages mentioned above.

This will bring back useful offline decoding (addresses + minimal fault info) even when a toolPath exists but cannot be used.

---

🧠 Learnings used

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 0
File: :0-0
Timestamp: 2026-04-28T12:52:35.037Z
Learning: In Jason2866/ESP-Decoder, src/vendor/trbr/capturer/framer.js: Bug 3 (isCompleteBlock() dead code / ESP8266 `>>>stack>>>` not recognized as terminator) was completely fixed by commit 98e3ffc56697ad6a0a38b142eb4b9e46668ae451. The fix wires isCompleteBlock() as a required AND-condition inside _finalize(). Note: isCompleteBlock() is still only evaluated at finalization time (quiet-period timer or flush), not proactively per line — so ESP8266 blocks still wait the 500ms quiet period rather than being finalized immediately when `>>>stack>>>` arrives. This is acceptable behavior, not a bug.

---

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 0
File: :0-0
Timestamp: 2026-04-28T09:30:14.062Z
Learning: In Jason2866/ESP-Decoder, src/vendor/trbr/decode/coredump.js: The ELF64 fallback path is out of scope — trbr only targets ESP32/ESP8266, which are both ELF32; no ELF64 coredumps exist in this context.

---

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 0
File: :0-0
Timestamp: 2026-04-28T09:30:14.062Z
Learning: In Jason2866/ESP-Decoder, src/vendor/trbr/capturer/framer.js: The hasSignal redundancy is intentional — it is kept as a defensive safeguard, and the refactor risk outweighs the cleanup benefit.

---

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 0
File: :0-0
Timestamp: 2026-04-28T13:44:19.117Z
Learning: In Jason2866/ESP-Decoder, src/vendor/trbr/capturer/framer.js: Commit 34aa7b78b5a0708b2be6e12cb9394e1818dfa7e3 implements immediate ESP8266 finalization. A new isImmediateFinalizeLine() function matches both /^Rebooting\.\.\./i and /^<<<stack<<</i, replacing the old single-pattern per-line check in pushData. ESP8266 blocks are now finalized immediately when '<<<stack<<<' (the stack end marker) arrives, without waiting for the 500ms quiet-period timer. Note: '>>>stack>>>' is the start marker; '<<<stack<<<' is the end marker.

---

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 0
File: :0-0
Timestamp: 2026-04-28T09:30:14.062Z
Learning: In Jason2866/ESP-Decoder, src/vendor/trbr/decode/decode.js: Using JSON.parse(JSON.stringify(...)) for deep-cloning is intentional; all clone targets are plain JSON-safe data objects, so switching to structuredClone would be purely cosmetic.

---

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 0
File: :0-0
Timestamp: 2026-04-28T09:30:14.062Z
Learning: In Jason2866/ESP-Decoder, src/vendor/trbr/decode/_tinyrainbow.js: The FORCE_COLOR warning is a non-issue because esp-decoder always passes `color: 'disable'` to stringifyDecodeResult, so the no-op color shim never runs in colored mode.

---

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 0
File: :0-0
Timestamp: 2026-04-21T10:36:15.736Z
Learning: In Jason2866/ESP-Decoder (src/webviewPanel.ts), the `reverse` video mode combined with a partially-set RGB color (only fg or only bg having an RGB value) is a known pre-existing limitation. Without knowing the terminal's default colors, there is no correct fix. This should not be raised as a new issue in future reviews.

---

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 25
File: src/extension.ts:507-528
Timestamp: 2026-04-09T15:32:51.709Z
Learning: In Jason2866/ESP-Decoder (src/extension.ts), the `isEspDevice()` function intentionally matches all common ESP bridge chips (CP210x VID 10c4, CH340 VID 1a86, FTDI VID 0403, Espressif native VID 303a) as well as manufacturer strings. This broad matching is by design: the extension is purpose-built for ESP devices, and most ESP boards use generic bridge chips. Users who want to disable auto-connect entirely use the `esp-decoder.autoConnect = "off"` setting. Do NOT suggest splitting into high-confidence vs. low-confidence tiers — the maintainer has explicitly rejected this approach.

---

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 13
File: src/webviewPanel.ts:102-106
Timestamp: 2026-03-28T16:41:54.551Z
Learning: In Jason2866/ESP-Decoder (src/webviewPanel.ts), the repeated `vscode.window.showWarningMessage` calls for `toolsMissing` across all decode paths (auto-decode, manual decodeCrash, decodeCoredumpElf, decodeCoredumpBase64) are intentional. The maintainer wants the warning shown on every decode attempt where tools are missing, not just once per session, because every such attempt produces an incorrect result and maximum warning visibility is the desired UX.

[Jason2866]

@coderabbitai you are wrong, fixed with 3c40322

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
gh api repos/Jason2866/ESP-Decoder/commits/3c40322f22b94b7c053c0274a8ba22ed6e0e9151 --jq '.commit.message'
echo "---DIFF---"
gh api repos/Jason2866/ESP-Decoder/commits/3c40322f22b94b7c053c0274a8ba22ed6e0e9151 | jq -r '.files[] | "FILE: " + .filename + "\n" + .patch'

Length of output: 1156

---

@Jason2866, apologies — my analysis was incorrect. I went down the wrong path focusing on isGDBLine filtering in trbr.

The actual root cause was much simpler: when crash text is pasted (offline / selection decode), crashCapturer.pushData() was called but crashCapturer.flush() was never called afterward. Because pasted data is a one-shot with no natural terminator (Rebooting..., Backtrace:, etc.) following it, the framer's quiet-period timer is the only thing that would ever finalize the block — and in the selection-decode flow that timer apparently never fires either. The crash block just sat unfinalised in the capturer and was never decoded.

The fix in commit 3c40322 is clean and correct:

• Paste path (message.text): call this.crashCapturer.flush() immediately after this.crashCapturer.pushData(data).
• Disconnect/cleanup path: call this.crashCapturer.flush() after this.utf8Decoder.end() to drain any pending block when the connection drops before a terminator arrives.

Thanks for the correction!

---

✏️ Learnings added

Learnt from: Jason2866
URL: https://github.com/Jason2866/ESP-Decoder/issues/40

Timestamp: 2026-04-28T15:23:51.335Z
Learning: In Jason2866/ESP-Decoder (src/webviewPanel.ts), the offline/selection decode was broken because crashCapturer.flush() was never called after pushing pasted data. Pasted data is a one-shot with no natural terminator, so the framer's quiet-period timer never fires and the crash block sits unfinalized. Fixed in commit 3c40322f22b94b7c053c0274a8ba22ed6e0e9151 by adding this.crashCapturer.flush() immediately after this.crashCapturer.pushData(data) in the paste path, and also after this.utf8Decoder.end() in the disconnect/cleanup path.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

---

🧠 Learnings used

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 0
File: :0-0
Timestamp: 2026-04-28T12:52:35.037Z
Learning: In Jason2866/ESP-Decoder, src/vendor/trbr/capturer/framer.js: Bug 3 (isCompleteBlock() dead code / ESP8266 `>>>stack>>>` not recognized as terminator) was completely fixed by commit 98e3ffc56697ad6a0a38b142eb4b9e46668ae451. The fix wires isCompleteBlock() as a required AND-condition inside _finalize(). Note: isCompleteBlock() is still only evaluated at finalization time (quiet-period timer or flush), not proactively per line — so ESP8266 blocks still wait the 500ms quiet period rather than being finalized immediately when `>>>stack>>>` arrives. This is acceptable behavior, not a bug.

---

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 13
File: src/webviewPanel.ts:102-106
Timestamp: 2026-03-28T16:41:54.551Z
Learning: In Jason2866/ESP-Decoder (src/webviewPanel.ts), the repeated `vscode.window.showWarningMessage` calls for `toolsMissing` across all decode paths (auto-decode, manual decodeCrash, decodeCoredumpElf, decodeCoredumpBase64) are intentional. The maintainer wants the warning shown on every decode attempt where tools are missing, not just once per session, because every such attempt produces an incorrect result and maximum warning visibility is the desired UX.

---

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 0
File: :0-0
Timestamp: 2026-04-28T09:30:14.062Z
Learning: In Jason2866/ESP-Decoder, src/vendor/trbr/decode/coredump.js: The ELF64 fallback path is out of scope — trbr only targets ESP32/ESP8266, which are both ELF32; no ELF64 coredumps exist in this context.

---

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 0
File: :0-0
Timestamp: 2026-04-21T10:36:15.736Z
Learning: In Jason2866/ESP-Decoder (src/webviewPanel.ts), the `reverse` video mode combined with a partially-set RGB color (only fg or only bg having an RGB value) is a known pre-existing limitation. Without knowing the terminal's default colors, there is no correct fix. This should not be raised as a new issue in future reviews.

---

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 0
File: :0-0
Timestamp: 2026-04-28T09:30:14.062Z
Learning: In Jason2866/ESP-Decoder, src/vendor/trbr/decode/_tinyrainbow.js: The FORCE_COLOR warning is a non-issue because esp-decoder always passes `color: 'disable'` to stringifyDecodeResult, so the no-op color shim never runs in colored mode.

---

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 0
File: :0-0
Timestamp: 2026-04-28T09:30:14.062Z
Learning: In Jason2866/ESP-Decoder, src/vendor/trbr/capturer/framer.js: The hasSignal redundancy is intentional — it is kept as a defensive safeguard, and the refactor risk outweighs the cleanup benefit.

---

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 0
File: :0-0
Timestamp: 2026-04-28T13:44:19.117Z
Learning: In Jason2866/ESP-Decoder, src/vendor/trbr/capturer/framer.js: Commit 34aa7b78b5a0708b2be6e12cb9394e1818dfa7e3 implements immediate ESP8266 finalization. A new isImmediateFinalizeLine() function matches both /^Rebooting\.\.\./i and /^<<<stack<<</i, replacing the old single-pattern per-line check in pushData. ESP8266 blocks are now finalized immediately when '<<<stack<<<' (the stack end marker) arrives, without waiting for the 500ms quiet-period timer. Note: '>>>stack>>>' is the start marker; '<<<stack<<<' is the end marker.

---

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 25
File: src/extension.ts:507-528
Timestamp: 2026-04-09T15:32:51.709Z
Learning: In Jason2866/ESP-Decoder (src/extension.ts), the `isEspDevice()` function intentionally matches all common ESP bridge chips (CP210x VID 10c4, CH340 VID 1a86, FTDI VID 0403, Espressif native VID 303a) as well as manufacturer strings. This broad matching is by design: the extension is purpose-built for ESP devices, and most ESP boards use generic bridge chips. Users who want to disable auto-connect entirely use the `esp-decoder.autoConnect = "off"` setting. Do NOT suggest splitting into high-confidence vs. low-confidence tiers — the maintainer has explicitly rejected this approach.

---

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 0
File: :0-0
Timestamp: 2026-04-28T09:30:14.062Z
Learning: In Jason2866/ESP-Decoder, src/vendor/trbr/decode/decode.js: Using JSON.parse(JSON.stringify(...)) for deep-cloning is intentional; all clone targets are plain JSON-safe data objects, so switching to structuredClone would be purely cosmetic.

---

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 0
File: :0-0
Timestamp: 2026-04-21T10:36:15.736Z
Learning: In Jason2866/ESP-Decoder, the `ANSI_256` 256-color palette IIFE in src/webviewPanel.ts is defined inside the webview <script> block (injected HTML), not in the TypeScript host code. It therefore runs exactly once when the webview page loads — there is no per-call rebuild cost. Do not flag it as a performance issue.

---

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 0
File: :0-0
Timestamp: 2026-04-28T09:30:14.062Z
Learning: In Jason2866/ESP-Decoder, src/vendor/trbr/decode/stringify.js: Mutation of process.env.FORCE_COLOR is intentional and safe — it is only triggered by `color: 'force'`, which esp-decoder never passes.

---

Learnt from: Jason2866
Repo: Jason2866/ESP-Decoder PR: 0
File: :0-0
Timestamp: 2026-04-21T10:36:15.736Z
Learning: In Jason2866/ESP-Decoder (src/webviewPanel.ts), colon-separated ANSI SGR subparameters (e.g., `\033[38:5:196m`) are intentionally not handled. ESP serial output exclusively uses semicolons as SGR separators, so colon-syntax support is out of scope.