fix(bootstrap): keep the whole first-run log, render only the tail
MAX_LOG_LINES = 200 capped the `logs` state itself, so on a cold install every line past the 200th destroyed an earlier one. Four consumers read that array and all four degraded once a real install ran past the cap: - the Activity heading renders logs.length, so the counter sat pinned at 200 for the rest of a multi-minute bootstrap while lines kept streaming. Live progress read as stalled when it was not. This is what #1847 reported. - handleCopyLogs serializes the same array, so Copy could only ever return the newest 200 lines -- and this splash is the only place in the app with a copy-log affordance at all. - detectHints(message, logs) scans for actionable failure markers. A failure early in a long install lost its marker, so the card fell back to hint_default and told the user nothing specific. This is the severe one: the screen still looks helpful while saying nothing. - isUnrecoverableFailure(message, logs) runs off the same scan. Only the <pre> needed the cap -- it is a DOM budget, not a retention policy. Keep the run in state, slice at the one place that writes DOM, and rename the constant to VISIBLE_LOG_LINES so the next reader cannot make the same mistake. The array is bounded by one bootstrap: both retry paths clear it and App.jsx unmounts the splash when the stage flips to 'ready'. The two full-array scans are behind `isFailed`, so they never run while lines are streaming. Not fixed here: the other half of #1847, that the splash vanishes on success with no completion state and the log is then unrecoverable. That needs either a lifecycle change in App.jsx or a Rust-side persisted stream, and the report frames the two as separable. Refs #1847 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Chang-Jin-Lee <ckdwls525@gmail.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
9790d28922
commit
03c1396b14
@@ -50,6 +50,8 @@ the frozen-backend fallback mirror it for their toolchains.
|
||||
|
||||
### Fixed
|
||||
|
||||
- The first-run Activity log counts every line instead of freezing at 200 while the install is still running, and Copy now hands back the whole run rather than the last 200 lines (#1847) — thanks @psiberfunk!
|
||||
- A first-run failure that happened early in a long install keeps its specific advice, instead of falling back to the generic retry hint once the log scrolled past 200 lines (#1847) — thanks @psiberfunk!
|
||||
- Install documentation help now prints correctly on Windows consoles using legacy encodings (#1815) — thanks @dajiaohuang!
|
||||
- Saved transcriptions with missing or invalid timestamps now remain readable (#1799) — thanks @yunaremaia and @tvbht!
|
||||
- Copying a saved transcription now uses the shared clipboard helper and reports failed copies accurately (#1803) — thanks @tvbht!
|
||||
|
||||
@@ -120,7 +120,13 @@ const STEPS = [
|
||||
'starting_backend',
|
||||
];
|
||||
|
||||
const MAX_LOG_LINES = 200;
|
||||
// How many log lines the <pre> renders. The full run is kept in state (#1847):
|
||||
// the counter, the Copy button, the actionable failure hints and the
|
||||
// unrecoverable-retry gate all read every line, and only this <pre> is capped —
|
||||
// it is a DOM cost, not a retention policy. The array lives for exactly one
|
||||
// bootstrap: both retry paths clear it, and App.jsx unmounts the splash the
|
||||
// moment the stage flips to 'ready'.
|
||||
const VISIBLE_LOG_LINES = 200;
|
||||
|
||||
/** Scan logs + error message for known failure patterns and return i18n keys
|
||||
* for actionable hints (resolved with `t(...)` at render — English defaults
|
||||
@@ -549,8 +555,7 @@ export function BootstrapSplash({ stage, message }) {
|
||||
// Deduplicate against backfill by checking the last few lines.
|
||||
const lastFew = prev.slice(-5);
|
||||
if (lastFew.some((l) => l.stage === s && l.line === line)) return prev;
|
||||
const next = prev.concat([{ stage: s, line, t: Date.now() }]);
|
||||
return next.length > MAX_LOG_LINES ? next.slice(next.length - MAX_LOG_LINES) : next;
|
||||
return prev.concat([{ stage: s, line, t: Date.now() }]);
|
||||
});
|
||||
});
|
||||
unlistenProgress = await listen('bootstrap-progress', (e) => {
|
||||
@@ -591,6 +596,9 @@ export function BootstrapSplash({ stage, message }) {
|
||||
if (isFailed) setLogsOpen(true);
|
||||
}, [isFailed]);
|
||||
|
||||
// Serializes the WHOLE run, not the visible tail (#1847). A user filing a
|
||||
// bootstrap bug is asked for this output, and the early lines — which stage
|
||||
// failed first, which mirror was reached — are the ones that scrolled out.
|
||||
const handleCopyLogs = () => {
|
||||
const logText =
|
||||
logs.length === 0
|
||||
@@ -863,7 +871,10 @@ export function BootstrapSplash({ stage, message }) {
|
||||
>
|
||||
{logs.length === 0
|
||||
? t('bootstrap.waiting_output', 'Waiting for output…')
|
||||
: logs.map((l) => `[${l.stage}] ${l.line}`).join('\n')}
|
||||
: logs
|
||||
.slice(-VISIBLE_LOG_LINES)
|
||||
.map((l) => `[${l.stage}] ${l.line}`)
|
||||
.join('\n')}
|
||||
</pre>
|
||||
)}
|
||||
</section>
|
||||
|
||||
@@ -0,0 +1,140 @@
|
||||
/**
|
||||
* The bootstrap log window is a DOM budget, not a retention policy (#1847).
|
||||
*
|
||||
* `MAX_LOG_LINES = 200` capped the `logs` state itself, so every line past the
|
||||
* 200th silently destroyed the earlier ones. Four things read that array, and
|
||||
* all four degraded once a real cold install went past the cap:
|
||||
*
|
||||
* - the Activity heading rendered `logs.length`, so the counter sat pinned at
|
||||
* exactly 200 for the rest of a multi-minute bootstrap while lines were
|
||||
* still streaming underneath. Live progress read as stalled when it was not.
|
||||
* - `handleCopyLogs` serialized the same array, so Copy could only ever hand
|
||||
* back the newest 200 lines — and this splash is the only place in the app
|
||||
* with a Copy-log affordance at all.
|
||||
* - `detectHints(message, logs)` scans for actionable failure markers. A
|
||||
* failure that happened early in a long install lost its marker, so the
|
||||
* card fell back to `hint_default` and the user was told nothing useful.
|
||||
* - `isUnrecoverableFailure(message, logs)` runs off the same scan.
|
||||
*
|
||||
* The fix keeps the whole run in state and slices only where the DOM is
|
||||
* written. The array is bounded by one bootstrap: both retry paths clear it,
|
||||
* and App.jsx unmounts the splash the moment the stage flips to 'ready'.
|
||||
*/
|
||||
import React from 'react';
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
import { act, render, screen, waitFor } from '@testing-library/react';
|
||||
import { BootstrapSplash } from '../components/BootstrapSplash';
|
||||
|
||||
const invoke = vi.fn();
|
||||
vi.mock('@tauri-apps/api/core', () => ({ invoke: (...a) => invoke(...a) }));
|
||||
|
||||
let logHandler = null;
|
||||
vi.mock('@tauri-apps/api/event', () => ({
|
||||
listen: vi.fn(async (name, cb) => {
|
||||
if (name === 'bootstrap-log') logHandler = cb;
|
||||
return () => {};
|
||||
}),
|
||||
}));
|
||||
vi.mock('@tauri-apps/plugin-opener', () => ({ revealItemInDir: vi.fn() }));
|
||||
|
||||
const copied = [];
|
||||
vi.mock('../utils/copyText', () => ({
|
||||
copyText: (text) => {
|
||||
copied.push(text);
|
||||
return Promise.resolve();
|
||||
},
|
||||
}));
|
||||
|
||||
/**
|
||||
* Stream `n` lines through the live `bootstrap-log` listener, the first
|
||||
* carrying a marker that has to survive.
|
||||
*
|
||||
* Deliberately NOT the `get_bootstrap_logs` backfill: that path replaced the
|
||||
* array wholesale and never sliced, so it was already unaffected by the cap.
|
||||
* Only the live append trimmed, which is why the bug needed a long *running*
|
||||
* install to show up rather than a long buffered one.
|
||||
*/
|
||||
const stream = async (n, firstLine) => {
|
||||
await act(async () => {
|
||||
for (let i = 0; i < n; i += 1) {
|
||||
logHandler({
|
||||
payload: {
|
||||
stage: 'installing_deps',
|
||||
line: i === 0 ? firstLine : `dependency step ${i}`,
|
||||
},
|
||||
});
|
||||
}
|
||||
});
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
logHandler = null;
|
||||
copied.length = 0;
|
||||
invoke.mockReset();
|
||||
invoke.mockResolvedValue([]);
|
||||
window.__TAURI_INTERNALS__ = {};
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
delete window.__TAURI_INTERNALS__;
|
||||
});
|
||||
|
||||
describe('a bootstrap longer than the visible window', () => {
|
||||
it('counts every line instead of freezing at the window size', async () => {
|
||||
render(<BootstrapSplash stage="installing_deps" />);
|
||||
await waitFor(() => expect(logHandler).toBeTypeOf('function'));
|
||||
await stream(640, 'starting install');
|
||||
|
||||
// Pre-fix this read "200 lines" and stayed there for the rest of the run,
|
||||
// which is the whole complaint: the heading is the only live signal that
|
||||
// a multi-minute install is still moving.
|
||||
await screen.findByText('640 lines');
|
||||
});
|
||||
|
||||
it('still writes only the window to the DOM', async () => {
|
||||
const { container } = render(<BootstrapSplash stage="installing_deps" />);
|
||||
await waitFor(() => expect(logHandler).toBeTypeOf('function'));
|
||||
await stream(640, 'starting install');
|
||||
await screen.findByText('640 lines');
|
||||
|
||||
const pre = container.querySelector('pre');
|
||||
const rendered = pre.textContent.split('\n');
|
||||
// Retention grew; the DOM budget did not.
|
||||
expect(rendered).toHaveLength(200);
|
||||
expect(rendered.at(-1)).toContain('dependency step 639');
|
||||
expect(pre.textContent).not.toContain('starting install');
|
||||
});
|
||||
|
||||
it('copies the whole run, including what scrolled out of the window', async () => {
|
||||
render(<BootstrapSplash stage="installing_deps" />);
|
||||
await waitFor(() => expect(logHandler).toBeTypeOf('function'));
|
||||
await stream(640, 'starting install');
|
||||
await screen.findByText('640 lines');
|
||||
|
||||
screen.getByRole('button', { name: /Copy/ }).click();
|
||||
await waitFor(() => expect(copied).toHaveLength(1));
|
||||
const text = copied[0];
|
||||
// The first line is the one a bug report needs and the one the cap ate.
|
||||
expect(text).toContain('starting install');
|
||||
expect(text).toContain('dependency step 639');
|
||||
expect(text.split('\n')).toHaveLength(640);
|
||||
});
|
||||
|
||||
it('keeps the actionable hint for a failure that happened early', async () => {
|
||||
// The severe case: `uv sync failed` scrolls out, `detectHints` finds
|
||||
// nothing, and the failure card offers the generic hint instead of the
|
||||
// one that names the actual problem.
|
||||
render(<BootstrapSplash stage="failed" message="Setup failed" />);
|
||||
await waitFor(() => expect(logHandler).toBeTypeOf('function'));
|
||||
await stream(640, 'uv sync failed: exit code 1');
|
||||
await screen.findByText('640 lines');
|
||||
|
||||
// en.json bootstrap.hint_uv_sync. Asserted together with the ABSENCE of
|
||||
// hint_default, because falling back to the generic advice is exactly the
|
||||
// failure mode: the card still looks helpful while saying nothing.
|
||||
await waitFor(() => {
|
||||
expect(document.body.textContent).toContain('Dependency install failed');
|
||||
});
|
||||
expect(document.body.textContent).not.toContain('Try "Retry" first');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user