Skip to content

Commit 06983cd

Browse files
committed
fix(chat): preserve link contrast and bound preview retries
1 parent 78f7955 commit 06983cd

11 files changed

Lines changed: 168 additions & 18 deletions

File tree

‎apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/source-link.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ export function linkSiteName(url: string, siteName?: string | null): string {
2424
* A prose link shares the platform blue and retains a visible keyboard focus outline.
2525
*/
2626
export const PROSE_LINK_CLASS =
27-
'not-prose [&_strong]:text-inherit [&_em]:text-inherit [&_code]:text-inherit [&_del]:text-inherit text-[var(--brand-blue)] no-underline hover-hover:text-[var(--brand-secondary)] focus-visible:outline focus-visible:outline-2 focus-visible:outline-[var(--text-primary)]'
27+
'not-prose [&_strong]:text-inherit [&_em]:text-inherit [&_code]:text-inherit [&_del]:text-inherit text-[var(--brand-blue)] no-underline focus-visible:outline focus-visible:outline-2 focus-visible:outline-[var(--text-primary)]'
2828

2929
/**
3030
* In the desktop app, a plain click diverts into the embedded Sim browser

‎apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/source-preview.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ function SourcePreviewContent({ source }: Pick<SourcePreviewProps, 'source'>) {
119119
source.snippet?.trim() || (!source.connectorType && preview?.description?.trim())
120120
return (
121121
<div className='flex flex-col gap-3 p-1.5'>
122-
<div className='flex min-w-0 items-center justify-between gap-3 text-[var(--text-muted)] text-caption'>
122+
<div className='flex min-w-0 items-center justify-between gap-3 text-[var(--text-tertiary)] text-caption'>
123123
<span className='flex min-w-0 items-center gap-2'>
124124
<SourceIcon source={source} />
125125
<OverflowText label={siteName} tooltipEnabled={false} />

‎apps/sim/lib/core/errors/retryable-infrastructure.test.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,17 @@ describe('isRetryableInfrastructureError', () => {
4141
})
4242
})
4343

44+
it.each(['DNS_TIMEOUT', 'EAI_AGAIN'])('recognizes transient DNS errors: %s', (code) => {
45+
expect(
46+
isRetryableInfrastructureError(new Error('DNS lookup failed', { cause: errorWithCode(code) }))
47+
).toBe(true)
48+
})
49+
50+
it('does not retry permanent DNS or destination-policy failures', () => {
51+
expect(isRetryableInfrastructureError(errorWithCode('ENOTFOUND'))).toBe(false)
52+
expect(isRetryableInfrastructureError(new Error('Destination blocked'))).toBe(false)
53+
})
54+
4455
it('does not classify semantic SQL errors as retryable', () => {
4556
expect(isRetryableInfrastructureError(errorWithCode('42703'))).toBe(false)
4657
expect(isRetryableInfrastructureError(new Error('workflow not found'))).toBe(false)

‎apps/sim/lib/core/errors/retryable-infrastructure.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ const RETRYABLE_DB_ERROR_CODES = new Set([
1616
])
1717

1818
const RETRYABLE_NETWORK_ERROR_CODES = new Set([
19+
'DNS_TIMEOUT',
20+
'EAI_AGAIN',
1921
'ETIMEDOUT',
2022
'ECONNRESET',
2123
'ECONNREFUSED',

‎apps/sim/lib/core/security/egress/validate.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ export interface EgressValidationSuccess {
3838
export interface EgressValidationFailure {
3939
readonly isValid: false
4040
readonly error: string
41+
/** Retains resolver failure codes so callers can distinguish an outage from a policy denial. */
42+
readonly cause?: unknown
4143
}
4244

4345
export type EgressValidationResult = EgressValidationSuccess | EgressValidationFailure
@@ -125,7 +127,7 @@ export async function validateEgressUrl(
125127
? { profile, paramName }
126128
: { profile, paramName, host, error: toError(error).message }
127129
)
128-
return { isValid: false, error: `${paramName} hostname could not be resolved` }
130+
return { isValid: false, error: `${paramName} hostname could not be resolved`, cause: error }
129131
}
130132

131133
// Refused records are filtered rather than failing the whole host: pinning to a

‎apps/sim/lib/core/security/input-validation.server.test.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,16 @@ describe('validateUrlWithDNS address classification', () => {
128128
).toBe(false)
129129
})
130130

131+
it('retains resolver causes for retry classification without admitting the request', async () => {
132+
const cause = Object.assign(new Error('Temporary DNS failure'), { code: 'EAI_AGAIN' })
133+
mockResolve.mockRejectedValue(cause)
134+
await expect(
135+
secureFetchWithValidation('https://example.com/preview', {
136+
profile: 'contentFetch',
137+
})
138+
).rejects.toMatchObject({ message: 'url hostname could not be resolved', cause })
139+
})
140+
131141
it('can conceal credential-derived host details in validation logs', async () => {
132142
mockResolve.mockRejectedValue(new Error('DNS failure with credential-host-canary'))
133143

‎apps/sim/lib/core/security/input-validation.server.ts‎

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,13 @@ const logger = createLogger('InputValidation')
4141
*/
4242
export type AsyncValidationResult =
4343
| { isValid: true; resolvedIP: string; originalHostname: string; error?: undefined }
44-
| { isValid: false; error: string; resolvedIP?: undefined; originalHostname?: undefined }
44+
| {
45+
isValid: false
46+
error: string
47+
cause?: unknown
48+
resolvedIP?: undefined
49+
originalHostname?: undefined
50+
}
4551

4652
/**
4753
* Validates a URL, resolves its DNS, and returns the address to pin.
@@ -65,7 +71,7 @@ export async function validateUrlWithDNS(
6571
const result = await validateEgressUrl(url, paramName, profile, options)
6672
return result.isValid
6773
? { isValid: true, resolvedIP: result.resolvedIP, originalHostname: result.originalHostname }
68-
: { isValid: false, error: result.error }
74+
: { isValid: false, error: result.error, cause: result.cause }
6975
}
7076

7177
/**
@@ -1215,7 +1221,9 @@ export async function secureFetchWithPinnedIP(
12151221
})
12161222
.then((validation) => {
12171223
if (!validation.isValid) {
1218-
settledReject(new Error(`Redirect blocked: ${validation.error}`))
1224+
settledReject(
1225+
new Error(`Redirect blocked: ${validation.error}`, { cause: validation.cause })
1226+
)
12191227
return
12201228
}
12211229
const redirectPolicy = options.redirectPolicy
@@ -1516,7 +1524,11 @@ export async function secureFetchWithPinnedIP(
15161524
req.on('error', settledReject)
15171525
req.on('timeout', () => {
15181526
destroyRequest()
1519-
settledReject(new Error(`Request timed out after ${requestOptions.timeout}ms`))
1527+
settledReject(
1528+
Object.assign(new Error(`Request timed out after ${requestOptions.timeout}ms`), {
1529+
code: 'ETIMEDOUT',
1530+
})
1531+
)
15201532
})
15211533
send = () => {
15221534
req.end(options.body)
@@ -1560,7 +1572,7 @@ export async function secureFetchWithValidation(
15601572
signal: options.signal,
15611573
})
15621574
if (!validation.isValid) {
1563-
throw new Error(validation.error)
1575+
throw new Error(validation.error, { cause: validation.cause })
15641576
}
15651577
return secureFetchWithPinnedIP(url, validation.resolvedIP, options)
15661578
}

‎apps/sim/lib/core/security/pinned-redirect-replay.server.test.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
*/
77
import http from 'node:http'
88
import type { AddressInfo } from 'node:net'
9+
import { resolveHostAddresses } from '@sim/security/dns'
910
import { afterEach, describe, expect, it, vi } from 'vitest'
1011

1112
vi.mock('@sim/security/dns', () => ({
@@ -58,6 +59,20 @@ async function startRecordingServer(hops: RecordedHop[]): Promise<string> {
5859
}
5960

6061
describe('secureFetchWithPinnedIP redirect replay', () => {
62+
it('preserves transient DNS failure causes on redirects', async () => {
63+
const cause = Object.assign(new Error('Temporary DNS failure'), { code: 'EAI_AGAIN' })
64+
vi.mocked(resolveHostAddresses).mockRejectedValueOnce(cause)
65+
const origin = await startServer((_req, res) => {
66+
res.writeHead(302, { location: 'https://example.com/next' })
67+
res.end()
68+
})
69+
await expect(
70+
secureFetchWithPinnedIP(origin, '127.0.0.1', {
71+
profile: 'contentFetch',
72+
})
73+
).rejects.toMatchObject({ cause })
74+
})
75+
6176
it('rejects a redirect target before following it', async () => {
6277
const hops: RecordedHop[] = []
6378
const target = await startRecordingServer(hops)

‎apps/sim/lib/core/security/secure-fetch-request-framing.server.test.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,16 @@ describe('secureFetchWithPinnedIP request framing', () => {
194194
expect(receivedHost).toBe(url.host)
195195
})
196196

197+
it('exposes a retryable code when the request times out before headers', async () => {
198+
const origin = await startServer((req) => req.resume())
199+
await expect(
200+
secureFetchWithPinnedIP(origin, '127.0.0.1', {
201+
timeout: 20,
202+
profile: 'configuredEndpoint',
203+
})
204+
).rejects.toMatchObject({ code: 'ETIMEDOUT' })
205+
})
206+
197207
it('cancels a framed request while waiting for response headers', async () => {
198208
const controller = new AbortController()
199209
const origin = await startServer((req) => {

‎apps/sim/lib/link-preview/fetch-preview.test.ts‎

Lines changed: 82 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ vi.mock('@/lib/core/security/input-validation.server', () => ({
77
secureFetchWithValidation: fetchMock,
88
}))
99

10+
import { PayloadSizeLimitError } from '@/lib/core/utils/stream-limits'
1011
import { fetchLinkPreview } from '@/lib/link-preview/fetch-preview'
1112

1213
const PAGE = 'https://example.com/docs/guide'
@@ -79,14 +80,92 @@ describe('public link preview images', () => {
7980
expect(fetchMock.mock.calls[1][0]).toBe('https://example.com/card.png')
8081
})
8182

82-
it('retains text metadata when the image is blocked by the network guard', async () => {
83-
fetchMock.mockResolvedValueOnce(page()).mockRejectedValueOnce(new Error('Private IP blocked'))
84-
expect(await fetchLinkPreview(PAGE)).toMatchObject({
83+
it.each([
84+
new Error('Private IP blocked'),
85+
new Error('Redirect blocked'),
86+
new PayloadSizeLimitError({ label: 'response body', maxBytes: 2 * 1024 * 1024 }),
87+
])('does not retry permanent image failures: %s', async (error) => {
88+
fetchMock.mockResolvedValueOnce(page()).mockRejectedValueOnce(error)
89+
expect(await fetchLinkPreview(PAGE)).toEqual({
8590
title: 'Guide',
8691
description: 'A useful guide',
92+
siteName: null,
8793
})
8894
})
8995

96+
it.each(['ECONNRESET', 'ETIMEDOUT', 'EAI_AGAIN', 'DNS_TIMEOUT'])(
97+
'retries transient fetch failures through their cause chain: %s',
98+
async (code) => {
99+
const cause = Object.assign(new Error('Upstream unavailable'), { code })
100+
fetchMock
101+
.mockResolvedValueOnce(page())
102+
.mockRejectedValueOnce(new Error('Fetch failed', { cause }))
103+
expect(await fetchLinkPreview(PAGE)).toMatchObject({ title: 'Guide', imageRetryable: true })
104+
}
105+
)
106+
107+
it('does not retry a malformed image reference', async () => {
108+
fetchMock.mockResolvedValueOnce(page('https://['))
109+
expect(await fetchLinkPreview(PAGE)).toEqual({
110+
title: 'Guide',
111+
description: 'A useful guide',
112+
siteName: null,
113+
})
114+
expect(fetchMock).toHaveBeenCalledTimes(1)
115+
})
116+
117+
it('does not retry malformed raster data', async () => {
118+
fetchMock
119+
.mockResolvedValueOnce(page())
120+
.mockResolvedValueOnce(
121+
new Response('not a PNG', { headers: { 'content-type': 'image/png' } })
122+
)
123+
expect(await fetchLinkPreview(PAGE)).toEqual({
124+
title: 'Guide',
125+
description: 'A useful guide',
126+
siteName: null,
127+
})
128+
})
129+
130+
it.each([
131+
{ status: 404, contentType: 'image/png', retryable: undefined },
132+
{ status: 429, contentType: 'image/png', retryable: true },
133+
{ status: 503, contentType: 'image/png', retryable: true },
134+
{ status: 200, contentType: 'image/svg+xml', retryable: undefined },
135+
])(
136+
'cancels rejected image bodies: $status $contentType',
137+
async ({ status, contentType, retryable }) => {
138+
const cancel = vi.fn()
139+
const response = new Response(new ReadableStream({ cancel }), {
140+
status,
141+
headers: { 'content-type': contentType },
142+
})
143+
const read = vi.spyOn(response, 'arrayBuffer')
144+
fetchMock.mockResolvedValueOnce(page()).mockResolvedValueOnce(response)
145+
const result = await fetchLinkPreview(PAGE)
146+
expect(result?.imageRetryable).toBe(retryable)
147+
expect(cancel).toHaveBeenCalledOnce()
148+
expect(read).not.toHaveBeenCalled()
149+
}
150+
)
151+
152+
it.each([
153+
{ status: 404, contentType: 'text/html' },
154+
{ status: 503, contentType: 'text/html' },
155+
{ status: 200, contentType: 'application/octet-stream' },
156+
])('cancels rejected page bodies: $status $contentType', async ({ status, contentType }) => {
157+
const cancel = vi.fn()
158+
const response = new Response(new ReadableStream({ cancel }), {
159+
status,
160+
headers: { 'content-type': contentType },
161+
})
162+
const read = vi.spyOn(response, 'text')
163+
fetchMock.mockResolvedValueOnce(response)
164+
expect(await fetchLinkPreview(PAGE)).toBeNull()
165+
expect(cancel).toHaveBeenCalledOnce()
166+
expect(read).not.toHaveBeenCalled()
167+
})
168+
90169
it('rejects SVG even when served with a raster content type', async () => {
91170
fetchMock
92171
.mockResolvedValueOnce(page())

0 commit comments

Comments
 (0)