Skip to content

Commit 47412e5

Browse files
Refactor buffer processing for clarity and add edge case tests
- Extract storeLine() and accumulate() helper closures to eliminate duplicated line processing and truncation logic - Simplify main loop by using early return pattern (newlineIdx < 0 -> break) - Add test for empty response body edge case - Add test for exact maxLineSize boundary condition (10MB) The refactored code reduces nesting and makes the flow clearer: accumulate handles byte collection with truncation detection, storeLine handles ring buffer storage with truncation markers.
1 parent 4bb1ba9 commit 47412e5

File tree

2 files changed

+75
-60
lines changed

2 files changed

+75
-60
lines changed

pkg/buffer/buffer.go

Lines changed: 42 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -48,79 +48,61 @@ func ProcessResponseAsRingBufferToEnd(httpResp *http.Response, maxJobLogLines in
4848
var currentLine strings.Builder
4949
lineTruncated := false
5050

51+
// storeLine saves the current line to the ring buffer and resets state
52+
storeLine := func() {
53+
line := currentLine.String()
54+
if lineTruncated && len(line) > maxDisplayLength {
55+
line = line[:maxDisplayLength]
56+
}
57+
if lineTruncated {
58+
line += "... [TRUNCATED]"
59+
}
60+
lines[writeIndex] = line
61+
validLines[writeIndex] = true
62+
totalLines++
63+
writeIndex = (writeIndex + 1) % maxJobLogLines
64+
currentLine.Reset()
65+
lineTruncated = false
66+
}
67+
68+
// accumulate adds bytes to currentLine up to maxLineSize, sets lineTruncated if exceeded
69+
accumulate := func(data []byte) {
70+
if lineTruncated {
71+
return
72+
}
73+
remaining := maxLineSize - currentLine.Len()
74+
if remaining <= 0 {
75+
lineTruncated = true
76+
return
77+
}
78+
if remaining > len(data) {
79+
remaining = len(data)
80+
}
81+
currentLine.Write(data[:remaining])
82+
if currentLine.Len() >= maxLineSize {
83+
lineTruncated = true
84+
}
85+
}
86+
5187
for {
5288
n, err := httpResp.Body.Read(readBuf)
5389
if n > 0 {
5490
chunk := readBuf[:n]
5591
for len(chunk) > 0 {
56-
// Find the next newline in the chunk
5792
newlineIdx := bytes.IndexByte(chunk, '\n')
58-
59-
if newlineIdx >= 0 {
60-
// Found a newline - complete the current line
61-
if !lineTruncated {
62-
remaining := maxLineSize - currentLine.Len()
63-
if remaining > newlineIdx {
64-
remaining = newlineIdx
65-
}
66-
if remaining > 0 {
67-
currentLine.Write(chunk[:remaining])
68-
}
69-
if currentLine.Len() >= maxLineSize {
70-
lineTruncated = true
71-
}
72-
}
73-
74-
// Store the completed line
75-
line := currentLine.String()
76-
if lineTruncated {
77-
// Only keep first maxDisplayLength chars for truncated lines
78-
if len(line) > maxDisplayLength {
79-
line = line[:maxDisplayLength]
80-
}
81-
line += "... [TRUNCATED]"
82-
}
83-
lines[writeIndex] = line
84-
validLines[writeIndex] = true
85-
totalLines++
86-
writeIndex = (writeIndex + 1) % maxJobLogLines
87-
88-
// Reset for next line
89-
currentLine.Reset()
90-
lineTruncated = false
91-
chunk = chunk[newlineIdx+1:]
92-
} else {
93-
// No newline in remaining chunk - accumulate if not truncated
94-
if !lineTruncated {
95-
remaining := maxLineSize - currentLine.Len()
96-
if remaining > len(chunk) {
97-
remaining = len(chunk)
98-
}
99-
if remaining > 0 {
100-
currentLine.Write(chunk[:remaining])
101-
}
102-
if currentLine.Len() >= maxLineSize {
103-
lineTruncated = true
104-
}
105-
}
93+
if newlineIdx < 0 {
94+
accumulate(chunk)
10695
break
10796
}
97+
accumulate(chunk[:newlineIdx])
98+
storeLine()
99+
chunk = chunk[newlineIdx+1:]
108100
}
109101
}
110102

111103
if err == io.EOF {
112-
// Handle final line without newline
113104
if currentLine.Len() > 0 {
114-
line := currentLine.String()
115-
if lineTruncated {
116-
if len(line) > maxDisplayLength {
117-
line = line[:maxDisplayLength]
118-
}
119-
line += "... [TRUNCATED]"
120-
}
121-
lines[writeIndex] = line
122-
validLines[writeIndex] = true
123-
totalLines++
105+
storeLine()
124106
}
125107
break
126108
}

pkg/buffer/buffer_test.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,39 @@ func TestProcessResponseAsRingBufferToEnd(t *testing.T) {
117117
assert.NotContains(t, result, "TRUNCATED")
118118
})
119119

120+
t.Run("empty response body", func(t *testing.T) {
121+
resp := &http.Response{
122+
Body: io.NopCloser(strings.NewReader("")),
123+
}
124+
125+
result, totalLines, respOut, err := ProcessResponseAsRingBufferToEnd(resp, 10)
126+
if respOut != nil && respOut.Body != nil {
127+
defer respOut.Body.Close()
128+
}
129+
require.NoError(t, err)
130+
assert.Equal(t, 0, totalLines)
131+
assert.Equal(t, "", result)
132+
})
133+
134+
t.Run("line at exactly maxLineSize boundary", func(t *testing.T) {
135+
// Create a line at exactly maxLineSize (10MB) - should be truncated
136+
exactLine := strings.Repeat("z", 10*1024*1024)
137+
body := "before\n" + exactLine + "\nafter\n"
138+
resp := &http.Response{
139+
Body: io.NopCloser(strings.NewReader(body)),
140+
}
141+
142+
result, totalLines, respOut, err := ProcessResponseAsRingBufferToEnd(resp, 10)
143+
if respOut != nil && respOut.Body != nil {
144+
defer respOut.Body.Close()
145+
}
146+
require.NoError(t, err)
147+
assert.Equal(t, 3, totalLines)
148+
assert.Contains(t, result, "before")
149+
assert.Contains(t, result, "TRUNCATED")
150+
assert.Contains(t, result, "after")
151+
})
152+
120153
t.Run("ring buffer keeps truncated line when in last N", func(t *testing.T) {
121154
// Long line followed by only 2 more lines, with ring buffer size 5
122155
longLine := strings.Repeat("y", 11*1024*1024)

0 commit comments

Comments
 (0)