Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: added timeouts to requests to prevent blocking requests #890

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

RRua
Copy link

@RRua RRua commented Nov 11, 2024

Pull Request: Add Optional Timeout Parameters to HTTP Requests

Summary

This pull request introduces optional timeout parameters to all functions in the FirecrawlApp class that make HTTP requests. This enhancement improves the robustness and resilience of the application by ensuring that calls do not hang indefinitely, which helps prevent resource starvation and supports better error handling.

Key Changes

  • Added timeout as an optional argument to the following functions:

    • scrape_url
    • crawl_url
    • async_crawl_url
    • check_crawl_status
    • ....
  • Updated internal _post_request and _get_request methods to use the timeout value when provided.

  • Default behavior remains unchanged if timeout is not specified, ensuring backward compatibility.

Benefits

  • Prevents indefinite blocking on slow or unresponsive network requests.
  • Enhances system reliability and resilience by managing resource utilization more effectively.
  • Offers users the flexibility to customize timeout settings based on their specific needs.

Copy link
Member

@mogery mogery left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@nickscamara
Copy link
Member

  • Update the docs
  • Maybe update the name to not conflict with params.timeout

@nickscamara nickscamara self-requested a review December 11, 2024 23:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants