Skip to content

fix(completion): recover from panics in the application completer#116

Merged
maxlandon merged 1 commit into
masterfrom
dev
Jul 5, 2026
Merged

fix(completion): recover from panics in the application completer#116
maxlandon merged 1 commit into
masterfrom
dev

Conversation

@maxlandon

Copy link
Copy Markdown
Member

Problem

readline calls the application-provided Completer on nearly every keystroke — both explicit menu completion and as-you-type autocomplete funnel through commandCompletion. If that user code panics (a bad completer, or transient command-tree state), the panic propagated out of the completion path and crashed the whole interactive shell.

Fix

Recover around the completer invocation in commandCompletion and surface the panic value as a completion message, so a single faulty completion degrades into a visible completion error: … instead of taking down the process. Both completion paths share this one adapter, so the single choke point covers them all.

func (rl *Shell) commandCompletion() (values completion.Values) {
    ...
    defer func() {
        if r := recover(); r != nil {
            msg := CompleteMessage("completion error: %v", r)
            values = msg.convert()
        }
    }()
    ...
}

Why here and not downstream

This is the readline-side fix for reeflective/console#81, which worked around the crash by recovering inside console's own completer. Fixing it in readline protects every caller — no downstream project needs to wrap its completer in a recover(). Once this lands, console #81 can be dropped (or trimmed to just its cobra command-tree reset, which is console-specific).

Testing

  • New TestCommandCompletionRecoversFromPanic (panicking completer → recovered, message surfaced) and TestCommandCompletionNilCompleter (clean no-op).
  • go build, go vet, and go test ./... all pass.

readline calls the application-provided Completer on nearly every keystroke,
for both menu completion and as-you-type autocomplete (both funnel through
commandCompletion). A panic in that user code — a bad completer or transient
command-tree state — propagated out of the completion path and crashed the
whole interactive shell.

Recover around the completer invocation and surface the panic value as a
completion message instead, so a single faulty completion degrades into a
visible error rather than taking down the process. Because both completion
paths share commandCompletion, this one choke point covers them all.

This is the readline-side fix for reeflective/console#81, which worked around
the crash by recovering in its own completer; downstream callers no longer
need to guard their completers against panics.
@codecov

codecov Bot commented Jul 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 39.33%. Comparing base (be2926f) to head (7ee6a10).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #116      +/-   ##
==========================================
+ Coverage   39.04%   39.33%   +0.28%     
==========================================
  Files          60       60              
  Lines        9606     9610       +4     
==========================================
+ Hits         3751     3780      +29     
+ Misses       5767     5742      -25     
  Partials       88       88              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@maxlandon maxlandon merged commit 19f6714 into master Jul 5, 2026
11 checks passed
maxlandon added a commit to reeflective/console that referenced this pull request Jul 5, 2026
Consumes the readline-side fixes and features from v1.3.0, notably the
completer panic recovery (reeflective/readline#116): readline now recovers
from a panicking application completer at its own boundary, so console's
completer no longer needs to guard against it.

Also picks up the right-edge redraw fix, the history out-of-range fix, and the
new paste-transformer and inline-suggestion APIs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant