🐛 fix: Prevent Infinite Render Loop on Code-Execution File Preview (#13922)

* 🐛 fix: Prevent Infinite Render Loop on Code-Execution File Preview

Loading a conversation that contains a large (>1MB) code-execution
office file crashed the whole app with React error #185 ("Maximum
update depth exceeded") on hard refresh.

Root cause (client-only): the terminal-write effect in
useAttachmentPreviewSync writes the resolved preview record back into
messageAttachmentsMap with a fresh object identity on every run, and
`attachment` is in the effect's dependency array. useAttachments
re-derives `attachment` ({...db, ...liveEntry}) with a new identity on
every map write, so once polling resolves (pending -> ready on a loaded
conversation) the effect ping-pongs forever:
setAttachmentsMap -> re-derive -> effect -> setAttachmentsMap.

Only files large/slow enough to defer extraction are persisted at
status: 'pending', which is why small documents never triggered it.

Fix: an idempotency gate that bails before setAttachmentsMap when the
merged attachment already carries the resolved status/text/textFormat/
previewError. The write happens once and then settles.

Tests:
- useAttachmentPreviewSync.loop.spec.tsx wires the real
  useAttachments -> hook feedback to reproduce the loop (verified to
  throw #185 without the gate, settle with it).
- e2e/specs/mock/attachment-preview-loop.spec.ts loads a conversation
  with a pending code-exec attachment whose preview resolves ready and
  asserts the app does not crash.

Closes #13916

* 🔧 feat: Make Office Preview Extraction Cap Configurable (default 2MB)

The inline code-execution preview extraction ceiling was a hardcoded 1MB
constant (MAX_TEXT_EXTRACT_BYTES). Office/text artifacts over that skip
the inline preview and resolve to "Preview unavailable" (download-only).

Make it configurable via FILE_PREVIEW_MAX_EXTRACT_BYTES and raise the
default to 2MB so larger documents get an inline preview out of the box.
The rendered HTML remains independently capped at MAX_TEXT_CACHE_BYTES
(512KB), so image-heavy files over that still fall back to the existing
"preview too large" banner rather than rendering unbounded output.

- resolveMaxTextExtractBytes(env) parses the override, falling back to
  2MB on missing/non-numeric/non-positive values (warns on invalid).
- Documented in .env.example next to the other file-size limits.
- Unit tests cover default, valid override, fractional flooring, and
  invalid fallback.

* 🐛 fix: Guard sub-byte preview cap from flooring to zero

A fractional FILE_PREVIEW_MAX_EXTRACT_BYTES in (0, 1) passed the
positive-number check then floored to 0, making MAX_TEXT_EXTRACT_BYTES
zero and treating every non-empty artifact as oversized. Floor first,
then require the result to be >= 1 byte before accepting it; otherwise
fall back to the 2 MB default. Adds coverage for the sub-byte case.

*  test: Make exported-ceiling assertion env-independent

The "exported ceiling" assertion compared MAX_TEXT_EXTRACT_BYTES to a
literal 2 MB, but that const is initialized from
FILE_PREVIEW_MAX_EXTRACT_BYTES at module load — so the suite would
falsely fail when run with the override set. Assert the export tracks
resolveMaxTextExtractBytes(env) for the current environment instead; the
undefined-case test continues to pin the 2 MB default.
This commit is contained in:
Danny Avila 2026-06-23 16:34:43 -04:00 committed by GitHub
parent f616a58fb7
commit 562bd8ec5f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 433 additions and 1 deletions

View file

@ -880,6 +880,11 @@ HELP_AND_FAQ_URL=https://librechat.ai
# such as the below example of 250 mib
# CONVERSATION_IMPORT_MAX_FILE_SIZE_BYTES=262144000
# Max size (bytes) of a code-execution artifact (docx/xlsx/csv/pptx/text/pdf) rendered as an
# inline preview. Larger files fall back to download-only. Default: 2 MB (2097152). Note the
# rendered HTML is independently capped at 512 KB, so very rich files may still skip preview.
# FILE_PREVIEW_MAX_EXTRACT_BYTES=2097152
#===============#
# REDIS Options #

View file

@ -0,0 +1,166 @@
/**
* Regression for issue #13916: hard-refreshing a conversation that
* contains a large (>1MB) code-execution office file crashed the app
* with React error #185 ("Maximum update depth exceeded").
*
* The loop only manifests with the REAL consumer wiring, which the
* focused unit spec (sibling file) can't reproduce because it passes a
* fixed `attachment` prop:
*
* ContentRender useAttachments (re-derives `attachment` with a fresh
* identity on every messageAttachmentsMap write) FileAttachment
* useAttachmentPreviewSync (writes the resolved record back into
* messageAttachmentsMap; `attachment` is in the effect's deps).
*
* On a loaded conversation the message's attachment is frozen at the
* immediate-persist snapshot `status: 'pending'`; polling resolves it to
* `ready`, the effect writes it into the map, useAttachments re-derives a
* new attachment identity, the effect re-fires, and so on without ever
* reaching a fixed point. This test reproduces that exact feedback path
* and asserts it settles instead of looping.
*/
import { RecoilRoot } from 'recoil';
import { Tools } from 'librechat-data-provider';
import { render } from '@testing-library/react';
import type {
TFile,
TAttachment,
TFilePreview,
TAttachmentMetadata,
} from 'librechat-data-provider';
const mockUseFilePreview = jest.fn();
jest.mock('~/data-provider', () => ({
useFilePreview: (...args: unknown[]) => mockUseFilePreview(...args),
}));
jest.mock('~/hooks/useLocalize', () => () => (key: string) => key);
import useAttachments from '~/hooks/Messages/useAttachments';
import useAttachmentPreviewSync from '../useAttachmentPreviewSync';
const messageId = 'msg-1';
const fileId = 'fid-1';
/** DB-frozen, immediate-persist snapshot: status pending, no text yet. */
const dbAttachment = {
user: 'user-1',
object: 'file',
bytes: 1256732,
embedded: false,
usage: 0,
file_id: fileId,
filename: 'repro-13916-large.docx',
filepath: '/uploads/repro-13916-large.docx',
type: Tools.execute_code,
messageId,
toolCallId: 'tc-1',
text: undefined,
textFormat: undefined,
status: 'pending',
} as TFile & TAttachmentMetadata as TAttachment;
/* Stable array identity so the only identity churn during the test comes
* from messageAttachmentsMap writes, not from re-creating this prop. */
const dbAttachments: TAttachment[] = [dbAttachment];
let renderCount = 0;
function PreviewBridge({ attachment }: { attachment: TAttachment }) {
renderCount += 1;
useAttachmentPreviewSync(attachment);
return null;
}
/** Mirrors ContentRender: the rendered attachment is the merged value
* produced by useAttachments, re-derived from the live map on each write. */
function Consumer() {
const { attachments } = useAttachments({ messageId, attachments: dbAttachments });
const attachment = attachments[0];
if (!attachment) {
return null;
}
return <PreviewBridge attachment={attachment} />;
}
describe('useAttachmentPreviewSync — #13916 infinite-loop regression', () => {
beforeEach(() => {
renderCount = 0;
mockUseFilePreview.mockReset();
});
it('settles (no React #185) when a pending code-exec preview resolves to ready on load', () => {
mockUseFilePreview.mockReturnValue({
data: {
file_id: fileId,
status: 'ready',
text: '<p>edited paragraph added</p>',
textFormat: 'html',
} as TFilePreview,
isFetching: false,
});
expect(() =>
render(
<RecoilRoot>
<Consumer />
</RecoilRoot>,
),
).not.toThrow();
/* A correct fixed point reaches the resolved state in a handful of
* renders. A regressed write-back would blow past React's nested
* update ceiling (50) and throw long before any sane bound. */
expect(renderCount).toBeLessThan(10);
});
it('still merges the resolved record into the map exactly once', () => {
let latestMap: Record<string, TAttachment[] | undefined> = {};
mockUseFilePreview.mockReturnValue({
data: {
file_id: fileId,
status: 'ready',
text: '<p>final</p>',
textFormat: 'html',
} as TFilePreview,
isFetching: false,
});
function MapProbe() {
const { attachments } = useAttachments({ messageId, attachments: dbAttachments });
latestMap = { [messageId]: attachments };
const attachment = attachments[0];
return attachment ? <PreviewBridge attachment={attachment} /> : null;
}
render(
<RecoilRoot>
<MapProbe />
</RecoilRoot>,
);
const resolved = latestMap[messageId]?.[0] as TFile & TAttachmentMetadata;
expect(resolved.status).toBe('ready');
expect(resolved.text).toBe('<p>final</p>');
expect(resolved.textFormat).toBe('html');
});
it('settles for a failed resolution too', () => {
mockUseFilePreview.mockReturnValue({
data: {
file_id: fileId,
status: 'failed',
previewError: 'render-timeout',
} as TFilePreview,
isFetching: false,
});
expect(() =>
render(
<RecoilRoot>
<Consumer />
</RecoilRoot>,
),
).not.toThrow();
expect(renderCount).toBeLessThan(10);
});
});

View file

@ -163,6 +163,25 @@ export default function useAttachmentPreviewSync(
if (!polled || polled.status === 'pending' || !messageId || !fileId || !attachment) {
return;
}
/* Idempotency gate. `useAttachments` re-derives `attachment` with a
* fresh object identity on every messageAttachmentsMap write, and
* `attachment` is in this effect's deps so without a gate the
* write-back ping-pongs forever (setAttachmentsMap re-derive
* effect setAttachmentsMap ), tripping React's "Maximum update
* depth exceeded" (#185) once the preview resolves on a loaded
* conversation. Once the merged attachment already carries the
* resolved fields, there's nothing to write stop. */
const current = attachment as Partial<TFile> & TAttachment;
const nextText = polled.text ?? current.text ?? null;
const nextTextFormat = polled.textFormat ?? current.textFormat ?? null;
if (
current.status === polled.status &&
current.text === nextText &&
current.textFormat === nextTextFormat &&
current.previewError === polled.previewError
) {
return;
}
setAttachmentsMap((prevMap) => {
const messageAttachments =
(prevMap as Record<string, TAttachment[] | undefined>)[messageId] || [];

View file

@ -0,0 +1,169 @@
import { expect, test } from '@playwright/test';
import type { Route } from '@playwright/test';
/**
* Regression for issue #13916: hard-refreshing a conversation that
* contains a large (>1MB) code-execution office file crashed the whole
* app with React error #185 ("Maximum update depth exceeded").
*
* Root cause (client-only): on a loaded conversation the message's
* attachment is frozen at the immediate-persist snapshot
* `status: 'pending'`. `useAttachmentPreviewSync` polls the preview
* endpoint, and once it resolves it writes the record back into
* `messageAttachmentsMap`. `useAttachments` re-derives the attachment
* with a fresh identity on every such write, and that attachment is in
* the effect's dependency array so the write-back ping-ponged forever.
*
* This spec reconstructs that exact load: it intercepts the
* conversation, messages, and preview endpoints to serve a code-exec
* tool call carrying a still-`pending` office attachment whose preview
* resolves to `ready`. With the bug present the page throws #185 and the
* route's error boundary replaces the chat; with the fix it settles and
* the conversation renders normally.
*
* It uses network interception (not the fake model) because the trigger
* is the persisted deferred-preview lifecycle, which the mock LLM does
* not produce.
*/
const NO_PARENT = '00000000-0000-0000-0000-000000000000';
const unique = (p: string) => `${p}-${Date.now()}-${Math.floor(Math.random() * 1e6)}`;
const escapeRe = (v: string) => v.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
test.describe('issue #13916 — code-exec attachment preview', () => {
test('loading a conversation with a pending >1MB doc attachment does not crash (React #185)', async ({
page,
}) => {
test.setTimeout(120000);
const conversationId = unique('e2e-13916');
const messageId = `${conversationId}-msg`;
const fileId = `${conversationId}-file`;
const filename = 'repro-13916-large.docx';
const now = new Date(0).toISOString();
/** DB-frozen, immediate-persist snapshot: pending, no resolved text. */
const attachment = {
file_id: fileId,
filename,
filepath: `/uploads/682f49b90f07376815c38ef2/${fileId}__${filename}`,
type: 'execute_code',
source: 'local',
bytes: 1256732,
messageId,
conversationId,
toolCallId: 'tc-13916',
status: 'pending',
metadata: {
codeEnvRef: { kind: 'user', id: '682f49b90f07376815c38ef2', storage_session_id: 'sess' },
},
};
const message = {
messageId,
conversationId,
parentMessageId: NO_PARENT,
isCreatedByUser: false,
sender: 'Assistant',
endpoint: 'Mock Provider A',
model: 'mock-model-a',
text: '',
content: [
{
type: 'tool_call',
tool_call: {
id: 'tc-13916',
name: 'execute_code',
args: '{"lang":"py","code":"# edit the document"}',
output: 'edited 1 paragraph',
progress: 1,
},
},
],
attachments: [attachment],
createdAt: now,
updatedAt: now,
};
const conversation = {
conversationId,
title: 'File Editing Request',
endpoint: 'Mock Provider A',
endpointType: 'custom',
model: 'mock-model-a',
createdAt: now,
updatedAt: now,
};
/* The preview poll resolves immediately to `ready`; this terminal
* response is the edge that kicks off the (formerly infinite) write-back. */
const previewReady = {
file_id: fileId,
status: 'ready',
text: '<p>edited paragraph added</p>',
textFormat: 'html',
};
const convoIdRe = escapeRe(conversationId);
await page.route(new RegExp(`/api/convos/${convoIdRe}(?:\\?.*)?$`), (route: Route) =>
route.fulfill({
status: 200,
contentType: 'application/json',
body: JSON.stringify(conversation),
}),
);
await page.route(new RegExp(`/api/messages/${convoIdRe}(?:\\?.*)?$`), (route: Route) =>
route.fulfill({
status: 200,
contentType: 'application/json',
body: JSON.stringify([message]),
}),
);
await page.route(/\/api\/files\/[^/]+\/preview(?:\?.*)?$/, (route: Route) =>
route.fulfill({
status: 200,
contentType: 'application/json',
body: JSON.stringify(previewReady),
}),
);
/* The exact failure signature from the issue. Capture both the
* uncaught throw and the console error React logs alongside it. */
const maxDepth = /Maximum update depth exceeded/i;
const fatalErrors: string[] = [];
page.on('pageerror', (err) => {
if (maxDepth.test(err.message)) {
fatalErrors.push(err.message);
}
});
page.on('console', (msg) => {
if (msg.type() === 'error' && maxDepth.test(msg.text())) {
fatalErrors.push(msg.text());
}
});
await page.goto(`/c/${conversationId}`, { timeout: 30000 });
/* Anti-false-pass: the attachment path must actually execute, else
* the buggy build would never loop and this test would guard nothing.
* The filename renders only once the code-exec attachment mounts. */
await expect(page.getByText(new RegExp(escapeRe('repro-13916'))).first()).toBeVisible({
timeout: 30000,
});
/* Let the preview poll + any re-render storm play out. */
await page.waitForTimeout(3000);
expect(
fatalErrors,
`React #185 fired during conversation load:\n${fatalErrors.join('\n---\n')}`,
).toHaveLength(0);
/* The route's error boundary replaces the composer on crash, so its
* presence is an independent "the app survived" signal. */
await expect(page.getByRole('textbox', { name: 'Message input' })).toBeVisible({
timeout: 10000,
});
});
});

View file

@ -3,6 +3,7 @@ import * as path from 'path';
import {
extractCodeArtifactText,
getExtractedTextFormat,
resolveMaxTextExtractBytes,
MAX_TEXT_CACHE_BYTES,
MAX_TEXT_EXTRACT_BYTES,
} from './extract';
@ -677,3 +678,43 @@ describe('extractCodeArtifactText office-html concurrency', () => {
expect(htmlCount).toBe(4);
});
});
describe('resolveMaxTextExtractBytes', () => {
const TWO_MB = 2 * 1024 * 1024;
it('defaults to 2 MB when unset or blank', () => {
expect(resolveMaxTextExtractBytes(undefined)).toBe(TWO_MB);
expect(resolveMaxTextExtractBytes('')).toBe(TWO_MB);
expect(resolveMaxTextExtractBytes(' ')).toBe(TWO_MB);
});
it('honors a valid positive byte override', () => {
expect(resolveMaxTextExtractBytes('1048576')).toBe(1048576);
expect(resolveMaxTextExtractBytes('5242880')).toBe(5242880);
});
it('floors fractional values', () => {
expect(resolveMaxTextExtractBytes('1048576.9')).toBe(1048576);
});
it('falls back to the default on non-numeric or non-positive input', () => {
expect(resolveMaxTextExtractBytes('nope')).toBe(TWO_MB);
expect(resolveMaxTextExtractBytes('0')).toBe(TWO_MB);
expect(resolveMaxTextExtractBytes('-100')).toBe(TWO_MB);
});
it('falls back to the default for sub-byte values that floor to zero', () => {
expect(resolveMaxTextExtractBytes('0.5')).toBe(TWO_MB);
expect(resolveMaxTextExtractBytes('0.999')).toBe(TWO_MB);
});
it('wires the exported ceiling through the resolver for the current env', () => {
/* Asserting a literal 2 MB here would falsely fail when the suite runs
* with FILE_PREVIEW_MAX_EXTRACT_BYTES set (the export is initialized
* from the env at module load). Verify the export tracks the resolver
* instead; the `undefined` case above pins the 2 MB default. */
expect(MAX_TEXT_EXTRACT_BYTES).toBe(
resolveMaxTextExtractBytes(process.env.FILE_PREVIEW_MAX_EXTRACT_BYTES),
);
});
});

View file

@ -10,7 +10,39 @@ import { parseDocument } from '~/files/documents/crud';
import { isBinaryBuffer } from '~/skills/binary';
export const MAX_TEXT_CACHE_BYTES: number = 512 * 1024;
export const MAX_TEXT_EXTRACT_BYTES: number = 1024 * 1024;
/** Default inline-preview extraction ceiling: 2 MB. Office/text artifacts
* larger than this skip inline preview and fall back to download-only. */
const DEFAULT_MAX_TEXT_EXTRACT_BYTES = 2 * 1024 * 1024;
/**
* Resolve the inline-preview extraction ceiling from
* `FILE_PREVIEW_MAX_EXTRACT_BYTES`, falling back to the 2 MB default when
* the value is missing, non-numeric, or non-positive. Raising it lets
* larger documents render an inline preview; the rendered HTML is still
* independently capped at {@link MAX_TEXT_CACHE_BYTES} (512 KB), so
* image-heavy files over that show the "too large" banner instead.
*/
export function resolveMaxTextExtractBytes(value: string | undefined): number {
if (value == null || value.trim() === '') {
return DEFAULT_MAX_TEXT_EXTRACT_BYTES;
}
/* Floor first, then validate: a fractional value in (0, 1) passes a
* `> 0` check but floors to 0, which would treat every non-empty
* artifact as oversized fall back to the default instead. */
const floored = Math.floor(Number(value));
if (!Number.isFinite(floored) || floored < 1) {
logger.warn(
`[extract] Invalid FILE_PREVIEW_MAX_EXTRACT_BYTES "${value}"; using ${DEFAULT_MAX_TEXT_EXTRACT_BYTES} bytes.`,
);
return DEFAULT_MAX_TEXT_EXTRACT_BYTES;
}
return floored;
}
export const MAX_TEXT_EXTRACT_BYTES: number = resolveMaxTextExtractBytes(
process.env.FILE_PREVIEW_MAX_EXTRACT_BYTES,
);
const DOCUMENT_PARSE_TIMEOUT_MS = 8_000;
const OFFICE_HTML_TIMEOUT_MS = 12_000;
const TRUNCATION_MARKER = '\n\n…[truncated]';