Skip to content

Fix wss urls and session_id lost#559

Merged
route merged 4 commits intorubycdp:mainfrom
fit2you:main
Feb 12, 2026
Merged

Fix wss urls and session_id lost#559
route merged 4 commits intorubycdp:mainfrom
fit2you:main

Conversation

@neo87cs
Copy link
Contributor

@neo87cs neo87cs commented Jan 28, 2026

The current implementation of Ferrum is missing two important points:

  1. the ws_url is missing support to wss://..., which is a SSL implementation;
  2. cause of subscribers in contexts.rb being async, the context.rb#add_target was loosing the session_id property

@route
Copy link
Member

route commented Feb 10, 2026

can you rebase on main? tests are fixed now.

@neo87cs
Copy link
Contributor Author

neo87cs commented Feb 11, 2026

Done, tests are passing now

@route
Copy link
Member

route commented Feb 11, 2026

Sorry to say but linters don't :)

@neo87cs
Copy link
Contributor Author

neo87cs commented Feb 12, 2026

I am sorry, i don't know why i thought it was related only to specs, i fixed the lint issue

@route
Copy link
Member

route commented Feb 12, 2026

There's only single place where we don't pass session id because it's not provided:

@client.on("Target.targetCreated") do |params|
...
@contexts[context_id]&.add_target(params: info)

Theoretically the order of events in Chrome is not deterministic, so it might be called before Target.attachedToTarget happens. If it does we can end up with target w/o session id.

To be precise we should never overwrite session id, so I think condition should be if session_id && target.session_id.nil? but I can correct that. Please let me know If my thinking is correct or you know the way to reproduce the issue. Better to add test.

@route route merged commit de385e2 into rubycdp:main Feb 12, 2026
8 checks passed
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.

2 participants