mirror of
https://github.com/NanmiCoder/claude-code-haha.git
synced 2026-10-10 11:53:10 +08:00
fix: merge QA-002 Markdown image errors and fresh retries
This commit is contained in:
@@ -1,6 +1,7 @@
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import {
|
||||
api,
|
||||
apiGetBlob,
|
||||
ApiResponseParseError,
|
||||
getApiUrl,
|
||||
getDefaultBaseUrl,
|
||||
@@ -11,6 +12,16 @@ import {
|
||||
import { browserHost } from '../lib/desktopHost/browserHost'
|
||||
|
||||
describe('api diagnostics reporting', () => {
|
||||
it('passes cache bypass to fetch for an explicit image retry while keeping authentication', async () => {
|
||||
setBaseUrl('http://127.0.0.1:3456')
|
||||
setAuthToken('fixture-image-token')
|
||||
const fetchMock = vi.spyOn(globalThis, 'fetch').mockResolvedValue(new Response('png', { status: 200 }))
|
||||
await apiGetBlob('/preview-fs/s1/late.png', { cache: 'no-store' })
|
||||
expect(fetchMock).toHaveBeenCalledWith('http://127.0.0.1:3456/preview-fs/s1/late.png', expect.objectContaining({
|
||||
cache: 'no-store',
|
||||
headers: expect.objectContaining({ Authorization: 'Bearer fixture-image-token' }),
|
||||
}))
|
||||
})
|
||||
afterEach(() => {
|
||||
window.history.replaceState({}, '', '/')
|
||||
vi.useRealTimers()
|
||||
|
||||
@@ -483,7 +483,7 @@ function sanitizeDiagnosticValue(value: unknown): unknown {
|
||||
* image fires `error`). Fetching the bytes here and handing the DOM a blob URL
|
||||
* uses the credential path that already works for every other call.
|
||||
*/
|
||||
export async function apiGetBlob(path: string, options?: ApiRequestOptions): Promise<Blob> {
|
||||
export async function apiGetBlob(path: string, options?: ApiRequestOptions & { cache?: RequestCache }): Promise<Blob> {
|
||||
const controller = new AbortController()
|
||||
const timeoutMs = options?.timeout ?? DEFAULT_REQUEST_TIMEOUT_MS
|
||||
const timeout = setTimeout(() => controller.abort(), timeoutMs)
|
||||
@@ -495,6 +495,7 @@ export async function apiGetBlob(path: string, options?: ApiRequestOptions): Pro
|
||||
method: 'GET',
|
||||
headers: buildHeaders(),
|
||||
signal: controller.signal,
|
||||
...(options?.cache ? { cache: options.cache } : {}),
|
||||
})
|
||||
if (!res.ok) {
|
||||
throw new ApiError(res.status, await res.text().catch(() => ''))
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import '@testing-library/jest-dom'
|
||||
import { act, fireEvent, render, screen, within } from '@testing-library/react'
|
||||
import { act, fireEvent, render, screen, waitFor, within } from '@testing-library/react'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import type { WorkspaceStatusResult } from '../../api/sessions'
|
||||
@@ -11,6 +11,13 @@ import { useWorkspaceContentStore } from '../../stores/workspaceContentStore'
|
||||
import { AssistantMessage } from './AssistantMessage'
|
||||
|
||||
const BASE = 'http://127.0.0.1:4321'
|
||||
const apiGetBlob = vi.hoisted(() => vi.fn())
|
||||
|
||||
vi.mock('../../api/client', async (original) => ({
|
||||
...(await original<Record<string, unknown>>()),
|
||||
apiGetBlob,
|
||||
getBaseUrl: () => 'http://127.0.0.1:4321',
|
||||
}))
|
||||
|
||||
vi.mock('../../lib/desktopRuntime', async (orig) => ({
|
||||
...(await orig<Record<string, unknown>>()),
|
||||
@@ -40,19 +47,50 @@ function renderMessage(content: string, props: { isStreaming?: boolean } = {}) {
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
vi.spyOn(window, 'open').mockImplementation(() => null)
|
||||
apiGetBlob.mockReset().mockRejectedValue(new Error('404'))
|
||||
useSettingsStore.setState({ locale: 'en' })
|
||||
useOverlayStore.setState(useOverlayStore.getInitialState(), true)
|
||||
withWorkDir('/repo')
|
||||
vi.mocked(openLocalFileWithSystem).mockClear()
|
||||
vi.mocked(openLocalFileWithSystem).mockReset().mockResolvedValue(undefined)
|
||||
Reflect.deleteProperty(window, 'desktopHost')
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
withWorkDir(undefined)
|
||||
act(() => withWorkDir(undefined))
|
||||
Reflect.deleteProperty(window, 'desktopHost')
|
||||
vi.restoreAllMocks()
|
||||
})
|
||||
|
||||
describe('AssistantMessage · Markdown pictures on disk', () => {
|
||||
it('resets an outside-workspace image failure when the session changes even though its URL stays the same', async () => {
|
||||
const content = ''
|
||||
const { container, rerender } = render(<AssistantMessage sessionId="s1" content={content} />)
|
||||
fireEvent.error(proseImages(container)[0]!)
|
||||
await screen.findByRole('alert')
|
||||
act(() => useWorkspaceContentStore.setState({
|
||||
statusBySession: { s2: { state: 'ok', workDir: '/repo', repoName: null, branch: null, isGitRepo: false, changedFiles: [] } },
|
||||
}))
|
||||
rerender(<AssistantMessage sessionId="s2" content={content} />)
|
||||
expect(screen.queryByRole('alert')).not.toBeInTheDocument()
|
||||
expect(proseImages(container)[0]).toHaveAttribute('src', filesystem('/tmp/missing.png'))
|
||||
expect(proseImages(container)[0]).toBeVisible()
|
||||
})
|
||||
|
||||
it('does not apply a late image from the previous session to the new one', async () => {
|
||||
let finish!: (blob: Blob) => void
|
||||
apiGetBlob.mockReturnValueOnce(new Promise<Blob>((resolve) => { finish = resolve }))
|
||||
Object.defineProperty(URL, 'createObjectURL', { value: vi.fn(() => 'blob:old-session'), configurable: true, writable: true })
|
||||
Object.defineProperty(URL, 'revokeObjectURL', { value: vi.fn(), configurable: true, writable: true })
|
||||
const content = ''
|
||||
const { container, rerender } = render(<AssistantMessage sessionId="s1" content={content} />)
|
||||
fireEvent.error(proseImages(container)[0]!)
|
||||
rerender(<AssistantMessage sessionId="s2" content={content} />)
|
||||
act(() => finish(new Blob(['png'], { type: 'image/png' })))
|
||||
await waitFor(() => expect(URL.revokeObjectURL).toHaveBeenCalledWith('blob:old-session'))
|
||||
expect(screen.queryByRole('alert')).not.toBeInTheDocument()
|
||||
expect(proseImages(container)[0]).not.toHaveAttribute('src', 'blob:old-session')
|
||||
})
|
||||
it('serves a picture in the workdir from the session sandbox', () => {
|
||||
const { container } = renderMessage('')
|
||||
|
||||
|
||||
@@ -164,6 +164,7 @@ export const AssistantMessage = memo(function AssistantMessage({
|
||||
className="w-full text-[var(--color-text-primary)]"
|
||||
>
|
||||
<MarkdownRenderer
|
||||
key={`${sessionId ?? ''}|${workDir ?? ''}`}
|
||||
className="chat-reading-markdown"
|
||||
content={content}
|
||||
variant={documentLayout ? 'document' : 'default'}
|
||||
|
||||
@@ -65,4 +65,42 @@ describe('AuthedImage', () => {
|
||||
|
||||
await waitFor(() => expect(screen.getByRole('img')).toHaveAttribute('src', 'blob:two'))
|
||||
})
|
||||
|
||||
it('fetches a user retry directly with the credential instead of reusing a cached broken URL', async () => {
|
||||
fetchServerImageBlobUrl.mockResolvedValue('blob:retry')
|
||||
render(<AuthedImage src="http://127.0.0.1:1/a.png" alt="a" retryWithCredential />)
|
||||
|
||||
await waitFor(() => expect(screen.getByRole('img')).toHaveAttribute('src', 'blob:retry'))
|
||||
expect(fetchServerImageBlobUrl).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('ignores duplicate bare errors while authentication is pending and reports a decode failure once', async () => {
|
||||
let finish!: (url: string) => void
|
||||
fetchServerImageBlobUrl.mockReturnValue(new Promise<string>((resolve) => { finish = resolve }))
|
||||
const onFailure = vi.fn()
|
||||
render(<AuthedImage src="http://127.0.0.1:1/a.png" alt="a" onFailure={onFailure} />)
|
||||
fireEvent.error(screen.getByRole('img'))
|
||||
fireEvent.error(screen.getByRole('img'))
|
||||
expect(onFailure).not.toHaveBeenCalled()
|
||||
finish('blob:invalid')
|
||||
await waitFor(() => expect(screen.getByRole('img')).toHaveAttribute('src', 'blob:invalid'))
|
||||
fireEvent.error(screen.getByRole('img'))
|
||||
fireEvent.error(screen.getByRole('img'))
|
||||
expect(onFailure).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('discards a late copy after switching sources, even when returning to the original source', async () => {
|
||||
let finish!: (url: string) => void
|
||||
fetchServerImageBlobUrl.mockReturnValueOnce(new Promise<string>((resolve) => { finish = resolve }))
|
||||
.mockResolvedValueOnce('blob:new-a')
|
||||
const { rerender } = render(<AuthedImage src="http://127.0.0.1:1/a.png" alt="a" />)
|
||||
fireEvent.error(screen.getByRole('img'))
|
||||
rerender(<AuthedImage src="http://127.0.0.1:1/b.png" alt="a" />)
|
||||
rerender(<AuthedImage src="http://127.0.0.1:1/a.png" alt="a" />)
|
||||
finish('blob:stale-a')
|
||||
await waitFor(() => expect(URL.revokeObjectURL).toHaveBeenCalledWith('blob:stale-a'))
|
||||
expect(screen.getByRole('img')).toHaveAttribute('src', 'http://127.0.0.1:1/a.png')
|
||||
fireEvent.error(screen.getByRole('img'))
|
||||
await waitFor(() => expect(screen.getByRole('img')).toHaveAttribute('src', 'blob:new-a'))
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,13 +1,15 @@
|
||||
import type { ImgHTMLAttributes } from 'react'
|
||||
import { useAuthedImageFallback } from '../../lib/useAuthedImageFallback'
|
||||
import { useAuthedImageFallback } from '@/lib/useAuthedImageFallback'
|
||||
|
||||
type Props = Omit<ImgHTMLAttributes<HTMLImageElement>, 'onError'> & {
|
||||
/** Runs once the image has failed even with the app's credential. */
|
||||
onFailure?: () => void
|
||||
/** A user retry skips the potentially cached bare failure and fetches afresh. */
|
||||
retryWithCredential?: boolean
|
||||
}
|
||||
|
||||
/** An `<img>` for a local-server URL that also loads where a bare request is refused (web UI, H5). */
|
||||
export function AuthedImage({ src, onFailure, alt = '', ...rest }: Props) {
|
||||
const image = useAuthedImageFallback(src, onFailure)
|
||||
export function AuthedImage({ src, onFailure, retryWithCredential = false, alt = '', ...rest }: Props) {
|
||||
const image = useAuthedImageFallback(src, onFailure, retryWithCredential)
|
||||
return <img {...rest} alt={alt} src={image.src} onError={image.onError} />
|
||||
}
|
||||
|
||||
@@ -0,0 +1,79 @@
|
||||
import '@testing-library/jest-dom'
|
||||
import { act, fireEvent, render, screen, waitFor } from '@testing-library/react'
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { useSettingsStore } from '@/stores/settingsStore'
|
||||
import { MarkdownHtml } from './MarkdownHtml'
|
||||
|
||||
const fetchServerImageBlobUrl = vi.hoisted(() => vi.fn())
|
||||
vi.mock('@/lib/authedImage', () => ({ fetchServerImageBlobUrl }))
|
||||
|
||||
beforeEach(() => {
|
||||
fetchServerImageBlobUrl.mockReset().mockRejectedValue(new Error('404'))
|
||||
useSettingsStore.setState({ locale: 'en' })
|
||||
Object.defineProperty(URL, 'revokeObjectURL', { value: vi.fn(), configurable: true, writable: true })
|
||||
})
|
||||
|
||||
describe('MarkdownHtml image lifecycle', () => {
|
||||
it('keeps prose without images in its original layout', () => {
|
||||
const { container } = render(<MarkdownHtml html="<p>Text</p>" />)
|
||||
expect(container.firstElementChild?.firstElementChild?.tagName).toBe('P')
|
||||
})
|
||||
|
||||
it('preserves image presentation but ignores raw HTML claims to a portal slot', () => {
|
||||
const { container } = render(<MarkdownHtml html={'<p><span data-md-image="">text</span><img src="/a.png" alt="a" title="title" width="80" height="40" class="picture"></p>'} />)
|
||||
expect(container.querySelectorAll('[data-md-image]')).toHaveLength(1)
|
||||
const image = screen.getByRole('img', { name: 'a' })
|
||||
expect(image).toHaveAttribute('title', 'title')
|
||||
expect(image).toHaveAttribute('width', '80')
|
||||
expect(image).toHaveAttribute('height', '40')
|
||||
expect(image).toHaveClass('picture')
|
||||
})
|
||||
|
||||
it.each([
|
||||
['en', 'Unable to load image', 'Retry image: 图片.png', 'Retry'],
|
||||
['zh', '无法加载图片', '重试图片:图片.png', '重试'],
|
||||
['zh-TW', '無法載入圖片', '重試圖片:图片.png', '重試'],
|
||||
['jp', '画像を読み込めません', '画像を再読み込み: 图片.png', '再試行'],
|
||||
['kr', '이미지를 불러올 수 없습니다', '이미지 다시 시도: 图片.png', '다시 시도'],
|
||||
] as const)('translates the error, retry, and accessible name in %s', async (locale, title, name, retry) => {
|
||||
useSettingsStore.setState({ locale })
|
||||
render(<MarkdownHtml html={'<p><img src="/preview-fs/s1/%E5%9B%BE%E7%89%87.png" alt="description"></p>'} />)
|
||||
fireEvent.error(screen.getByRole('img'))
|
||||
expect(await screen.findByRole('alert')).toHaveTextContent(title)
|
||||
expect(screen.getByRole('button', { name })).toHaveTextContent(retry)
|
||||
})
|
||||
|
||||
it('reacts to a locale change while the failure is visible', async () => {
|
||||
render(<MarkdownHtml html={'<img src="/missing.png" alt="description">'} />)
|
||||
fireEvent.error(screen.getByRole('img'))
|
||||
await screen.findByRole('alert')
|
||||
act(() => useSettingsStore.setState({ locale: 'zh' }))
|
||||
expect(screen.getByRole('alert')).toHaveTextContent('无法加载图片')
|
||||
expect(screen.getByRole('button', { name: '重试图片:missing.png' })).toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('keeps the placeholder when the retry returns a body that cannot decode', async () => {
|
||||
fetchServerImageBlobUrl.mockRejectedValueOnce(new Error('404')).mockResolvedValueOnce('blob:invalid')
|
||||
const { container } = render(<MarkdownHtml html={'<img src="/missing.png" alt="description">'} />)
|
||||
fireEvent.error(screen.getByRole('img'))
|
||||
await screen.findByRole('alert')
|
||||
fireEvent.click(screen.getByRole('button', { name: /Retry image/ }))
|
||||
await waitFor(() => expect(container.querySelector('img')).toHaveAttribute('src', 'blob:invalid'))
|
||||
fireEvent.error(container.querySelector('img')!)
|
||||
expect(screen.getByRole('alert')).toHaveTextContent('missing.png')
|
||||
expect(screen.getByRole('button', { name: /Retry image/ })).toBeEnabled()
|
||||
})
|
||||
|
||||
it('frees the old image copy and resets failures when the Markdown changes', async () => {
|
||||
fetchServerImageBlobUrl.mockResolvedValue('blob:old')
|
||||
const { rerender } = render(<MarkdownHtml html={'<img src="/a.png" alt="a">'} />)
|
||||
fireEvent.error(screen.getByRole('img'))
|
||||
await waitFor(() => expect(screen.getByRole('img')).toHaveAttribute('src', 'blob:old'))
|
||||
fireEvent.error(screen.getByRole('img'))
|
||||
expect(screen.getByRole('alert')).toBeInTheDocument()
|
||||
rerender(<MarkdownHtml html={'<p>new</p><img src="/b.png" alt="b">'} />)
|
||||
expect(screen.queryByRole('alert')).not.toBeInTheDocument()
|
||||
expect(screen.getByRole('img')).toHaveAttribute('src', '/b.png')
|
||||
expect(URL.revokeObjectURL).toHaveBeenCalledWith('blob:old')
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,120 @@
|
||||
import { useId, useLayoutEffect, useMemo, useRef, useState } from 'react'
|
||||
import type { HTMLAttributes, ImgHTMLAttributes } from 'react'
|
||||
import { createPortal } from 'react-dom'
|
||||
import { AuthedImage } from '@/components/chat/AuthedImage'
|
||||
import { Button } from '@/components/ui/Button'
|
||||
import { ErrorState } from '@/components/ui/ErrorState'
|
||||
import { useTranslation } from '@/i18n'
|
||||
|
||||
type ImageProps = Pick<ImgHTMLAttributes<HTMLImageElement>, 'src' | 'alt' | 'title' | 'width' | 'height' | 'className'>
|
||||
type Props = HTMLAttributes<HTMLDivElement> & { html: string }
|
||||
|
||||
function imageName(src: string, fallback: string): string {
|
||||
if (/^(blob:|data:)/i.test(src)) return fallback
|
||||
try {
|
||||
const url = new URL(src, 'http://markdown.invalid')
|
||||
const path = url.searchParams.get('path') ?? decodeURIComponent(url.pathname)
|
||||
return path.split(/[\\/]/).filter(Boolean).pop() || fallback
|
||||
} catch {
|
||||
return fallback
|
||||
}
|
||||
}
|
||||
|
||||
function MarkdownImage(props: ImageProps) {
|
||||
const t = useTranslation()
|
||||
const errorId = useId()
|
||||
const [status, setStatus] = useState<'initial' | 'failed' | 'retrying'>('initial')
|
||||
const [attempt, setAttempt] = useState(0)
|
||||
const name = imageName(props.src ?? '', props.alt || t('assistantOutputs.kind.image'))
|
||||
const showError = status !== 'initial'
|
||||
|
||||
return (
|
||||
<>
|
||||
<AuthedImage
|
||||
{...props}
|
||||
key={attempt}
|
||||
retryWithCredential={attempt > 0}
|
||||
hidden={showError}
|
||||
style={{ display: showError ? 'none' : undefined }}
|
||||
onLoad={() => setStatus('initial')}
|
||||
onFailure={() => setStatus('failed')}
|
||||
/>
|
||||
{showError && (
|
||||
<span className="not-prose my-2 flex max-w-full flex-col gap-2">
|
||||
<span id={errorId}>
|
||||
<ErrorState
|
||||
as="span"
|
||||
size="sm"
|
||||
title={t('chat.imageLoadFailed')}
|
||||
detail={<><span className="block break-all">{name}</span>{t('chat.imageLoadFailedHint')}</>}
|
||||
/>
|
||||
</span>
|
||||
<Button
|
||||
variant="secondary"
|
||||
size="lg"
|
||||
className="min-h-11 self-start"
|
||||
aria-label={t('chat.retryImage', { name })}
|
||||
aria-describedby={errorId}
|
||||
loading={status === 'retrying'}
|
||||
onClick={(event) => {
|
||||
// Images can sit inside links. Retrying belongs to the image only.
|
||||
event.preventDefault()
|
||||
event.stopPropagation()
|
||||
setStatus('retrying')
|
||||
setAttempt((previous) => previous + 1)
|
||||
}}
|
||||
>
|
||||
{t(status === 'retrying' ? 'chat.imageRetrying' : 'common.retry')}
|
||||
</Button>
|
||||
</span>
|
||||
)}
|
||||
</>
|
||||
)
|
||||
}
|
||||
|
||||
/** Mount existing image components in sanitized HTML without reparsing its layout. */
|
||||
function MarkdownHtmlContent({ html, ...props }: Props) {
|
||||
const containerRef = useRef<HTMLDivElement>(null)
|
||||
const [mounts, setMounts] = useState<HTMLElement[]>([])
|
||||
const prepared = useMemo(() => {
|
||||
const template = document.createElement('template')
|
||||
template.innerHTML = html
|
||||
const images: ImageProps[] = []
|
||||
// Raw HTML cannot claim a portal slot; only images surviving the resolver get one.
|
||||
template.content.querySelectorAll('[data-md-image]').forEach((node) => node.removeAttribute('data-md-image'))
|
||||
template.content.querySelectorAll('img').forEach((image) => {
|
||||
images.push({
|
||||
src: image.getAttribute('src') ?? undefined,
|
||||
alt: image.getAttribute('alt') ?? '',
|
||||
title: image.getAttribute('title') ?? undefined,
|
||||
width: image.getAttribute('width') ?? undefined,
|
||||
height: image.getAttribute('height') ?? undefined,
|
||||
className: image.getAttribute('class') ?? undefined,
|
||||
})
|
||||
const mount = document.createElement('span')
|
||||
mount.setAttribute('data-md-image', '')
|
||||
image.replaceWith(mount)
|
||||
})
|
||||
return { html: template.innerHTML, images }
|
||||
}, [html])
|
||||
|
||||
useLayoutEffect(() => {
|
||||
setMounts(Array.from(containerRef.current?.querySelectorAll<HTMLElement>('[data-md-image]') ?? []))
|
||||
}, [prepared])
|
||||
|
||||
return (
|
||||
<div {...props} ref={containerRef}>
|
||||
<div dangerouslySetInnerHTML={{ __html: prepared.html }} />
|
||||
{mounts.map((mount, index) => createPortal(<MarkdownImage {...prepared.images[index]} />, mount, String(index)))}
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
||||
export function MarkdownHtml(props: Props) {
|
||||
if (!/<img\b/i.test(props.html)) {
|
||||
const { html, ...rest } = props
|
||||
return <div {...rest} dangerouslySetInnerHTML={{ __html: html }} />
|
||||
}
|
||||
// HTML changes discard the previous image instances, including pending retries.
|
||||
return <MarkdownHtmlContent key={props.html} {...props} />
|
||||
}
|
||||
@@ -1,5 +1,5 @@
|
||||
import '@testing-library/jest-dom'
|
||||
import { fireEvent, render, waitFor } from '@testing-library/react'
|
||||
import { fireEvent, render, screen, waitFor, within } from '@testing-library/react'
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
const apiGetBlob = vi.hoisted(() => vi.fn())
|
||||
@@ -11,16 +11,87 @@ vi.mock('../../api/client', async (original) => ({
|
||||
}))
|
||||
|
||||
import { MarkdownRenderer } from './MarkdownRenderer'
|
||||
import { useSettingsStore } from '@/stores/settingsStore'
|
||||
|
||||
const LOCAL = 'http://127.0.0.1:3456/api/filesystem/file?path=%2Ftmp%2Fchart.png'
|
||||
|
||||
beforeEach(() => {
|
||||
useSettingsStore.setState({ locale: 'en' })
|
||||
apiGetBlob.mockReset().mockResolvedValue(new Blob(['png'], { type: 'image/png' }))
|
||||
Object.defineProperty(URL, 'createObjectURL', { value: vi.fn(() => 'blob:http://localhost/chart'), configurable: true, writable: true })
|
||||
Object.defineProperty(URL, 'revokeObjectURL', { value: vi.fn(), configurable: true, writable: true })
|
||||
})
|
||||
|
||||
describe('MarkdownRenderer local images', () => {
|
||||
// QA-002: a terminal image error used to leave only the browser's broken icon.
|
||||
it.each([404, 403])('keeps a named error placeholder after HTTP %s and lets a retry recover', async (status) => {
|
||||
apiGetBlob.mockRejectedValueOnce(new Error(String(status)))
|
||||
.mockRejectedValueOnce(new Error(String(status)))
|
||||
.mockResolvedValueOnce(new Blob(['png'], { type: 'image/png' }))
|
||||
const { container } = render(
|
||||
<MarkdownRenderer content="" resolveImageSrc={() => LOCAL.replace('chart.png', 'missing.png')} />,
|
||||
)
|
||||
fireEvent.error(container.querySelector('img')!)
|
||||
|
||||
const alert = await screen.findByRole('alert')
|
||||
expect(alert).toHaveTextContent('Unable to load image')
|
||||
expect(alert).toHaveTextContent('missing.png')
|
||||
expect(alert).toHaveTextContent('The file may be missing or access may be denied.')
|
||||
expect(screen.queryByRole('img')).not.toBeInTheDocument()
|
||||
const retry = screen.getByRole('button', { name: 'Retry image: missing.png' })
|
||||
expect(retry).toHaveAttribute('aria-describedby')
|
||||
expect(retry.className).toContain('focus-visible:ring-2')
|
||||
fireEvent.click(retry)
|
||||
await waitFor(() => expect(apiGetBlob).toHaveBeenCalledTimes(2))
|
||||
expect(screen.getByRole('alert')).toHaveTextContent('missing.png')
|
||||
await waitFor(() => expect(retry).toBeEnabled())
|
||||
|
||||
fireEvent.click(retry)
|
||||
await waitFor(() => expect(container.querySelector('img')).toHaveAttribute('src', 'blob:http://localhost/chart'))
|
||||
// A successful HTTP response is insufficient: keep the notice until decode succeeds.
|
||||
expect(screen.getByRole('alert')).toBeInTheDocument()
|
||||
fireEvent.load(container.querySelector('img')!)
|
||||
expect(screen.queryByRole('alert')).not.toBeInTheDocument()
|
||||
expect(screen.getByRole('img', { name: 'description' })).toBeVisible()
|
||||
})
|
||||
|
||||
it('keeps failures local to one image and excludes it from the viewer', async () => {
|
||||
apiGetBlob.mockRejectedValue(new Error('404'))
|
||||
const onImageClick = vi.fn()
|
||||
const { container } = render(
|
||||
<MarkdownRenderer content="\n\n" resolveImageSrc={(src) => LOCAL.replace('chart.png', src)} onImageClick={onImageClick} />,
|
||||
)
|
||||
fireEvent.error(container.querySelector('img')!)
|
||||
await screen.findByRole('alert')
|
||||
fireEvent.click(screen.getByRole('img', { name: 'valid' }))
|
||||
expect(onImageClick).toHaveBeenCalledWith({ images: [{ src: LOCAL, alt: 'valid' }], index: 0 })
|
||||
})
|
||||
|
||||
it('does not navigate a surrounding link when retry is clicked', async () => {
|
||||
apiGetBlob.mockRejectedValue(new Error('404'))
|
||||
const onLinkClick = vi.fn()
|
||||
const { container } = render(
|
||||
<MarkdownRenderer content="[](https://example.com)" resolveImageSrc={() => LOCAL} onLinkClick={onLinkClick} />,
|
||||
)
|
||||
fireEvent.error(container.querySelector('img')!)
|
||||
await screen.findByRole('alert')
|
||||
fireEvent.click(screen.getByRole('button', { name: /Retry image/ }))
|
||||
await waitFor(() => expect(apiGetBlob).toHaveBeenCalledTimes(2))
|
||||
await waitFor(() => expect(screen.getByRole('button', { name: /Retry image/ })).toBeEnabled())
|
||||
expect(onLinkClick).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('does not reuse a failure from another renderer with the same parsed Markdown', async () => {
|
||||
apiGetBlob.mockRejectedValue(new Error('404'))
|
||||
const content = ''
|
||||
const { container } = render(<><MarkdownRenderer content={content} resolveImageSrc={() => LOCAL} /><MarkdownRenderer content={content} resolveImageSrc={() => LOCAL} /></>)
|
||||
const prose = container.querySelectorAll<HTMLElement>('.markdown-prose')
|
||||
fireEvent.error(prose[0]!.querySelector('img')!)
|
||||
await within(prose[0]!).findByRole('alert')
|
||||
expect(within(prose[1]!).queryByRole('alert')).not.toBeInTheDocument()
|
||||
expect(within(prose[1]!).getByRole('img')).toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('retries a refused local image with the app credential', async () => {
|
||||
const { container } = render(
|
||||
<MarkdownRenderer content="" resolveImageSrc={() => LOCAL} />,
|
||||
@@ -50,7 +121,7 @@ describe('MarkdownRenderer local images', () => {
|
||||
)
|
||||
|
||||
fireEvent.error(container.querySelector('img')!)
|
||||
await Promise.resolve()
|
||||
await screen.findByRole('alert')
|
||||
|
||||
expect(apiGetBlob).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { memo, useMemo, useCallback, useRef } from 'react'
|
||||
import { memo, useMemo, useCallback } from 'react'
|
||||
import type { MouseEvent as ReactMouseEvent } from 'react'
|
||||
import DOMPurify from 'dompurify'
|
||||
import katex from 'katex'
|
||||
@@ -20,7 +20,7 @@ import { isSafeMarkdownImageSource, normalizeMarkdownImageDestination } from '@/
|
||||
import { CodeViewer } from '../chat/CodeViewer'
|
||||
import { MermaidRenderer } from '../chat/MermaidRenderer'
|
||||
import { copyTextToClipboard } from '@/lib/clipboard'
|
||||
import { attachAuthedImageFallback } from '@/lib/authedImage'
|
||||
import { MarkdownHtml } from '@/components/markdown/MarkdownHtml'
|
||||
import { t } from '../../i18n'
|
||||
|
||||
type Props = {
|
||||
@@ -618,7 +618,7 @@ function reportImageClick(
|
||||
): void {
|
||||
const clicked = target?.closest<HTMLImageElement>('img')
|
||||
if (!clicked || !container.contains(clicked)) return
|
||||
const images = Array.from(container.querySelectorAll<HTMLImageElement>('img')).filter((image) => image.getAttribute('src'))
|
||||
const images = Array.from(container.querySelectorAll<HTMLImageElement>('img')).filter((image) => !image.hidden && image.getAttribute('src'))
|
||||
const index = images.indexOf(clicked)
|
||||
if (index < 0) return
|
||||
onImageClick({
|
||||
@@ -716,30 +716,21 @@ export const MarkdownRenderer = memo(function MarkdownRenderer({ content, varian
|
||||
}, 1500)
|
||||
}, [onImageClick, onLinkClick])
|
||||
|
||||
// Local images the server refuses to hand to a bare <img> (web UI, H5) get one
|
||||
// authenticated retry; a callback ref keeps the listener on whichever div renders.
|
||||
const detachImageFallback = useRef<(() => void) | null>(null)
|
||||
const imageFallbackRef = useCallback((node: HTMLDivElement | null) => {
|
||||
detachImageFallback.current?.()
|
||||
detachImageFallback.current = node ? attachAuthedImageFallback(node) : null
|
||||
}, [])
|
||||
|
||||
if (codeBlocks.length === 0) {
|
||||
return (
|
||||
<div
|
||||
ref={imageFallbackRef}
|
||||
<MarkdownHtml
|
||||
className={proseClasses}
|
||||
dangerouslySetInnerHTML={{ __html: parts[0]?.type === 'html' ? parts[0].content : '' }}
|
||||
html={parts[0]?.type === 'html' ? parts[0].content : ''}
|
||||
onClick={handleClick}
|
||||
/>
|
||||
)
|
||||
}
|
||||
|
||||
return (
|
||||
<div ref={imageFallbackRef} className={proseClasses} onClick={handleClick}>
|
||||
<div className={proseClasses} onClick={handleClick}>
|
||||
{parts.map((part, i) =>
|
||||
part.type === 'html' ? (
|
||||
<div key={i} dangerouslySetInnerHTML={{ __html: part.content }} />
|
||||
<MarkdownHtml key={i} html={part.content} />
|
||||
) : shouldRenderAsMermaid(part.block) ? (
|
||||
streaming ? (
|
||||
<MermaidStreamingPlaceholder key={part.block.id} />
|
||||
|
||||
@@ -5,6 +5,11 @@ import { describe, expect, it, vi } from 'vitest'
|
||||
import { ErrorState } from './ErrorState'
|
||||
|
||||
describe('ErrorState', () => {
|
||||
it('can render inside inline Markdown without putting a div inside a paragraph', () => {
|
||||
render(<p><ErrorState as="span" title="Image failed" detail="missing.png" /></p>)
|
||||
expect(screen.getByRole('alert').tagName).toBe('SPAN')
|
||||
expect(screen.getByRole('alert')).toHaveTextContent('missing.png')
|
||||
})
|
||||
it('announces the failure through an alert', () => {
|
||||
// Most of the replaced markup was a plain <div>, which a screen reader user
|
||||
// only encounters by chance.
|
||||
|
||||
@@ -5,6 +5,8 @@ import { Button } from './Button'
|
||||
import type { StateSize } from './EmptyState'
|
||||
|
||||
export type ErrorStateProps = {
|
||||
/** Inline Markdown needs phrasing content inside paragraphs and links. */
|
||||
as?: 'div' | 'span'
|
||||
title: string
|
||||
detail?: ReactNode
|
||||
onRetry?: () => void
|
||||
@@ -34,6 +36,7 @@ const SIZE_CLASSES: Record<StateSize, string> = {
|
||||
* reader user only discovers by chance.
|
||||
*/
|
||||
export function ErrorState({
|
||||
as: Tag = 'div',
|
||||
title,
|
||||
detail,
|
||||
onRetry,
|
||||
@@ -43,7 +46,7 @@ export function ErrorState({
|
||||
className,
|
||||
}: ErrorStateProps) {
|
||||
return (
|
||||
<div
|
||||
<Tag
|
||||
role="alert"
|
||||
className={cx(
|
||||
'flex flex-col rounded-[var(--radius-lg)] border',
|
||||
@@ -63,6 +66,6 @@ export function ErrorState({
|
||||
{retryLabel}
|
||||
</Button>
|
||||
)}
|
||||
</div>
|
||||
</Tag>
|
||||
)
|
||||
}
|
||||
|
||||
@@ -2621,6 +2621,8 @@ Row 9, all 8 cells: continuing from straight down, turning left through lower-le
|
||||
'chat.addSelectionToChat': 'Add to chat',
|
||||
'chat.imageLoadFailed': 'Unable to load image',
|
||||
'chat.imageLoadFailedHint': 'The file may be missing or access may be denied.',
|
||||
'chat.retryImage': 'Retry image: {name}',
|
||||
'chat.imageRetrying': 'Retrying…',
|
||||
'chat.branchFromHere': 'Fork a new conversation',
|
||||
'chat.branchSuccess': 'Created forked conversation "{title}".',
|
||||
'chat.branchError': 'Failed to branch from this message. Detail: {detail}',
|
||||
|
||||
@@ -2622,6 +2622,8 @@ export const jp: Record<TranslationKey, string> = {
|
||||
'chat.addSelectionToChat': 'チャットに追加',
|
||||
'chat.imageLoadFailed': '画像を読み込めません',
|
||||
'chat.imageLoadFailedHint': 'ファイルが存在しないか、アクセスが許可されていない可能性があります。',
|
||||
'chat.retryImage': '画像を再読み込み: {name}',
|
||||
'chat.imageRetrying': '再読み込み中…',
|
||||
'chat.branchFromHere': '新しい会話を分岐',
|
||||
'chat.branchSuccess': '分岐した会話「{title}」を作成しました。',
|
||||
'chat.branchError': 'このメッセージから分岐できませんでした。詳細: {detail}',
|
||||
|
||||
@@ -2624,6 +2624,8 @@ export const kr: Record<TranslationKey, string> = {
|
||||
'chat.addSelectionToChat': '채팅에 추가',
|
||||
'chat.imageLoadFailed': '이미지를 불러올 수 없습니다',
|
||||
'chat.imageLoadFailedHint': '파일이 없거나 접근 권한이 없을 수 있습니다.',
|
||||
'chat.retryImage': '이미지 다시 시도: {name}',
|
||||
'chat.imageRetrying': '다시 시도 중…',
|
||||
'chat.branchFromHere': '새 대화 분기',
|
||||
'chat.branchSuccess': '분기된 대화 "{title}"을(를) 만들었습니다.',
|
||||
'chat.branchError': '이 메시지에서 분기할 수 없습니다. 세부 정보: {detail}',
|
||||
|
||||
@@ -2621,6 +2621,8 @@ export const zh: Record<TranslationKey, string> = {
|
||||
'chat.addSelectionToChat': '新增到對話',
|
||||
'chat.imageLoadFailed': '無法載入圖片',
|
||||
'chat.imageLoadFailedHint': '檔案可能不存在,或沒有存取權限。',
|
||||
'chat.retryImage': '重試圖片:{name}',
|
||||
'chat.imageRetrying': '正在重試…',
|
||||
'chat.branchFromHere': 'Fork 一個新對話',
|
||||
'chat.branchSuccess': '已 Fork 新對話“{title}”。',
|
||||
'chat.branchError': '從該訊息 Fork 新對話失敗。詳情:{detail}',
|
||||
|
||||
@@ -2620,6 +2620,8 @@ export const zh: Record<TranslationKey, string> = {
|
||||
'chat.addSelectionToChat': '添加到对话',
|
||||
'chat.imageLoadFailed': '无法加载图片',
|
||||
'chat.imageLoadFailedHint': '文件可能不存在,或没有访问权限。',
|
||||
'chat.retryImage': '重试图片:{name}',
|
||||
'chat.imageRetrying': '正在重试…',
|
||||
'chat.branchFromHere': 'Fork 一个新对话',
|
||||
'chat.branchSuccess': '已 Fork 新对话“{title}”。',
|
||||
'chat.branchError': '从该消息 Fork 新对话失败。详情:{detail}',
|
||||
|
||||
@@ -19,6 +19,10 @@ afterEach(() => {
|
||||
})
|
||||
|
||||
describe('fetchServerImageBlobUrl', () => {
|
||||
it('bypasses cached missing-file responses when the user explicitly retries', async () => {
|
||||
await fetchServerImageBlobUrl('http://127.0.0.1:3456/preview-fs/s1/late.png', true)
|
||||
expect(apiGetBlob).toHaveBeenCalledWith('/preview-fs/s1/late.png', { cache: 'no-store' })
|
||||
})
|
||||
it('fetches a local-server image through the credentialed client and returns an object URL', async () => {
|
||||
const src = `http://127.0.0.1:3456/api/filesystem/file?path=${encodeURIComponent('/tmp/fti work/chart.png')}`
|
||||
|
||||
|
||||
@@ -12,11 +12,14 @@ import { apiGetBlob, getBaseUrl } from '../api/client'
|
||||
* Only URLs on the local server's own origin are fetched: the credential must not
|
||||
* follow an arbitrary image URL to another host.
|
||||
*/
|
||||
export async function fetchServerImageBlobUrl(src: string): Promise<string> {
|
||||
export async function fetchServerImageBlobUrl(src: string, bypassCache = false): Promise<string> {
|
||||
const base = new URL(getBaseUrl())
|
||||
const target = new URL(src, base)
|
||||
if (target.origin !== base.origin) throw new Error('Not a local-server image URL')
|
||||
const blob = await apiGetBlob(`${target.pathname}${target.search}`)
|
||||
const path = `${target.pathname}${target.search}`
|
||||
// Missing files can become available between retries. Reusing a cached 404
|
||||
// would make the same error permanent even after the user adds the file.
|
||||
const blob = bypassCache ? await apiGetBlob(path, { cache: 'no-store' }) : await apiGetBlob(path)
|
||||
return URL.createObjectURL(blob)
|
||||
}
|
||||
|
||||
|
||||
@@ -10,9 +10,11 @@ import { fetchServerImageBlobUrl } from './authedImage'
|
||||
* is not an image — so callers keep their own failure notice for real failures and
|
||||
* never flash it for a request that was merely missing a header.
|
||||
*/
|
||||
export function useAuthedImageFallback(src: string | undefined, onFailure?: () => void) {
|
||||
const [resolved, setResolved] = useState<{ source: string; url: string } | null>(null)
|
||||
const triedSource = useRef<string | undefined>(undefined)
|
||||
export function useAuthedImageFallback(src: string | undefined, onFailure?: () => void, retryWithCredential = false) {
|
||||
const attempt = useRef({ src, state: 'idle' as 'idle' | 'fetching' | 'resolved' | 'failed' })
|
||||
if (attempt.current.src !== src) attempt.current = { src, state: 'idle' }
|
||||
const current = attempt.current
|
||||
const [resolved, setResolved] = useState<{ attempt: typeof current; url: string } | null>(null)
|
||||
const alive = useRef(true)
|
||||
const objectUrls = useRef<string[]>([])
|
||||
const onFailureRef = useRef(onFailure)
|
||||
@@ -29,22 +31,32 @@ export function useAuthedImageFallback(src: string | undefined, onFailure?: () =
|
||||
}, [])
|
||||
|
||||
const onError = useCallback(() => {
|
||||
if (!src || triedSource.current === src) {
|
||||
if (attempt.current !== current || current.state === 'fetching' || current.state === 'failed') return
|
||||
if (!src || current.state === 'resolved') {
|
||||
current.state = 'failed'
|
||||
onFailureRef.current?.()
|
||||
return
|
||||
}
|
||||
triedSource.current = src
|
||||
void fetchServerImageBlobUrl(src).then((url) => {
|
||||
if (!alive.current) {
|
||||
current.state = 'fetching'
|
||||
void fetchServerImageBlobUrl(src, retryWithCredential).then((url) => {
|
||||
if (!alive.current || attempt.current !== current) {
|
||||
URL.revokeObjectURL(url)
|
||||
return
|
||||
}
|
||||
current.state = 'resolved'
|
||||
objectUrls.current.push(url)
|
||||
setResolved({ source: src, url })
|
||||
setResolved({ attempt: current, url })
|
||||
}).catch(() => {
|
||||
if (alive.current && triedSource.current === src) onFailureRef.current?.()
|
||||
if (alive.current && attempt.current === current) {
|
||||
current.state = 'failed'
|
||||
onFailureRef.current?.()
|
||||
}
|
||||
})
|
||||
}, [src])
|
||||
}, [src, current, retryWithCredential])
|
||||
|
||||
return { src: resolved && resolved.source === src ? resolved.url : src, onError }
|
||||
useEffect(() => {
|
||||
if (retryWithCredential) onError()
|
||||
}, [retryWithCredential, onError])
|
||||
|
||||
return { src: resolved?.attempt === current ? resolved.url : retryWithCredential ? undefined : src, onError }
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user