Refactor timeout handling
Which package is the feature request for? If unsure which one to select, leave blank
None
Feature
Currently, we have two main options to control timeouts of the request handler:
navigationTimeoutSecsfor making the request (opening a page, or firing an HTTP request)requestHandlerTimeoutSecsfor the request handler itself (user-defined function)
Due to how things are implemented in both HTTP crawlers and browser crawlers, the navigation (including navigation hooks) is executed inside the time window of the request handler. We also add a 10s "buffer" to accommodate internal code and the navigation hooks.
This can be very confusing, because a user sees a 60s request handler timeout, but the errors will mention 130s (or 100s for HTTP crawlers where navigation timeout is only 30s) instead, since we sum those two timeouts together, and add a magic 10s buffer that is not really documented anywhere.
We should refactor things so that the request handler timeout is only applied to the actual request handler function provided by the user. We already handle the navigation timeout specifically for the navigation.
Motivation
.
Ideal solution or implementation, and any additional constraints
.
Alternative solutions or implementations
No response
Other context
We should also consider introducing a new timeout option for the navigation hooks, or keep them as part of the navigation timeout. Currently, we pass this timeout to the underlying library (playwright/puppeteer/got-scraping/...), and only the main "combined" timeout is now applied. We probably want to keep this combined timeout too, in case some parts of our internal code get stuck (e.g. API calls), but the error message for it should clearly state the logic behind this number, and it shouldn't be called "requestHandler timeout" in the first place. We already have an internal timeout so maybe we could reuse that naming here.
https://apify.slack.com/archives/C0L33UM7Z/p1745846410237329
Working on this with @ezequiel38
Note that we consider those changes breaking; they are planned for the next major version. We already have a branch for that: https://github.com/apify/crawlee/tree/v4
After some testing and exploring the codebase, we have a few ideas and questions.
You included a link at the end, but we aren't able to view it. What timeout naming did you want us to use? I found an internal timeout in the code that is already being used, is that the one that you're referring to in the link? We want to rename this line that gets called when the combined timeout is reached.
You mentioned that the navigation timeout is already handled specifically for navigation, which is this file that is called here. Should we use a similar structure for the user request handler?
We did some tests and found out that the navigations hooks are not included in the _handleNavigationTimeout. So if the hooks are slow they can cause the combined timeout to be triggered which could lead to some confusion. Did you want us to incorporate it into the _handleNavigationTimeout or create separate timeout for the hooks, if so how long?