{{ message }}
fix(js-sdk): normalize pollInterval to prevent silent 83-min hang (ms/s double-conversion)#3382
Open
mad1081 wants to merge 1 commit intofirecrawl:mainfrom
Open
Conversation
…s/s double-conversion Fixes firecrawl#3373 — firecrawl crawl --wait (without --progress) hangs ~83 minutes per poll because the CLI converts --poll-interval from seconds to milliseconds before calling app.crawl(), while the SDK polling functions also multiply by 1000 (they expect seconds). With --poll-interval 5 this produces 5 x 1000 x 1000 = 5,000,000 ms = 83 min per cycle. Add a normalization guard at the top of every polling function: if pollInterval >= 1000 it was almost certainly passed in milliseconds and is divided by 1000 before use. No legitimate use case polls every 1000+ seconds so the threshold has no false positives for normal callers. Affected: waitForCrawlCompletion, waitForBatchCompletion, monitorJobStatus
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/js-sdk/firecrawl/src/v1/index.ts">
<violation number="1" location="apps/js-sdk/firecrawl/src/v1/index.ts:1515">
P2: Heuristic misclassifies valid large second-based intervals as milliseconds, silently collapsing them to the 2s minimum polling interval.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
Contributor
There was a problem hiding this comment.
P2: Heuristic misclassifies valid large second-based intervals as milliseconds, silently collapsing them to the 2s minimum polling interval.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/js-sdk/firecrawl/src/v1/index.ts, line 1515:
<comment>Heuristic misclassifies valid large second-based intervals as milliseconds, silently collapsing them to the 2s minimum polling interval.</comment>
<file context>
@@ -1510,6 +1510,9 @@ export default class FirecrawlApp {
) {
+ // Normalize: if checkInterval looks like milliseconds (>= 1000), convert to seconds.
+ // This prevents a silent 1000x hang when callers accidentally pass ms instead of s.
+ if (checkInterval >= 1000) checkInterval = checkInterval / 1000;
checkInterval = Math.max(checkInterval, 2);
await new Promise((resolve) =>
</file context>
Author
There was a problem hiding this comment.
Valid point. In practice no polling use case requires ≥1000s intervals — but happy to fix the root cause in the CLI instead if that's preferred.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #3373 —
firecrawl crawl --wait(without--progress) silently hangs ~83 minutes per poll cycle because of a double ms/s conversion between the CLI and the SDK.The CLI converts
--poll-interval 5→5000(ms) before passing toapp.crawl(). The SDK's polling functions then multiply by 1000 again (they expect seconds), producing5000 × 1000 = 5,000,000 ms ≈ 83 minper poll. The crawl completes server-side and credits are consumed, but the CLI never returns.Fix
Add a normalization guard at the entry of every SDK polling function: if
pollInterval >= 1000, treat it as milliseconds and divide by 1000 before use. No legitimate polling use case requires intervals of 1000+ seconds (16+ minutes), so the threshold has zero false positives for normal callers while silently correcting accidentally-large ms values.Three functions patched:
waitForCrawlCompletion—apps/js-sdk/firecrawl/src/v2/methods/crawl.tswaitForBatchCompletion—apps/js-sdk/firecrawl/src/v2/methods/batch.tsmonitorJobStatus—apps/js-sdk/firecrawl/src/v1/index.tsRelevant prior art & context
waitForBatchCompletionandwaitForCrawlCompletion; the review there called out the need to distinguish retryable vs non-retryable errors in the same polling loop we're touching here.monitorJobStatus) — last significant change tomonitorJobStatus; added theisRetryableErrorpattern and theMath.max(checkInterval, 2)floor that our normalization guard sits above.sleep()in all poll loops" as a mandatory fix before merge. That review established the cross-SDK precedent that poll interval inputs must be validated/normalized, which is exactly what this PR does for the JS SDK.Test plan
firecrawl crawl <url> --limit 1 --wait --poll-interval 5completes in seconds (not 83 min)firecrawl crawl <url> --limit 1 --wait --poll-interval 5000(ms accidentally passed) also completes correctly (normalized to 5s)pollInterval: 2) are unaffectedbatchScrapewith an ms-sizedpollIntervalalso normalizes correctlySummary by cubic
Prevent 83‑minute hangs by normalizing polling intervals in the JS SDK to handle ms vs s correctly. Fixes #3373 so
firecrawl crawl --waitpolls at the intended frequency and returns promptly.pollInterval/checkInterval: if ≥ 1000, treat as milliseconds and divide by 1000 before sleeping.waitForCrawlCompletion(v2),waitForBatchCompletion(v2), andmonitorJobStatus(v1).5000now behave as5s.Written for commit ffd1cce. Summary will update on new commits.