Skip to content

Fix timing issue where a disconnected node is being requested & returned - #362

Closed
stevschmid wants to merge 2 commits into
rubycdp:mainfrom
stevschmid:fix-disconnected-nodes-found
Closed

Fix timing issue where a disconnected node is being requested & returned#362
stevschmid wants to merge 2 commits into
rubycdp:mainfrom
stevschmid:fix-disconnected-nodes-found

Conversation

@stevschmid

Copy link
Copy Markdown

This PR fixes the issue outlined in rubycdp/cuprite#239.

I fixed this initially by adding a node.isConnected check to Cuprite.isVisible, but after digging a bit deeper, it seems that the node get disconnected between the call for _cuprite.find and its handle_response. In this case we request a node via DOM.requestNode which is not connected to the document anymore and does not seem to get obsolete (subsequent commands such as #visible? never raise NodeNotFoundError).

I'm not sure this is the right place, maybe adding a disconnected check in Cuprite Node might be a bit less invasive. Please tell me what you think.

Previous solution had still some leeway for timing issues, leading to
intermittent failing specs (albeit rarely)
@espen

espen commented Nov 10, 2023

Copy link
Copy Markdown

I am facing a similar issue in my tests. It will occasionally fail. However never when I do a puts page.body before interacting with the node.

While I only did very minimal testing I was not able to reproduce this bug with this patch.

@route

route commented Jan 8, 2025

Copy link
Copy Markdown
Member

While separating Ferrum and Cuprite I had only one idea — Ferrum should be very simple and straightforward. For example you want to get a node that's fine here it is, Ferrum doesn't care if node is visible/disconnected or disappear in the DOM while still being exist in ruby world. Cuprite on the other hand should handle that correctly, because if you have a node you might click on it and besides clicking we should scroll node into viewport, check if element is overlapped by other elements and so on.
I'd say Cuprite is ideal place for this check, because node in fact exists it's just disconnected.

@route

route commented Aug 16, 2026

Copy link
Copy Markdown
Member

I think I see 2 issues with this approach:

  1. It belongs to Cuprite not Ferrum. Ferrum is a think driver, it doesn't care what state node is, wether it visible or clickable. It just returns it and user does what he wants with this node.
  2. I think putting this disconnected check in runtime doesn't really solve anything. It's just about timing when node gets disconnected. It can happen after evaluate call.

There's a single central place in Cuprite node command method where we already even raise ObsoleteNode for missing nodes. I think this check belongs there. Though it's not atomic and will never be, I see no way to correctly fix it. Its designed like this in CDP proto. If in async world node can get disconnected at any time, it will happen in between our check some day, the check will just reduce chances.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants