Fix: double conversion of pollInterval and timeout in --wait mode - #99
Fix: double conversion of pollInterval and timeout in --wait mode#99shiva-manu wants to merge 1 commit into
Conversation
Sorry, something went wrong.
There was a problem hiding this comment.
Pull request overview
Fixes an issue in the CLI crawl --wait flow where pollInterval and timeout were being converted to milliseconds before calling the SDK, leading to an unintended “double conversion” and extremely long polling intervals.
Changes:
- Pass pollInterval and timeout to the SDK in seconds (no pre-conversion in CLI).
- Keep milliseconds conversion only for the CLI’s internal --progress polling loop.
- Tighten types for the constructed crawlOptions object.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sorry, something went wrong.
| const timeoutMs = | ||
| crawlOptions.timeout !== undefined | ||
| ? crawlOptions.timeout * 1000 | ||
| : undefined; |
Copilot
AI
Apr 16, 2026
There was a problem hiding this comment.
timeoutMs is computed using an explicit undefined check, but the timeout enforcement later in this function still relies on a truthy check (if (timeoutMs && ...)). As a result, a valid timeout value of 0 would be treated as “no timeout”. Consider keeping the explicit undefined semantics end-to-end by checking timeoutMs !== undefined where the timeout is enforced.
Sorry, something went wrong.
| // Set polling options (SDK expects seconds, not ms) | ||
| if (pollInterval !== undefined) { | ||
| crawlOptions.pollInterval = pollInterval * 1000; // Convert to milliseconds | ||
| crawlOptions.pollInterval = pollInterval; // seconds | ||
| } else { | ||
| // Default poll interval: 5 seconds | ||
| crawlOptions.pollInterval = 5000; | ||
| crawlOptions.pollInterval = 5; | ||
| } | ||
| if (timeout !== undefined) { | ||
| crawlOptions.timeout = timeout * 1000; // Convert to milliseconds | ||
| crawlOptions.timeout = timeout; // seconds | ||
| } |
Copilot
AI
Apr 16, 2026
There was a problem hiding this comment.
This change switches pollInterval/timeout passed to the SDK from milliseconds to seconds (default pollInterval is now 5, not 5000). The existing unit tests for executeCrawl currently assert millisecond values (e.g., pollInterval: 5000, timeout: 300000) and will need to be updated to match the new units so CI stays green.
Sorry, something went wrong.
Fix: Double conversion of pollInterval and timeout in --wait mode
Summary
Fixes an issue where pollInterval and timeout were incorrectly converted to milliseconds in the CLI before being passed to the SDK, which also performs its own conversion.
This resulted in extremely long polling intervals (e.g., 5s → ~83 minutes), causing firecrawl crawl --wait to appear stuck while still consuming credits server-side.
Root Cause
Result:
5 → 5000 → 5,000,000 ms (~83 minutes)
Changes
Impact
Verification