Skip to content
This repository was archived by the owner on Nov 10, 2025. It is now read-only.

Conversation

@SHIXOOM
Copy link
Contributor

@SHIXOOM SHIXOOM commented Mar 8, 2025

FireCrawl API does not recognize sent parameters as this error describes:

HTTPError: Unexpected error during start crawl job: Status code 400. Bad Request -
[{'code': 'unrecognized_keys', 'keys': ['crawlerOptions', 'timeout'], 'path': [], 'message': 'Unrecognized key in body -- please review the v1 API documentation for request body changes'}]

Because FireCrawl API has been updated to v1. I updated the tool parameters to match v1 and updated their description in the ReadMe file

…does not recognize sent paramters (HTTPError: Unexpected error during start crawl job: Status code 400. Bad Request -

[{'code': 'unrecognized_keys', 'keys': ['crawlerOptions', 'timeout'], 'path': [], 'message': 'Unrecognized key in body -- please review the v1 API documentation for request body changes'}]) because it has been updated to v1. I updated the sent parameters to match v1 and updated their description in the readme file
@joaomdmoura
Copy link
Collaborator

Disclaimer: This review was made by a crew of AI Agents.

Code Review Comment for PR #237 - FireCrawl API Update

Overview

This PR makes significant updates to the FireCrawl crawler implementation, aligning it with the v1 API requirements by modifying the parameter structure and accompanying documentation.

Code Quality Findings

  1. Documentation (README.md)

    • The updates to the documentation provide clarity on the new parameter structures introduced in this version. However, additional examples showcasing common usage scenarios would be beneficial.
    • Suggested improvements:
      • Add a section detailing version compatibility to guide users transitioning from older versions.
      • Include examples of valid crawler_options to enhance user understanding.
      • Document default values for parameters to avoid confusion:
        ## Version Compatibility
        This implementation is compatible with FireCrawl API v1.x
        
        ## Examples
        ```json
        {
          "crawler_options": {
            "maxDepth": 2,
            "limit": 10,
            "scrapeOptions": {
              "formats": ["markdown", "html"],
              "timeout": 30000
            }
          }
        }
  2. Implementation (firecrawl_crawl_website_tool.py)

    • The code correctly updates API compatibility, but notable areas for improvement include:
      • Error Handling: There is a lack of validation leading to potential runtime errors from invalid parameters. Implementing proper input validation is crucial. For instance:
        def _validate_options(self, options: dict, timeout: int) -> None:
            if timeout <= 0:
                raise ValueError("Timeout must be positive")
            if options.get("maxDepth", 0) < 1:
                raise ValueError("maxDepth must be at least 1")
            if options.get("limit", 0) < 1:
                raise ValueError("limit must be at least 1")
      • Default Values: The code contains hard-coded default values; consider defining these as constants within the class to adhere to DRY (Don't Repeat Yourself) principles.
        class FirecrawlCrawlWebsiteTool:
            DEFAULT_TIMEOUT = 30000
            DEFAULT_OPTIONS = { ... }
      • Error Messaging: Implementing enhanced error messages can help developers debug issues promptly.
  3. Type Hints and Documentation

    • Adding complete type hints for parameters and return types is highly encouraged. Furthermore, including docstrings that explain functions and their parameters will improve code readability and maintainability.
  4. Testing

    • It’s vital to add unit tests covering both typical and edge cases for the new parameter structure. Integration tests against the v1 API could also validate that these changes meet expected behavior in real-world scenarios.

Historical Context from Related PRs

While I could not fetch specific historical data, it’s important to acknowledge that related pull requests modified similar files and parameters. Reviewing these changes may reveal useful practices and pitfalls from past implementations.

Summary

The PR effectively addresses API compatibility, but there are additional enhancements required in error handling, documentation, and testing that will improve the robustness and usability of the tool. By implementing the suggested improvements, the development team can ensure a better user experience and maintainability of the codebase moving forward. Regular updates and community feedback will be key to sustaining this alignment with future API versions.

@SHIXOOM
Copy link
Contributor Author

SHIXOOM commented Mar 28, 2025

I updated the tool more given the review above:

1-Added examples for the API request in the readme.
2-Updated the parameters in the API fully for FireCrawl V1.
3-Added default values for the crawling options and the timeout.
4-Updated the tool to include important parameters explicitly, so that the agent can customize them unlike before.

Please check the tool now and tell me if there's more I can update/fix.

@SHIXOOM SHIXOOM changed the title Fix: FireCrawl FirecrawlCrawlWebsiteTool update parameters for FireCrawl API v1 Fix: FirecrawlCrawlWebsiteTool update parameters for FireCrawl API v1 and update run arguments for agents Apr 4, 2025
@SHIXOOM
Copy link
Contributor Author

SHIXOOM commented Apr 4, 2025

Hello @joaomdmoura & @lorenzejay. Can you please review this update, I also need this tool for my project and work. your efforts are truly appreciated.

@lorenzejay
Copy link
Contributor

Taking a look

Copy link
Contributor

@lorenzejay lorenzejay left a comment

Choose a reason for hiding this comment

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

LGTM

@SHIXOOM
Copy link
Contributor Author

SHIXOOM commented Apr 6, 2025

@lucasgomide

@lucasgomide lucasgomide merged commit 6bd9ba0 into crewAIInc:main Apr 6, 2025
@SHIXOOM SHIXOOM deleted the Fix-FireCrawl-Crawler-Tool branch April 24, 2025 09:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants