Skip to content

Commit 83e904d

Browse files
committed
fix(logger): infinite loop on log buffer when item size is max bytes
1 parent eafbe13 commit 83e904d

File tree

2 files changed

+79
-2
lines changed

2 files changed

+79
-2
lines changed

packages/logger/src/logBuffer.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,8 +110,12 @@ export class CircularMap<V> extends Map<string, SizedSet<V>> {
110110

111111
const buffer = this.get(key) || new SizedSet<V>();
112112

113-
if (buffer.currentBytesSize + item.byteSize >= this.#maxBytesSize) {
113+
if (
114+
buffer.currentBytesSize &&
115+
buffer.currentBytesSize + item.byteSize >= this.#maxBytesSize
116+
) {
114117
this.#deleteFromBufferUntilSizeIsLessThanMax(buffer, item);
118+
115119
if (this.#onBufferOverflow) {
116120
this.#onBufferOverflow();
117121
}
@@ -132,7 +136,10 @@ export class CircularMap<V> extends Map<string, SizedSet<V>> {
132136
buffer: SizedSet<V>,
133137
item: SizedItem<V>
134138
) {
135-
while (buffer.currentBytesSize + item.byteSize >= this.#maxBytesSize) {
139+
while (
140+
buffer.size &&
141+
buffer.currentBytesSize + item.byteSize >= this.#maxBytesSize
142+
) {
136143
buffer.shift();
137144
buffer.hasEvictedLog = true;
138145
}

packages/logger/tests/unit/logBufferStructures.test.ts

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,4 +144,74 @@ describe('CircularMap', () => {
144144
options.maxBytesSize
145145
);
146146
});
147+
148+
it('stores item in buffer when item size is equal to maxBytesSize', () => {
149+
// Prepare
150+
const maxBytes = 10;
151+
const onBufferOverflow = vi.fn();
152+
const circularMap = new CircularMap<string>({
153+
maxBytesSize: maxBytes,
154+
onBufferOverflow,
155+
});
156+
const message = 'x'.repeat(maxBytes);
157+
158+
// Act
159+
circularMap.setItem('trace-1', message, 1);
160+
161+
// Assess
162+
const buffer = circularMap.get('trace-1');
163+
164+
expect(buffer).toBeDefined();
165+
expect(buffer?.currentBytesSize).toBe(maxBytes);
166+
expect(buffer?.size).toBe(1);
167+
expect(onBufferOverflow).toHaveBeenCalledTimes(0);
168+
});
169+
170+
it('evicts existing non-empty item from buffer when item size is equal to maxBytesSize', () => {
171+
// Prepare
172+
const maxBytes = 10;
173+
const onBufferOverflow = vi.fn();
174+
const circularMap = new CircularMap<string>({
175+
maxBytesSize: maxBytes,
176+
onBufferOverflow,
177+
});
178+
179+
const message = 'x'.repeat(maxBytes);
180+
181+
// Act
182+
circularMap.setItem('trace-1', message, 1);
183+
circularMap.setItem('trace-1', message, 1);
184+
185+
// Assess
186+
const buffer = circularMap.get('trace-1');
187+
188+
expect(buffer).toBeDefined();
189+
expect(buffer?.currentBytesSize).toBe(maxBytes);
190+
expect(buffer?.size).toBe(1);
191+
expect(onBufferOverflow).toHaveBeenCalledTimes(1);
192+
});
193+
194+
it('should not evict existing empty item from buffer when new item size is equal to maxBytesSize', () => {
195+
// Prepare
196+
const maxBytes = 10;
197+
const onBufferOverflow = vi.fn();
198+
const circularMap = new CircularMap<string>({
199+
maxBytesSize: maxBytes,
200+
onBufferOverflow,
201+
});
202+
203+
const message = 'x'.repeat(maxBytes);
204+
205+
// Act
206+
circularMap.setItem('trace-1', '', 1);
207+
circularMap.setItem('trace-1', message, 1);
208+
209+
// Assess
210+
const buffer = circularMap.get('trace-1');
211+
212+
expect(buffer).toBeDefined();
213+
expect(buffer?.currentBytesSize).toBe(maxBytes);
214+
expect(buffer?.size).toBe(2);
215+
expect(onBufferOverflow).toHaveBeenCalledTimes(0);
216+
});
147217
});

0 commit comments

Comments
 (0)