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

fix(core): trigger errorHandler for session errors #2683

Merged
merged 1 commit into from
Sep 27, 2024

Conversation

B4nan
Copy link
Member

@B4nan B4nan commented Sep 26, 2024

Closes #2678

@B4nan B4nan requested a review from barjin September 26, 2024 13:32
@github-actions github-actions bot added this to the 99th sprint - Tooling team milestone Sep 26, 2024
@github-actions github-actions bot added t-tooling Issues with this label are in the ownership of the tooling team. tested Temporary label used only programatically for some analytics. labels Sep 26, 2024
@B4nan B4nan added the adhoc Ad-hoc unplanned task added during the sprint. label Sep 26, 2024
@janbuchar
Copy link
Contributor

Is this something that we should port over to crawlee-python?

@B4nan
Copy link
Member Author

B4nan commented Sep 26, 2024

yes, most likely we have the same problem there

Copy link
Contributor

@barjin barjin left a comment

Choose a reason for hiding this comment

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

Changes seem safe enough, thanks 👍🏽

I think the original idea was that the errorHandler should only be called for actionable errors, while handling the session rotation automatically. I don't really have a hard opinion on this, it might indeed be a bit confusing.

@B4nan B4nan merged commit 7d72bcb into master Sep 27, 2024
9 checks passed
@B4nan B4nan deleted the session-error-handler branch September 27, 2024 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
adhoc Ad-hoc unplanned task added during the sprint. t-tooling Issues with this label are in the ownership of the tooling team. tested Temporary label used only programatically for some analytics.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Session retries don't trigger the errorHandler
3 participants