fix: session.markBad() on requestHandler error #1709
Conversation
|
@barjin Isn't that Browser pool part just buggy? Seems like it always restarts the session as the one from launchContext. |
|
@metalwarrior665 To be fair, I'm not exactly sure - to me, it seems that the |
| @@ -943,7 +943,7 @@ export class BasicCrawler<Context extends CrawlingContext = BasicCrawlingContext | |||
|
|
|||
| // reclaim session if request finishes successfully | |||
| request.state = RequestState.DONE; | |||
| session?.markGood(); | |||
| crawlingContext.session?.markGood(); | |||
There was a problem hiding this comment.
I'm not seeing how this helps since both session and crawlingContext.session are pointing out to the same reference. Or is there some issue with the session initialization?
There was a problem hiding this comment.
Well, run it and see - they are not referencing the same object :)
The call to requestHandler at L930 passes the crawlingContext as a reference to the request handling machinery - which eventually, here, overwrites the internal session property with a new copy. The session variable in this scope (used to create the crawlingContext) still points to the old session object, but it's not the session that's in the crawlingContext now - that one that was used during the request handling.
There was a problem hiding this comment.
Makes sense. Indeed, those mutations...
|
i dont think i am the original author, you'd have to check the history in apify-ts repo :] i moved it here in one commit, so i am author of pretty much every line prior to crawlee release in git blame :D anyway, lets merge, refactoring can wait. |
fixes #1635
Enforces the access to the same
Sessioninstance throughout the inheritance chain (e.g.BrowserCrawlerandBasicCrawler). This is required because of rogueSessionmutations (e.g. here).