Skip to content

Refactor timeout handling #2951

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

Open
B4nan opened this issue Apr 30, 2025 · 0 comments
Open

Refactor timeout handling #2951

B4nan opened this issue Apr 30, 2025 · 0 comments
Labels
feature Issues that represent new features or improvements to existing features. product roadmap Issues synchronized to product roadmap. t-tooling Issues with this label are in the ownership of the tooling team.
Milestone

Comments

@B4nan
Copy link
Member

B4nan commented Apr 30, 2025

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:

  • navigationTimeoutSecs for making the request (opening a page, or firing an HTTP request)
  • requestHandlerTimeoutSecs for 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

@B4nan B4nan added the feature Issues that represent new features or improvements to existing features. label Apr 30, 2025
@github-actions github-actions bot added the t-tooling Issues with this label are in the ownership of the tooling team. label Apr 30, 2025
@B4nan B4nan added this to the 4.0 milestone Apr 30, 2025
@B4nan B4nan added the product roadmap Issues synchronized to product roadmap. label Apr 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Issues that represent new features or improvements to existing features. product roadmap Issues synchronized to product roadmap. t-tooling Issues with this label are in the ownership of the tooling team.
Projects
None yet
Development

No branches or pull requests

1 participant