Skip to content

Commit f295077

Browse files
committed
test: regression for chunk error identity preservation in default handler
1 parent 07b0d6c commit f295077

2 files changed

Lines changed: 59 additions & 1 deletion

File tree

pkg/handlers/default.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ func (h *defaultHandler) handleNonArchiveContent(
134134
dataOrErr := DataOrErr{}
135135
if err := chunkResult.Error(); err != nil {
136136
h.metrics.incErrors()
137-
dataOrErr.Err = fmt.Errorf("%w: error reading chunk: %w", ErrProcessingWarning, err)
137+
dataOrErr.Err = fmt.Errorf("%w: error reading chunk: %v", ErrProcessingWarning, err)
138138
if writeErr := common.CancellableWrite(ctx, dataOrErrChan, dataOrErr); writeErr != nil {
139139
return fmt.Errorf("%w: error writing to data channel: %v", ErrProcessingFatal, writeErr)
140140
}

pkg/handlers/default_test.go

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
package handlers
22

33
import (
4+
stdctx "context"
5+
"errors"
6+
"io"
47
"os"
58
"strings"
69
"testing"
@@ -117,3 +120,58 @@ func TestHandleFileLineNumbers(t *testing.T) {
117120
assert.Equal(t, int64(11), results[1].LineNumber, "peek data should not be counted")
118121
})
119122
}
123+
124+
// TestDefaultHandler_ChunkErrorPreservesIdentity is a regression test for the
125+
// %v wrap at default.go:137. Before the fix, wrapping the inner chunk-reader
126+
// error with %v dropped its identity from the errors.Is chain, which caused
127+
// isFatal in handleChunksWithError to misclassify real per-chunk timeouts as
128+
// non-fatal warnings. The test asserts both that the outer ErrProcessingWarning
129+
// wrap is preserved (so warning-class consumers still see it) and that the
130+
// inner cause remains inspectable.
131+
func TestDefaultHandler_ChunkErrorPreservesIdentity(t *testing.T) {
132+
cases := []struct {
133+
name string
134+
innerErr error
135+
wantFatal bool
136+
}{
137+
{
138+
name: "context.DeadlineExceeded is fatal",
139+
innerErr: stdctx.DeadlineExceeded,
140+
wantFatal: true,
141+
},
142+
{
143+
name: "context.Canceled is fatal",
144+
innerErr: stdctx.Canceled,
145+
wantFatal: true,
146+
},
147+
{
148+
name: "io.EOF is non-fatal",
149+
innerErr: io.EOF,
150+
wantFatal: false,
151+
},
152+
}
153+
154+
for _, tc := range cases {
155+
t.Run(tc.name, func(t *testing.T) {
156+
chunks := []sources.ChunkResult{sources.NewChunkResultError(tc.innerErr)}
157+
handler := newDefaultHandler(defaultHandlerType, withChunkReader(mockChunkReader(chunks)))
158+
reader, err := newFileReader(context.Background(), strings.NewReader("ignored"))
159+
require.NoError(t, err)
160+
161+
var got []DataOrErr
162+
for dataOrErr := range handler.HandleFile(context.Background(), reader) {
163+
got = append(got, dataOrErr)
164+
}
165+
166+
require.Len(t, got, 1)
167+
require.Error(t, got[0].Err)
168+
169+
assert.True(t, errors.Is(got[0].Err, ErrProcessingWarning),
170+
"outer ErrProcessingWarning wrap should be preserved")
171+
assert.True(t, errors.Is(got[0].Err, tc.innerErr),
172+
"inner cause should be inspectable via errors.Is")
173+
assert.Equal(t, tc.wantFatal, isFatal(got[0].Err),
174+
"isFatal should classify based on the inner cause")
175+
})
176+
}
177+
}

0 commit comments

Comments
 (0)