Show an error when the browser login cannot reach the web host - #231
Conversation
pollAuthStatus returned null for a user who had not authorised yet, for an HTTP error response and for a transport failure. waitForAuth read every null as "not yet" and polled again, so a blocked host made the CLI spin silently for the full 5 minute timeout. pollAuthStatus now returns one of five outcomes: pending, complete, server-busy, server-error and unreachable. waitForAuth keeps polling on pending. It gives up after 3 transport failures and prints the host, the underlying cause and the --web-url override. It gives up after 5 retryable server errors (429 and 5xx). It stops at once on any other HTTP status, such as 410 for an expired session. All three exit with code 1 through the login command. Each request now carries a 10 second timeout, so a connection that never answers cannot stall the poll. Set FIRECRAWL_DEBUG to log every failed attempt. The default web URL moves from the apex to www.firecrawl.dev. The apex redirects to www, and egress allowlists often permit only www. Polling www removes the redirect and the blocked apex. login.ts now imports the one constant instead of keeping a second copy. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BA2iV5PoGdGEdxJa2TwUEr
|
@cubic-dev-ai review this PR No automatic review landed on commit 45fd99a, so this is a manual request. Generated by Claude Code |
@claude[bot] I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 4 files
Shadow auto-approve: would not auto-approve because issues were found.
Fix all with cubic | Re-trigger cubic
waitForAuth kept both failure counters across different outcomes, so a host that alternated between a transport failure and a 429 could reach the transport budget after three failures that were not consecutive. Each branch now clears the other counter, which is what pending already did for both. The poll test suite used the production host as its web URL, so a code path that ignored the --web-url override still passed. It now uses test-host.example and asserts the request URL for both pollAuthStatus and waitForAuth. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BA2iV5PoGdGEdxJa2TwUEr
The transport and server budgets count consecutive failures. Nothing failed if a counter survived the other outcome, so the reset in each branch was untested. The case alternates a dead transport with a rate limit five times, which exhausts either budget when its counter is not cleared, then completes. Removing either reset fails it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BA2iV5PoGdGEdxJa2TwUEr
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would require human review. Fixes browser-login hang by distinguishing pending, server-error, and unreachable polls and failing fast; also changes default URL to www.firecrawl.dev. Decisive: new retry/timeout budgets and the default-host change rely on network behavior the author couldn't verify.
Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would require human review. Improves browser-login error handling with categorized poll outcomes, retries, and request timeouts, while changing the default web host. Human approval is needed for the new operational budgets and the unverified redirect/host behavior.
Re-trigger cubic
The override test alone left the production default host uncovered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VGBDZtB3mFP8PFu6RwRYX7
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would require human review. Improves browser-login error handling by distinguishing pending, server, and transport failures, with new retry/timeout budgets and a default host change to www.firecrawl.dev. Human sign-off is needed for the operational retry/timeout policy and the unverified redirect/host change.
Re-trigger cubic
Fixes the silent infinite hang in
firecrawl login --method browser.Asana: https://app.asana.com/1/1208514492316817/project/1218294299996935/task/1218519993225348
Before:
pollAuthStatus()returnednullthree different ways. It returnednullwhen the user had not clicked authorise yet, it returnednullwhen the server answered with an HTTP error, and it returnednullwhen the request never reached the server at all.waitForAuth()read everynullas "not yet" and polled again. On a network that blocks the host, the CLI printedWaiting for browser authentication...and nothing else for the full 5 minute timeout. The user could not tell a slow browser from a dead connection, and no message named the host or the reason. The default host made this easy to hit: the CLI polled the apexfirecrawl.dev, which sandboxed and corporate egress allowlists commonly deny while they permitwww.After: the three cases are separate. A user who has not authorised yet still polls every 2 seconds up to the same 5 minute timeout, unchanged. A host the CLI cannot reach fails after 3 attempts, about 6 seconds, with a message that names the host, the underlying network error, and the
--web-urloverride. A server that refuses the session, such as410 Session expired, fails on the first response. All of these exit with code 1 instead of spinning. The default host is nowwww.firecrawl.dev.Running the old build against a host it cannot reach:
The same command on this branch:
Also reproduced against the real blocked apex through an egress proxy, which is the case in the ticket.
How
pollAuthStatus()returns aPollAuthResultunion instead ofnull:pending,complete,server-busy,server-error,unreachable. Node reports DNS, TLS and proxy failures as a barefetch failedTypeError and puts the real reason oncause, so the code unwrapscauseand itscodefor the message.waitForAuth()acts on each case:pending200 {status:'pending'}complete200with an API keyunreachablefetchthrewserver-busy429or5xxserver-errorThe rate limit needs its own class: the server allows 30 polls per minute and the CLI polls every 2 seconds, so an occasional
429is normal and must not end the login.Each request carries a 10 second
AbortSignal.timeout, so a connection that accepts and then never answers cannot stall the poll. SetFIRECRAWL_DEBUGto log every failed attempt as it happens.On the default host
The ticket suggested defaulting to
www. I checked the redirect direction first, because a default that redirects into a blocked host fixes nothing. The apex redirects towww, not the other way round: a fetch ofhttps://firecrawl.dev/robots.txtlands onhttps://www.firecrawl.dev/robots.txt, and that file declaresHost: https://www.firecrawl.devwithwwwsitemaps.getURL()in firecrawl-web falls back tohttps://www.firecrawl.dev/as well, though several API routes there still fall back to the apex.So
wwwis the redirect target, and pointing the CLI at it removes a hop rather than adding one. That also removes the POST rewrite hazard the ticket raised, since there is no redirect left to follow.I could not verify the redirect status code from this environment, because the egress policy here denies both hosts. If that redirect is a
301or302rather than a307or308, then today's apex default has been silently converting the poll POST to a GET and dropping the body. That is worth a check on a normal network. The error handling above does not depend on the answer either way.src/commands/login.tskept its own copy of the default URL, which is how a single change would have missed one site. It now imports the constant fromsrc/utils/auth.ts.Tests
src/__tests__/utils/auth-poll.test.ts, 12 cases in vitest, the framework this repo already uses. The one that matters asserts that a network failure rejects with a message naming the host after 3 attempts, instead of polling on. The rest cover the pending loop still reaching the existing timeout, a transient429riding through to a successful login, an expired session stopping at once, and the underlyingcausereaching the message.Full CI locally:
format:check,type-check,build, andtest(461 tests) all pass.Not in this PR
🤖 Generated with Claude Code
https://claude.ai/code/session_01BA2iV5PoGdGEdxJa2TwUEr
Generated by Claude Code
Summary by cubic
Fixes the silent 5-minute hang in
firecrawl login --method browserwhen the CLI cannot reach the web host, so users now get a fast, actionable error instead of an endless "Waiting for browser authentication..." spinner.Bug Fixes
pollAuthStatus()now reports distinct outcomes for pending, complete, transiently busy, refused, and unreachable, instead of collapsing them all intonull.--web-urloverride; refusals like410 Session expiredfail on the first response.429/5xxresponses get up to 5 consecutive attempts; each failure type tracks its own counter and clears the other's (pending resets both), so alternating failures cannot accumulate.AbortSignal.timeoutso a stalled connection cannot hang the poll; the 2-second interval and 5-minute pending timeout are unchanged.www.firecrawl.dev(the apex redirects there and egress allowlists commonly deny it), andlogin.tsimports the sharedWEB_URLconstant instead of a duplicate copy.test-host.example) so the--web-urloverride is covered, and also assert the endpoint built from the production default host, including alternating failure types.Written for commit 2265c8e. Summary will update on new commits.