mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-28 21:44:38 +00:00
fix(flows): dismissible/auto-clearing run-error banner, no longer over undo/redo (#5011)
This commit is contained in:
@@ -146,6 +146,9 @@ export function asCopilotRepairSeed(state: unknown): CopilotRepairSeed | null {
|
||||
|
||||
const log = createDebug('app:flows:canvas');
|
||||
|
||||
/** How long the run-error banner stays up before it auto-dismisses. */
|
||||
const RUN_ERROR_AUTO_DISMISS_MS = 12_000;
|
||||
|
||||
/** Which panel (if any) the canvas side rail shows. Driven by the header toggle. */
|
||||
type SidePanel = 'copilot' | 'legend' | null;
|
||||
|
||||
@@ -159,6 +162,28 @@ function errorMessage(err: unknown): string {
|
||||
return err instanceof Error ? err.message : String(err);
|
||||
}
|
||||
|
||||
/**
|
||||
* Run-start failures bubble up through several `thiserror`-wrapped tinyflows
|
||||
* layers, each re-tagging the one beneath it (e.g. "capability error: graph
|
||||
* error: capability error: code node exited non-zero (timed_out=false):").
|
||||
* The nesting is meaningless to the user — strip repeated "<word> error: "
|
||||
* wrapper prefixes so only the innermost, actionable tail is shown. Scoped
|
||||
* to the run-error banner only (`handleRun`'s catch); other error surfaces
|
||||
* in this file keep the raw message since their shapes differ. Pure and
|
||||
* exported for direct unit testing without rendering `FlowEditor`.
|
||||
*/
|
||||
export function formatRunError(message: string): string {
|
||||
const wrapperPrefix = /^\w+\s+error:\s*/i;
|
||||
let rest = message;
|
||||
let match = wrapperPrefix.exec(rest);
|
||||
while (match && match[0].length < rest.length) {
|
||||
rest = rest.slice(match[0].length);
|
||||
match = wrapperPrefix.exec(rest);
|
||||
}
|
||||
const trimmed = rest.replace(/:\s*$/, '').trim();
|
||||
return trimmed || message.trim();
|
||||
}
|
||||
|
||||
/**
|
||||
* True when `title` is "unclaimed" — either blank or exactly the localized
|
||||
* generic placeholder (`t('flows.page.newWorkflow')`, "New workflow") — i.e.
|
||||
@@ -763,6 +788,17 @@ function FlowEditor({
|
||||
}
|
||||
}, [flowId]);
|
||||
|
||||
// Auto-dismiss the run-error banner so a stale failure doesn't linger
|
||||
// forever over the canvas. Re-runs (and thus restarts the timer) whenever
|
||||
// `runError` changes — including a new failure replacing an old one, since
|
||||
// `handleRun` always routes through `setRunError(null)` before setting the
|
||||
// next message, so a later error is always a distinct effect run.
|
||||
useEffect(() => {
|
||||
if (!runError) return;
|
||||
const timer = setTimeout(() => setRunError(null), RUN_ERROR_AUTO_DISMISS_MS);
|
||||
return () => clearTimeout(timer);
|
||||
}, [runError]);
|
||||
|
||||
// Return to wherever the user came from rather than always the list. React
|
||||
// Router stamps the initial history entry with key 'default', so when this
|
||||
// page was the first thing loaded (deep link / fresh load) there's nothing to
|
||||
@@ -968,12 +1004,44 @@ function FlowEditor({
|
||||
/>
|
||||
|
||||
{runError && (
|
||||
<div className="pointer-events-none absolute inset-x-3 top-3 z-20 flex justify-center">
|
||||
// top-14 (not top-3) so this never overlaps the canvas's own
|
||||
// top-right undo/redo controls, which sit at top-3; max-w-md
|
||||
// caps how wide a long nested error can grow. When the legend
|
||||
// palette is open it also docks at top-14/right-3 (w-48), so we
|
||||
// pull the banner's right edge in past it (right-56) rather than
|
||||
// centering across the full width, which would let a long
|
||||
// message reach under the palette and swallow its clicks.
|
||||
<div
|
||||
className={`pointer-events-none absolute left-3 top-14 z-20 flex justify-center ${
|
||||
sidePanel === 'legend' ? 'right-56' : 'right-3'
|
||||
}`}>
|
||||
<div
|
||||
role="alert"
|
||||
data-testid="flow-canvas-run-error"
|
||||
className="pointer-events-auto rounded-xl border border-coral-200 bg-coral-50 px-3 py-2 text-xs text-coral-700 dark:border-coral-500/30 dark:bg-coral-500/10 dark:text-coral-300">
|
||||
{t('flows.editor.runFailed')}: {runError}
|
||||
className="pointer-events-auto flex w-full max-w-md items-start gap-2 rounded-xl border border-coral-200 bg-coral-50 px-3 py-2 text-xs text-coral-700 dark:border-coral-500/30 dark:bg-coral-500/10 dark:text-coral-300">
|
||||
<span className="flex-1">
|
||||
{t('flows.editor.runFailed')}: {formatRunError(runError)}
|
||||
</span>
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => setRunError(null)}
|
||||
aria-label={t('common.dismiss')}
|
||||
title={t('common.dismiss')}
|
||||
data-testid="flow-canvas-run-error-dismiss"
|
||||
className="flex-shrink-0 text-coral-500 hover:text-coral-700 dark:text-coral-300 dark:hover:text-coral-100">
|
||||
<svg
|
||||
className="h-3.5 w-3.5"
|
||||
fill="none"
|
||||
stroke="currentColor"
|
||||
viewBox="0 0 24 24">
|
||||
<path
|
||||
strokeLinecap="round"
|
||||
strokeLinejoin="round"
|
||||
strokeWidth={2}
|
||||
d="M6 18L18 6M6 6l12 12"
|
||||
/>
|
||||
</svg>
|
||||
</button>
|
||||
</div>
|
||||
</div>
|
||||
)}
|
||||
|
||||
@@ -15,6 +15,7 @@ import FlowCanvasPage, {
|
||||
asCopilotBuildSeed,
|
||||
asCopilotPrefillSeed,
|
||||
FlowCanvasDraftPage,
|
||||
formatRunError,
|
||||
isPlaceholderTitle,
|
||||
} from '../FlowCanvasPage';
|
||||
|
||||
@@ -23,12 +24,14 @@ const updateFlow = vi.hoisted(() => vi.fn());
|
||||
const createFlow = vi.hoisted(() => vi.fn());
|
||||
const validateFlow = vi.hoisted(() => vi.fn());
|
||||
const listFlowConnections = vi.hoisted(() => vi.fn());
|
||||
const runFlow = vi.hoisted(() => vi.fn());
|
||||
vi.mock('../../services/api/flowsApi', () => ({
|
||||
getFlow,
|
||||
updateFlow,
|
||||
createFlow,
|
||||
validateFlow,
|
||||
listFlowConnections,
|
||||
runFlow,
|
||||
}));
|
||||
|
||||
// Stub the copilot panel: it drives the real chat runtime (redux + socket),
|
||||
@@ -95,6 +98,7 @@ describe('FlowCanvasPage', () => {
|
||||
createFlow.mockReset();
|
||||
validateFlow.mockReset();
|
||||
listFlowConnections.mockReset();
|
||||
runFlow.mockReset();
|
||||
validateFlow.mockResolvedValue({ valid: true, errors: [], warnings: [] });
|
||||
listFlowConnections.mockResolvedValue([]);
|
||||
updateFlow.mockResolvedValue(makeFlow());
|
||||
@@ -146,6 +150,93 @@ describe('FlowCanvasPage', () => {
|
||||
expect(screen.getByText('core unreachable')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
describe('run-error banner (#flows-canvas-error-banner)', () => {
|
||||
// Run always goes through the header icon → confirm popup → accept, same
|
||||
// as a real user flow (see `handleRun` wiring around `confirmAction`).
|
||||
// Two `fireEvent.click`s in a row: the first (Run) synchronously opens
|
||||
// the confirm popup via a plain `useState` setter, so no flush is needed
|
||||
// between them.
|
||||
function clickRun() {
|
||||
fireEvent.click(screen.getByTestId('flow-canvas-run'));
|
||||
fireEvent.click(screen.getByTestId('flow-action-confirm-accept'));
|
||||
}
|
||||
|
||||
it('renders the run-error banner (trimmed) without covering undo/redo, and lets it be dismissed', async () => {
|
||||
getFlow.mockResolvedValue(makeFlow());
|
||||
runFlow.mockRejectedValue(
|
||||
new Error(
|
||||
'capability error: graph error: capability error: code node exited non-zero (timed_out=false):'
|
||||
)
|
||||
);
|
||||
renderAtFlowId('test-id');
|
||||
await waitFor(() => expect(screen.getByTestId('flow-canvas')).toBeInTheDocument());
|
||||
|
||||
clickRun();
|
||||
|
||||
const banner = await screen.findByTestId('flow-canvas-run-error');
|
||||
// The raw nested "capability error: graph error: capability error: "
|
||||
// wrapper prefixes are stripped — only the innermost tail remains.
|
||||
expect(banner).toHaveTextContent('code node exited non-zero (timed_out=false)');
|
||||
expect(banner).not.toHaveTextContent('graph error');
|
||||
|
||||
// Regression for the overlap bug: the banner sits at top-14, well
|
||||
// below the canvas's own top-3 undo/redo controls, and both remain
|
||||
// present/interactive rather than one covering the other.
|
||||
expect(banner.parentElement).toHaveClass('top-14');
|
||||
expect(screen.getByTestId('flow-editor-undo')).toBeInTheDocument();
|
||||
expect(screen.getByTestId('flow-editor-redo')).toBeInTheDocument();
|
||||
|
||||
fireEvent.click(screen.getByTestId('flow-canvas-run-error-dismiss'));
|
||||
expect(screen.queryByTestId('flow-canvas-run-error')).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('auto-dismisses the run-error banner after the timeout, and restarts the timer on a new error', async () => {
|
||||
getFlow.mockResolvedValue(makeFlow());
|
||||
runFlow.mockRejectedValue(new Error('capability error: boom'));
|
||||
renderAtFlowId('test-id');
|
||||
await waitFor(() => expect(screen.getByTestId('flow-canvas')).toBeInTheDocument());
|
||||
|
||||
// Switch to fake timers only now — the load above already went through
|
||||
// `waitFor` (real timers); the run/timeout portion below only needs
|
||||
// fake macrotasks plus microtask flushes for the rejected `runFlow`
|
||||
// promise, matching the FlowRunsDrawer.test.tsx fake-timer precedent.
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
clickRun();
|
||||
await act(async () => {
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
});
|
||||
expect(screen.getByTestId('flow-canvas-run-error')).toBeInTheDocument();
|
||||
|
||||
// Not yet at the 12s mark — banner is still up.
|
||||
await act(async () => {
|
||||
vi.advanceTimersByTime(11_000);
|
||||
});
|
||||
expect(screen.getByTestId('flow-canvas-run-error')).toBeInTheDocument();
|
||||
|
||||
// A new failure before the old timer fires resets the clock rather
|
||||
// than stacking on top of it.
|
||||
clickRun();
|
||||
await act(async () => {
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
});
|
||||
await act(async () => {
|
||||
vi.advanceTimersByTime(11_000);
|
||||
});
|
||||
expect(screen.getByTestId('flow-canvas-run-error')).toBeInTheDocument();
|
||||
|
||||
await act(async () => {
|
||||
vi.advanceTimersByTime(2_000);
|
||||
});
|
||||
expect(screen.queryByTestId('flow-canvas-run-error')).not.toBeInTheDocument();
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
it('ignores a stale response for a superseded id after navigating to a new one', async () => {
|
||||
// Deferred promises so the test controls resolution order precisely: the
|
||||
// first (old-id) fetch resolves AFTER the second (new-id) one, mimicking
|
||||
@@ -529,6 +620,28 @@ describe('isPlaceholderTitle', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('formatRunError', () => {
|
||||
it('strips repeated nested "<word> error: " wrapper prefixes down to the innermost tail', () => {
|
||||
expect(
|
||||
formatRunError(
|
||||
'capability error: graph error: capability error: code node exited non-zero (timed_out=false):'
|
||||
)
|
||||
).toBe('code node exited non-zero (timed_out=false)');
|
||||
});
|
||||
|
||||
it('leaves a message with no wrapper prefix unchanged', () => {
|
||||
expect(formatRunError('flow has no trigger node')).toBe('flow has no trigger node');
|
||||
});
|
||||
|
||||
it('drops a bare trailing colon left over from a single, unstripped wrapper label', () => {
|
||||
expect(formatRunError('capability error:')).toBe('capability error');
|
||||
});
|
||||
|
||||
it('falls back to the original message rather than returning an empty string', () => {
|
||||
expect(formatRunError(':')).toBe(':');
|
||||
});
|
||||
});
|
||||
|
||||
// -----------------------------------------------------------------------------
|
||||
// Copilot proposal name adoption — accepting a `propose_workflow` proposal
|
||||
// carries a top-level `name` the canvas previously dropped, leaving the flow
|
||||
|
||||
Reference in New Issue
Block a user