fix(ui): cap oversized tool output before rendering to prevent renderer OOM
A tool that returns oversized external content (e.g. a fetched web page with full-resolution base64 images inlined) previously flowed through the tool-output rendering path as a single unbounded JS string: it was JSON.parsed, syntax highlighted, and attached to the DOM with no size check at any stage. A large enough single string exceeds V8's Zone allocator and hard-crashes the renderer process with "Zone Allocation failed" (issue #2265); the existing 1200-line highlight limit, 80-line virtualization threshold, and max-height CSS clip are all downstream of the allocation and do not prevent it. Add a bounded size guard (capToolOutputText / TOOL_OUTPUT_MAX_CHARS) and apply it at getToolOutputText, the single funnel every generic tool-output string passes through before JSON.parse / highlighting / DOM work. Output under the cap is unchanged; oversized output keeps a useful head and gets a visible truncation notice instead of crashing the renderer. Signed-off-by: JSup <hanjisang0914@gmail.com>
This commit is contained in:
@@ -33,6 +33,7 @@ import {
|
|||||||
renderTodoOutput,
|
renderTodoOutput,
|
||||||
tryParseJsonOutput,
|
tryParseJsonOutput,
|
||||||
coerceToText,
|
coerceToText,
|
||||||
|
capToolOutputText,
|
||||||
} from '../toolRenderers';
|
} from '../toolRenderers';
|
||||||
import { JsonTreeViewer } from '@/components/ui/JsonTreeViewer';
|
import { JsonTreeViewer } from '@/components/ui/JsonTreeViewer';
|
||||||
import { JsonSummaryView } from './JsonSummaryView';
|
import { JsonSummaryView } from './JsonSummaryView';
|
||||||
@@ -841,11 +842,15 @@ const getToolOutputText = (
|
|||||||
part: ToolPartType,
|
part: ToolPartType,
|
||||||
metadata: Record<string, unknown> | undefined,
|
metadata: Record<string, unknown> | undefined,
|
||||||
): string => {
|
): string => {
|
||||||
|
// Cap oversized payloads before JSON.parse / syntax highlighting / DOM work
|
||||||
|
// so a single huge tool output can't trigger a V8 Zone-allocation OOM that
|
||||||
|
// hard-crashes the renderer (issue #2265).
|
||||||
|
const capped = capToolOutputText(output);
|
||||||
if (part.tool === 'bash') {
|
if (part.tool === 'bash') {
|
||||||
return output;
|
return capped;
|
||||||
}
|
}
|
||||||
|
|
||||||
return formatEditOutput(output, part.tool, metadata);
|
return formatEditOutput(capped, part.tool, metadata);
|
||||||
};
|
};
|
||||||
|
|
||||||
const ToolScrollableTextOutput: React.FC<{
|
const ToolScrollableTextOutput: React.FC<{
|
||||||
|
|||||||
@@ -0,0 +1,67 @@
|
|||||||
|
import { describe, test, expect } from 'bun:test';
|
||||||
|
|
||||||
|
import { capToolOutputText, TOOL_OUTPUT_MAX_CHARS } from './toolRenderers';
|
||||||
|
|
||||||
|
// Regression coverage for issue #2265: the desktop renderer hard-crashes with a
|
||||||
|
// V8 "Zone Allocation failed" OOM when a tool returns oversized external content
|
||||||
|
// (e.g. a fetched Google Slides page with full-resolution base64 images inlined),
|
||||||
|
// because the whole payload previously flowed through JSON.parse / syntax
|
||||||
|
// highlighting / DOM rendering as a single unbounded JS string. capToolOutputText
|
||||||
|
// is the bounded size guard that runs before any of that work.
|
||||||
|
describe('capToolOutputText (issue #2265 renderer OOM guard)', () => {
|
||||||
|
test('exposes a sane positive default cap', () => {
|
||||||
|
expect(typeof TOOL_OUTPUT_MAX_CHARS).toBe('number');
|
||||||
|
expect(TOOL_OUTPUT_MAX_CHARS).toBeGreaterThan(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('returns short output unchanged', () => {
|
||||||
|
const output = 'hello world';
|
||||||
|
expect(capToolOutputText(output)).toBe(output);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('returns output at exactly the cap unchanged', () => {
|
||||||
|
const output = 'a'.repeat(TOOL_OUTPUT_MAX_CHARS);
|
||||||
|
expect(capToolOutputText(output)).toBe(output);
|
||||||
|
expect(capToolOutputText(output).length).toBe(TOOL_OUTPUT_MAX_CHARS);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('caps oversized output and never emits the full string', () => {
|
||||||
|
const oversized = 'x'.repeat(TOOL_OUTPUT_MAX_CHARS + 10_000);
|
||||||
|
const capped = capToolOutputText(oversized);
|
||||||
|
|
||||||
|
// The pathological full-size string must not survive to the renderer.
|
||||||
|
expect(capped.length).toBeLessThan(oversized.length);
|
||||||
|
// Head of the payload is preserved for the user.
|
||||||
|
expect(capped.startsWith('x'.repeat(1000))).toBe(true);
|
||||||
|
// A truncation notice is appended so the truncation is visible.
|
||||||
|
expect(capped).toContain('output truncated');
|
||||||
|
expect(capped).toContain('10000 more characters');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('honors a custom cap', () => {
|
||||||
|
const output = 'abcdefghij'; // 10 chars
|
||||||
|
const capped = capToolOutputText(output, 4);
|
||||||
|
expect(capped.startsWith('abcd')).toBe(true);
|
||||||
|
expect(capped).toContain('output truncated');
|
||||||
|
// Only the first 4 chars of the original body are retained.
|
||||||
|
expect(capped).not.toContain('efghij');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('simulated large webfetch payload is bounded well below original size', () => {
|
||||||
|
// ~6MB single string, matching the 5MB-20MB Zone-allocation trigger range
|
||||||
|
// described in the issue (a Slides page with embedded base64 images).
|
||||||
|
const base64Blob = 'QUJD'.repeat(1_500_000); // 6,000,000 chars
|
||||||
|
const capped = capToolOutputText(base64Blob);
|
||||||
|
|
||||||
|
expect(base64Blob.length).toBeGreaterThan(5_000_000);
|
||||||
|
expect(capped.length).toBeLessThan(TOOL_OUTPUT_MAX_CHARS + 256);
|
||||||
|
expect(capped).toContain('renderer from running out of memory');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('non-string input is returned unchanged (defensive)', () => {
|
||||||
|
// @ts-expect-error verifying runtime robustness against non-string inputs
|
||||||
|
expect(capToolOutputText(undefined)).toBeUndefined();
|
||||||
|
// @ts-expect-error verifying runtime robustness against non-string inputs
|
||||||
|
expect(capToolOutputText(null)).toBeNull();
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -22,6 +22,28 @@ export const coerceToText = (value: unknown, fallback = ''): string => {
|
|||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
|
// Guards the renderer process against V8 "Zone Allocation failed" OOM crashes
|
||||||
|
// (issue #2265). When a tool returns oversized external content — e.g. a fetched
|
||||||
|
// web page with full-resolution base64 images inlined — the entire payload flows
|
||||||
|
// through this module as a single JS string that is JSON.parsed, syntax
|
||||||
|
// highlighted, and attached to the DOM. A large enough single string exceeds
|
||||||
|
// V8's Zone allocator and hard-crashes the renderer before any virtualization or
|
||||||
|
// CSS clip can help. Capping the string length before that work happens keeps a
|
||||||
|
// useful head of the output while preventing the pathological allocation.
|
||||||
|
export const TOOL_OUTPUT_MAX_CHARS = 512 * 1024;
|
||||||
|
|
||||||
|
export const capToolOutputText = (
|
||||||
|
output: string,
|
||||||
|
maxChars: number = TOOL_OUTPUT_MAX_CHARS,
|
||||||
|
): string => {
|
||||||
|
if (typeof output !== 'string' || output.length <= maxChars) {
|
||||||
|
return output;
|
||||||
|
}
|
||||||
|
const omitted = output.length - maxChars;
|
||||||
|
const notice = `\n\n… [output truncated: ${omitted} more characters not shown to prevent the renderer from running out of memory]`;
|
||||||
|
return output.slice(0, maxChars) + notice;
|
||||||
|
};
|
||||||
|
|
||||||
const hasLspDiagnostics = (output: string): boolean => {
|
const hasLspDiagnostics = (output: string): boolean => {
|
||||||
if (!output) return false;
|
if (!output) return false;
|
||||||
return output.includes('<diagnostics')
|
return output.includes('<diagnostics')
|
||||||
|
|||||||
Reference in New Issue
Block a user