fix: cancel timed out requests

[MQ37]

Since we cannot remove request that are already being crawled we just return dataset id on timeout so the user knows where to check for results - as we discussed.

closes #31

[jirispilka]

On second thought, this goes against the idea of standby mode — it's meant to be request/response. No one will check the dataset, and that's not the goal.

It would be better to fix the issue by terminating the running request and returning empty results.
I can't image a use-case that someone would use it this way.

Can you?

[MQ37]

On second thought, this goes against the idea of standby mode — it's meant to be request/response. No one will check the dataset, and that's not the goal.

It would be better to fix the issue by terminating the running request and returning empty results. I can't image a use-case that someone would use it this way.

Can you?

Agree, not many people will check the dataset and they will consume mainly the response itself. Will check if there is a simple way to skip or prevent the request from crawling.

[MQ37]

@jirispilka Changed the implementation to cancel the requests of timed out response based on the discussion apify/crawlee#1215. This way the request content is not handled, they are only added to the dataset with status failed with reason timed out.

[matyascimbulka]

I also agree that in standby mode the dataset is irrelevant. Thank you the solutions looks good. I just have small comments.

The biggest issue for me is that the bounded array doesn't survive migrations which could potentially leave some requests running. @jirispilka Do you think this could be an issue?

And there are 2 lint errors.

[matyascimbulka]

@MQ37 I'm looking at the code again and I'm wondering if we can't just check if responseData in responses.ts has the responseId key when handling the request in the handler. That way we wouldn't need to manage another new array and it would survive migrations.

[MQ37]

@MQ37 I'm looking at the code again and I'm wondering if we can't just check if responseData in responses.ts has the responseId key when handling the request in the handler. That way we wouldn't need to manage another new array and it would survive migrations.

This might be simpler but we would still need to handle the migration, right? Or the response data is handled on migration?

[matyascimbulka]

I think migrations would work as expected. The responseData map would be initiated empty therefore any resurrected requests wouldn't find their responseId as its key. The missing key would signal the handler to set the request.noRetry = true; and throw the error.

[MQ37]

I think migrations would work as expected. The responseData map would be initiated empty therefore any resurrected requests wouldn't find their responseId as its key. The missing key would signal the handler to set the request.noRetry = true; and throw the error.

I guess the responseData approach would be simpler, thank you for suggestion 👍

What actually happens to the user request to the /search endpoint when the Actor migrates. Does Apify act as a persist the connection that is then handed over to the new Actor container or is it closed? If it is closed we actually do not need to handle this case.

[MQ37]

I think migrations would work as expected. The responseData map would be initiated empty therefore any resurrected requests wouldn't find their responseId as its key. The missing key would signal the handler to set the request.noRetry = true; and throw the error.

I guess the responseData approach would be simpler, thank you for suggestion 👍

What actually happens to the user request to the /search endpoint when the Actor migrates. Does Apify act as a persist the connection that is then handed over to the new Actor container or is it closed? If it is closed we actually do not need to handle this case.

Ahh, I see. We just sent Actor is migrating please try again and cut the connection.

[MQ37]

@matyascimbulka refactored based on your suggestion and the implementation is much simpler, thank you 👍

[matyascimbulka]

You're welcome. I'm happy to help. This looks good.

[jirispilka]

@MQ37 @matyascimbulka thank you guys!

@MQ37 please just update the CHANGELOG.md and we are good to go.