Skip to content

Commit 057ca4e

Browse files
committed
fix(firecrawl): correct map's declared shape, a crash path, and a timeout race
map declared links as an array of strings, but v2 returns MapDocument objects {url, title?, description?} -- so every consumer reading <firecrawl.links> as URLs got [object Object]. Same declared-vs-real class this branch fixed for search, one file over. scrape's transformResponse read data.data.markdown unguarded. Firecrawl's error bodies carry no data key, so any non-happy path threw a TypeError instead of surfacing the error; every sibling tool already guards. The country description claimed an unconditional "US" default. The schema applies it only when location is unset. sources and categories were unconstrained strings against a strict-object schema, so an LLM emitting an invalid value got a hard 400. Constrained with const unions, verified to survive into the model-visible schema. Adds the 'developer' category, which is a live enum member with its own aliases. timeout shadowed the transport's reserved param on the three external tools. The units agree, so this looked safe -- but the 30s headroom the executor adds applies only to internal routes, and the external branch passes the deadline bare. Sim's clock starts earlier, so Sim always won the race and the user never saw Firecrawl's structured 408. parse is internal-route and keeps the old name. The subBlock's condition only hid the field, so a stale value still reached the transport on operations that never declared it; the mapper now clears it once.
1 parent 1370c43 commit 057ca4e

8 files changed

Lines changed: 341 additions & 28 deletions

File tree

apps/sim/blocks/blocks/firecrawl.ts

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -527,7 +527,20 @@ Example 2 - Product Data:
527527
jobId,
528528
} = params
529529

530-
const result: Record<string, any> = { apiKey }
530+
/**
531+
* `timeout` is reserved by the tool request transport as the outbound fetch
532+
* deadline. On the external branch it is passed through bare, arming Sim's
533+
* abort at the exact budget Firecrawl was given, on a clock that starts
534+
* earlier — so Sim always wins and the caller loses Firecrawl's structured
535+
* 408. Clear it for every operation; the external tools carry the value as
536+
* `firecrawlTimeout` instead, while `parse` posts to an internal route that
537+
* wants the deadline propagated and sets it back below.
538+
*
539+
* Clearing it here rather than per-case also covers the operations whose
540+
* `timeout` subBlock is hidden by `condition`: the saved value survives an
541+
* operation switch and would otherwise still reach the transport.
542+
*/
543+
const result: Record<string, any> = { apiKey, timeout: undefined }
531544

532545
switch (operation) {
533546
case 'scrape':
@@ -544,15 +557,15 @@ Example 2 - Product Data:
544557
}
545558
}
546559
}
547-
if (timeout) result.timeout = Number.parseInt(timeout)
560+
if (timeout) result.firecrawlTimeout = Number.parseInt(timeout)
548561
if (waitFor) result.waitFor = Number.parseInt(waitFor)
549562
if (onlyMainContent != null) result.onlyMainContent = onlyMainContent
550563
if (mobile != null) result.mobile = mobile
551564
break
552565

553566
case 'search':
554567
if (query) result.query = query
555-
if (timeout) result.timeout = Number.parseInt(timeout)
568+
if (timeout) result.firecrawlTimeout = Number.parseInt(timeout)
556569
if (limit) result.limit = Number.parseInt(limit)
557570
if (params.ignoreInvalidURLs != null) {
558571
result.ignoreInvalidURLs = params.ignoreInvalidURLs
@@ -795,7 +808,11 @@ Example 2 - Product Data:
795808
invalidURLs: { type: 'json', description: 'URLs skipped because they were invalid' },
796809
// Map output
797810
success: { type: 'boolean', description: 'Operation success status' },
798-
links: { type: 'json', description: 'Discovered URLs array' },
811+
links: {
812+
type: 'json',
813+
description:
814+
'Discovered links, each an object with `url` and optional `title`/`description`. Reference `<firecrawl.links>[i].url` for the address — the entry itself is no longer a bare URL string.',
815+
},
799816
// Extract output
800817
sources: { type: 'json', description: 'Data sources array' },
801818
tokensUsed: { type: 'number', description: 'Tokens consumed by the extract job' },
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { describe, expect, it } from 'vitest'
5+
import { mapTool } from '@/tools/firecrawl/map'
6+
import type { MapParams } from '@/tools/firecrawl/types'
7+
8+
const jsonOk = (body: unknown) =>
9+
new Response(JSON.stringify(body), {
10+
status: 200,
11+
headers: { 'Content-Type': 'application/json' },
12+
})
13+
14+
const mapParams: MapParams = { apiKey: 'test-key', url: 'https://example.com' }
15+
16+
const resolveBody = (params: MapParams): Record<string, unknown> =>
17+
mapTool.request.body!(params) as Record<string, unknown>
18+
19+
describe('firecrawl map declared links shape', () => {
20+
it('declares links as MapDocument objects, not bare URL strings', () => {
21+
const links = mapTool.outputs.links as {
22+
type: string
23+
items?: { type?: string; properties?: Record<string, unknown> }
24+
}
25+
26+
expect(links.type).toBe('array')
27+
expect(links.items?.type).toBe('object')
28+
expect(Object.keys(links.items?.properties ?? {}).sort()).toEqual([
29+
'description',
30+
'title',
31+
'url',
32+
])
33+
})
34+
35+
it('passes the object-shaped links Firecrawl v2 actually returns straight through', async () => {
36+
const response = await mapTool.transformResponse!(
37+
jsonOk({
38+
success: true,
39+
id: 'map-1',
40+
links: [
41+
{ url: 'https://example.com/a', title: 'A', description: 'first' },
42+
{ url: 'https://example.com/b' },
43+
],
44+
}),
45+
mapParams
46+
)
47+
48+
expect(response.output.links).toEqual([
49+
{ url: 'https://example.com/a', title: 'A', description: 'first' },
50+
{ url: 'https://example.com/b' },
51+
])
52+
expect(response.output.links[0].url).toBe('https://example.com/a')
53+
})
54+
})
55+
56+
describe('firecrawl map timeout is not the transport deadline', () => {
57+
it('does not declare the transport-reserved `timeout` param', () => {
58+
expect(mapTool.params.timeout).toBeUndefined()
59+
expect(mapTool.params.firecrawlTimeout).toBeDefined()
60+
})
61+
62+
it('maps firecrawlTimeout onto the request body as Firecrawl `timeout`', () => {
63+
const body = resolveBody({ ...mapParams, firecrawlTimeout: 45000 })
64+
65+
expect(body.timeout).toBe(45000)
66+
})
67+
})

apps/sim/tools/firecrawl/map.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { firecrawlHosting } from '@/tools/firecrawl/hosting'
22
import type { MapParams, MapResponse } from '@/tools/firecrawl/types'
3+
import { MAP_DOCUMENT_OUTPUT_PROPERTIES } from '@/tools/firecrawl/types'
34
import type { ToolConfig } from '@/tools/types'
45

56
export const mapTool: ToolConfig<MapParams, MapResponse> = {
@@ -47,11 +48,12 @@ export const mapTool: ToolConfig<MapParams, MapResponse> = {
4748
description:
4849
'Maximum number of links to return (e.g., 100, 1000, 5000). Max: 100,000, default: 5,000',
4950
},
50-
timeout: {
51+
firecrawlTimeout: {
5152
type: 'number',
5253
required: false,
5354
visibility: 'user-only',
54-
description: 'Request timeout in milliseconds',
55+
description:
56+
"How long Firecrawl may spend on the map, in milliseconds. Sent as `timeout` in the request body; it does not bound Sim's own transport deadline.",
5557
},
5658
location: {
5759
type: 'json',
@@ -88,7 +90,7 @@ export const mapTool: ToolConfig<MapParams, MapResponse> = {
8890
if (typeof params.ignoreQueryParameters === 'boolean')
8991
body.ignoreQueryParameters = params.ignoreQueryParameters
9092
if (params.limit) body.limit = Number(params.limit)
91-
if (params.timeout) body.timeout = Number(params.timeout)
93+
if (params.firecrawlTimeout) body.timeout = Number(params.firecrawlTimeout)
9294
if (params.location) body.location = params.location
9395

9496
return body
@@ -115,9 +117,11 @@ export const mapTool: ToolConfig<MapParams, MapResponse> = {
115117
},
116118
links: {
117119
type: 'array',
118-
description: 'Array of discovered URLs from the website',
120+
description:
121+
'Discovered links. Each entry is an object with `url` and optional `title`/`description` — read `<firecrawl.links>[i].url` for the address, not the entry itself.',
119122
items: {
120-
type: 'string',
123+
type: 'object',
124+
properties: MAP_DOCUMENT_OUTPUT_PROPERTIES,
121125
},
122126
},
123127
},
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { describe, expect, it } from 'vitest'
5+
import { scrapeTool } from '@/tools/firecrawl/scrape'
6+
import type { ScrapeParams } from '@/tools/firecrawl/types'
7+
8+
const json = (body: unknown, status: number) =>
9+
new Response(JSON.stringify(body), {
10+
status,
11+
headers: { 'Content-Type': 'application/json' },
12+
})
13+
14+
const scrapeParams: ScrapeParams = { apiKey: 'test-key', url: 'https://example.com' }
15+
16+
const resolveBody = (params: ScrapeParams): Record<string, unknown> =>
17+
scrapeTool.request.body!(params) as Record<string, unknown>
18+
19+
describe('firecrawl scrape error handling', () => {
20+
it('does not throw on a Firecrawl error body that carries no `data` key', async () => {
21+
const response = await scrapeTool.transformResponse!(
22+
json({ success: false, error: 'Request timed out' }, 408),
23+
scrapeParams
24+
)
25+
26+
expect(response.output.markdown).toBeUndefined()
27+
expect(response.output.metadata).toBeUndefined()
28+
})
29+
30+
it('still projects the document on a successful response', async () => {
31+
const response = await scrapeTool.transformResponse!(
32+
json(
33+
{
34+
success: true,
35+
data: {
36+
markdown: '# Hi',
37+
html: '<h1>Hi</h1>',
38+
metadata: { title: 'Hi', sourceURL: 'https://example.com', statusCode: 200 },
39+
},
40+
creditsUsed: 1,
41+
},
42+
200
43+
),
44+
scrapeParams
45+
)
46+
47+
expect(response.output.markdown).toBe('# Hi')
48+
expect(response.output.html).toBe('<h1>Hi</h1>')
49+
expect(response.output.metadata.title).toBe('Hi')
50+
expect(response.output.creditsUsed).toBe(1)
51+
})
52+
})
53+
54+
describe('firecrawl scrape timeout is not the transport deadline', () => {
55+
it('does not declare the transport-reserved `timeout` param', () => {
56+
expect(scrapeTool.params.timeout).toBeUndefined()
57+
expect(scrapeTool.params.firecrawlTimeout).toBeDefined()
58+
})
59+
60+
it('maps firecrawlTimeout onto the request body as Firecrawl `timeout`', () => {
61+
const body = resolveBody({ ...scrapeParams, firecrawlTimeout: 45000 })
62+
63+
expect(body.timeout).toBe(45000)
64+
})
65+
})

apps/sim/tools/firecrawl/scrape.ts

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,13 @@ export const scrapeTool: ToolConfig<ScrapeParams, ScrapeResponse> = {
3636
visibility: 'hidden',
3737
description: 'Options for content scraping',
3838
},
39+
firecrawlTimeout: {
40+
type: 'number',
41+
required: false,
42+
visibility: 'user-only',
43+
description:
44+
"How long Firecrawl may spend on the scrape, in milliseconds. Sent as `timeout` in the request body; it does not bound Sim's own transport deadline.",
45+
},
3946
apiKey: {
4047
type: 'string',
4148
required: true,
@@ -91,7 +98,7 @@ export const scrapeTool: ToolConfig<ScrapeParams, ScrapeResponse> = {
9198
if (typeof params.mobile === 'boolean') body.mobile = params.mobile
9299
if (typeof params.skipTlsVerification === 'boolean')
93100
body.skipTlsVerification = params.skipTlsVerification
94-
if (params.timeout) body.timeout = Number(params.timeout)
101+
if (params.firecrawlTimeout) body.timeout = Number(params.firecrawlTimeout)
95102
if (params.parsers) body.parsers = params.parsers
96103
if (params.actions) body.actions = params.actions
97104
if (params.location) body.location = params.location
@@ -113,13 +120,14 @@ export const scrapeTool: ToolConfig<ScrapeParams, ScrapeResponse> = {
113120

114121
transformResponse: async (response: Response) => {
115122
const data = await response.json()
123+
const document = data.data ?? {}
116124

117125
return {
118126
success: true,
119127
output: {
120-
markdown: data.data.markdown,
121-
html: data.data.html,
122-
metadata: data.data.metadata,
128+
markdown: document.markdown,
129+
html: document.html,
130+
metadata: document.metadata,
123131
creditsUsed: data.creditsUsed,
124132
},
125133
}

0 commit comments

Comments
 (0)