mirror of
https://github.com/jamiepine/voicebox.git
synced 2026-10-04 01:25:18 -07:00
fix(ui): preserve the original error text and handle clipboard failures
Two CodeRabbit findings on #1058. condenseError trimmed the input before storing it in `full`, which is documented as the untouched original and is what the Copy action hands over. The trim now applies only to the working copy used for measuring and cutting, so `full` is byte-for-byte what the server sent while `display` and `omitted` still ignore surrounding blank space. The Copy handler called navigator.clipboard.writeText with no guard. Outside a secure context the property access itself throws, and writeText rejects when permission is denied; neither was handled, so a click could become an unhandled rejection with no sign that nothing was copied. Both paths are now caught and reported, pointing at Settings -> Logs as the fallback. Not taken: aligning MIN_TO_CONDENSE with the 400-char budget. The gap is deliberate -- cutting a 450-char error to 400 saves 50 characters in a description that already scrolls, and no Copy action is needed there because `display` holds the whole message. Documented in place. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
This commit is contained in:
committed by
capy-ai-staging[bot]
co-authored by
Claude Opus 5
parent
9ba2b33069
commit
96c6f9cad9
@@ -148,7 +148,23 @@ export function useGenerationProgress() {
|
|||||||
<ToastAction
|
<ToastAction
|
||||||
altText="Copy the full error text"
|
altText="Copy the full error text"
|
||||||
onClick={() => {
|
onClick={() => {
|
||||||
void navigator.clipboard.writeText(condensed.full);
|
// Two ways this fails: the Clipboard API is absent outside
|
||||||
|
// a secure context (property access throws), or writeText
|
||||||
|
// rejects because permission was denied. The try/catch
|
||||||
|
// covers both, so the click never becomes an unhandled
|
||||||
|
// rejection and the user is told nothing was copied.
|
||||||
|
void (async () => {
|
||||||
|
try {
|
||||||
|
await navigator.clipboard.writeText(condensed.full);
|
||||||
|
} catch {
|
||||||
|
toast({
|
||||||
|
title: 'Could not copy',
|
||||||
|
description:
|
||||||
|
'Clipboard unavailable. The full error is in Settings → Logs.',
|
||||||
|
variant: 'destructive',
|
||||||
|
});
|
||||||
|
}
|
||||||
|
})();
|
||||||
}}
|
}}
|
||||||
>
|
>
|
||||||
Copy
|
Copy
|
||||||
|
|||||||
@@ -16,36 +16,49 @@
|
|||||||
* paragraph — enough for a sentence or two of real message. */
|
* paragraph — enough for a sentence or two of real message. */
|
||||||
const TOAST_ERROR_BUDGET = 400;
|
const TOAST_ERROR_BUDGET = 400;
|
||||||
|
|
||||||
/** Below this there is nothing to gain by condensing. */
|
/** Below this, condensing is not worth it and the whole message is shown.
|
||||||
|
*
|
||||||
|
* Deliberately above the budget rather than equal to it. An error of 450
|
||||||
|
* characters would otherwise be cut to 400 to save 50 — a worse result than
|
||||||
|
* showing all of it, since the description scrolls anyway. The gap also gives
|
||||||
|
* the threshold hysteresis instead of flipping between full and truncated
|
||||||
|
* around a single character. No Copy action is offered in this range because
|
||||||
|
* nothing is being withheld: `display` already holds the entire message.
|
||||||
|
*/
|
||||||
const MIN_TO_CONDENSE = TOAST_ERROR_BUDGET + 120;
|
const MIN_TO_CONDENSE = TOAST_ERROR_BUDGET + 120;
|
||||||
|
|
||||||
export interface CondensedError {
|
export interface CondensedError {
|
||||||
/** What to show in the toast. */
|
/** What to show in the toast. Trimmed, and shortened when oversized. */
|
||||||
display: string;
|
display: string;
|
||||||
/** The untouched original, for copying. */
|
/** The original string exactly as received, for copying. Never modified. */
|
||||||
full: string;
|
full: string;
|
||||||
/** Whether `display` is shorter than `full`. */
|
/** Whether `display` omits part of the message. */
|
||||||
truncated: boolean;
|
truncated: boolean;
|
||||||
/** How many characters `display` leaves out. */
|
/** How many characters `display` leaves out. */
|
||||||
omitted: number;
|
omitted: number;
|
||||||
}
|
}
|
||||||
|
|
||||||
export function condenseError(raw: string | null | undefined): CondensedError {
|
export function condenseError(raw: string | null | undefined): CondensedError {
|
||||||
const full = (raw ?? '').trim();
|
// `full` is what the Copy action hands over, so it stays byte-for-byte what
|
||||||
|
// the server sent. All the measuring and cutting below works on the trimmed
|
||||||
|
// copy instead — surrounding blank space should not count toward the budget
|
||||||
|
// or the omitted count.
|
||||||
|
const full = raw ?? '';
|
||||||
|
const text = full.trim();
|
||||||
|
|
||||||
if (full.length <= MIN_TO_CONDENSE) {
|
if (text.length <= MIN_TO_CONDENSE) {
|
||||||
return { display: full, full, truncated: false, omitted: 0 };
|
return { display: text, full, truncated: false, omitted: 0 };
|
||||||
}
|
}
|
||||||
|
|
||||||
// A traceback's first line is nearly always the message; prefer it whenever
|
// A traceback's first line is nearly always the message; prefer it whenever
|
||||||
// it fits, since a newline is a stronger boundary than any punctuation.
|
// it fits, since a newline is a stronger boundary than any punctuation.
|
||||||
const firstLine = full.split('\n', 1)[0].trim();
|
const firstLine = text.split('\n', 1)[0].trim();
|
||||||
let head =
|
let head =
|
||||||
firstLine.length > 0 && firstLine.length <= TOAST_ERROR_BUDGET
|
firstLine.length > 0 && firstLine.length <= TOAST_ERROR_BUDGET
|
||||||
? firstLine
|
? firstLine
|
||||||
: full.slice(0, TOAST_ERROR_BUDGET);
|
: text.slice(0, TOAST_ERROR_BUDGET);
|
||||||
|
|
||||||
if (head.length < full.length && head === full.slice(0, head.length)) {
|
if (head.length < text.length && head === text.slice(0, head.length)) {
|
||||||
// Back off to the last sentence end inside the budget so the text does not
|
// Back off to the last sentence end inside the budget so the text does not
|
||||||
// stop mid-word. Only accept it if it keeps most of the budget — otherwise
|
// stop mid-word. Only accept it if it keeps most of the budget — otherwise
|
||||||
// a stray early period would throw away usable context.
|
// a stray early period would throw away usable context.
|
||||||
@@ -56,11 +69,11 @@ export function condenseError(raw: string | null | undefined): CondensedError {
|
|||||||
}
|
}
|
||||||
|
|
||||||
head = head.trimEnd();
|
head = head.trimEnd();
|
||||||
const omitted = full.length - head.length;
|
const omitted = text.length - head.length;
|
||||||
|
|
||||||
// Guard against the boundary search having produced nothing shorter.
|
// Guard against the boundary search having produced nothing shorter.
|
||||||
if (omitted <= 0) {
|
if (omitted <= 0) {
|
||||||
return { display: full, full, truncated: false, omitted: 0 };
|
return { display: text, full, truncated: false, omitted: 0 };
|
||||||
}
|
}
|
||||||
|
|
||||||
return { display: `${head} …`, full, truncated: true, omitted };
|
return { display: `${head} …`, full, truncated: true, omitted };
|
||||||
|
|||||||
Reference in New Issue
Block a user