Merge pull request #639 from peco/fix-external-filter-panic-swallow

don't throw away panics in external filters

filter/external.go

@@ -52,13 +52,16 @@ func (ecf ExternalCmd) String() string {
 }
 
 func (ecf *ExternalCmd) Apply(ctx context.Context, buf []line.Line, out pipeline.ChanOutput) (err error) {
+	var readerPanicErr error
+
 	defer func() {
-		if err := recover(); err != nil {
-			if pdebug.Enabled {
-				pdebug.Printf("err: %s", err)
-			}
+		if r := recover(); r != nil {
+			err = fmt.Errorf("panic in external filter %q: %v", ecf.name, r)
 		}
-	}() // ignore errors
+		if err == nil && readerPanicErr != nil {
+			err = readerPanicErr
+		}
+	}()
 	if pdebug.Enabled {
 		g := pdebug.Marker("ExternalCmd.Apply").BindError(&err)
 		defer g.End()
@@ -111,7 +114,11 @@ func (ecf *ExternalCmd) Apply(ctx context.Context, buf []line.Line, out pipeline
 	wg.Add(1)
 	go func(ctx context.Context, cmdCh chan line.Line, rdr *bufio.Reader) {
 		defer wg.Done()
-		defer func() { recover() }()
+		defer func() {
+			if r := recover(); r != nil {
+				readerPanicErr = fmt.Errorf("panic in external filter %q reader: %v", ecf.name, r)
+			}
+		}()
 		defer close(cmdCh)
 		defer cmd.Wait()
 		for {

filter/external_test.go

@@ -91,6 +91,25 @@ func TestExternalCmd_CancelCleansUpGoroutine(t *testing.T) {
 	t.Errorf("goroutine leak: before=%d, after=%d", before, runtime.NumGoroutine())
 }
 
+func TestExternalCmd_ApplyPanicReturnsError(t *testing.T) {
+	idgen := &testIDGen{}
+
+	lines := []line.Line{
+		line.NewRaw(idgen.Next(), "hello", false, false),
+	}
+
+	ecf := NewExternalCmd("cat", "cat", nil, 0, idgen, false)
+	out := pipeline.ChanOutput(make(chan line.Line, 256))
+
+	// Call Apply with a context that does NOT have the query key set.
+	// This triggers a nil interface type assertion panic at the line:
+	//   query := ctx.Value(queryKey).(string)
+	// The bug: this panic was silently swallowed, returning nil error.
+	err := ecf.Apply(context.Background(), lines, out)
+	require.Error(t, err, "Apply should return an error when an internal panic occurs, not swallow it silently")
+	require.Contains(t, err.Error(), "panic")
+}
+
 func TestExternalCmdFilter_NullSep(t *testing.T) {
 	t.Run("preserves Output with enableSep", func(t *testing.T) {
 		idgen := &testIDGen{}