diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 00000000..d958beb4 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,64 @@ +# AGENTS.md + +Ferrum is a pure-Ruby driver for Chrome over the Chrome DevTools Protocol (CDP). No Selenium, no WebDriver, +no Node. Chrome/Chromium only: Firefox support was removed, don't add it back or plan for BiDi. + +## Layout + +- `lib/ferrum/` - the library. `Browser` owns a `Client` (WebSocket + `Client::Subscriber` event dispatch) and + `Contexts`, which tracks `Context`s and their `Target`s; a target connects as a `Page` or `Worker`. +- `sig/` - RBS signatures mirroring `lib/`. Keep them in sync with every public or private method you add or change. +- `spec/` - RSpec suite, see [Tests](#tests). +- `docs/` - user-facing guides, numbered by chapter. +- `CHANGELOG.md` - `## [Unreleased]` section on top, grouped into Added / Changed / Fixed / Removed. + +## Commands + +```sh +bundle exec rake # whole suite (rspec with -w), what CI runs +bundle exec rspec spec/page_spec.rb # one file +bundle exec rubocop # lint, must be clean +HEADLESS=false bundle exec rspec ... # watch the browser +``` + +The suite boots a real Chrome and a Sinatra/Puma test app (`spec/support/application.rb`, views in +`spec/support/views/`). CI runs Ruby 3.1 to 4.0 against stable Chrome. + +## Tests + +- **Prefer system/feature specs**: drive a real browser against the test app and assert on observable behavior. + They live in `spec/` next to the feature, e.g. `spec/page_spec.rb`, `spec/network/`. +- **Unit specs go in `spec/unit/` and nowhere else.** A spec is a unit spec if it stubs or mocks Ferrum's own + objects (`allow(...).to receive`, `and_raise`, `and_wrap_original`), builds objects with `allocate` or + `instance_variable_set`, or tests one class in isolation. Mirror the `lib/` path: + `lib/ferrum/contexts.rb` -> `spec/unit/contexts_spec.rb`, `lib/ferrum/client/web_socket.rb` -> + `spec/unit/client/web_socket_spec.rb`. Never add them to the feature spec files in `spec/`. +- Don't test private methods; cover them through the public API. +- Almost no comments in specs; the example name says what is checked. +- For a bug fix, make sure the new spec fails without the fix and passes with it. + +## Code style + +- Ruby >= 3.1, `# frozen_string_literal: true`, double quotes, RuboCop config in `.rubocop.yml`. +- Prefer self-documenting code over comments. Document public methods with YARD (`@param`, `@return`, `@raise`). +- Add or update RBS types in `sig/` for anything you touch. +- American English everywhere: code, comments, docs, errors, commits. + +## Changelog + +Add an entry under `## [Unreleased]` for user-visible changes. A few lines at most: what broke and what it does +now, with the issue/PR number in brackets, e.g. `[#641]`. No essays. + +## Git + +- Conventional commits: `(): `, type one of `feat|fix|docs|style|ref|test|chore|perf`, + imperative mood, no trailing period, subject <= 100 chars. Add a body for non-trivial changes. +- One logical change per commit. +- No AI attribution of any kind: no `Co-Authored-By`, no session links. +- Work on a branch, commit locally. Never push or open a PR unless asked. + +## Chrome gotchas + +- Don't add `--disable-crashpad-for-testing` to the default flags: it breaks a normally launched Chrome (child + processes die with `FD ownership violation`, the network service crash-loops). Emulated amd64 Docker hides this. +- The browser's implicit (startup window) context can't be addressed by id or disposed, see `Context#implicit?`. diff --git a/CHANGELOG.md b/CHANGELOG.md index 9754673e..47a6221b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,9 @@ the browser; a failed `IO.close` is raised to the caller. ### Fixed +- A browser context whose `Target.disposeBrowserContext` timed out stayed registered, so every later `#reset` asked + again and raised `Disposal of browser context ... is already pending` for the rest of the process. It's now + forgotten either way, and `#reset` disposes the remaining contexts before raising the first error [#641] - An error in the client or websocket thread was re-raised in the main thread wherever it happened to be, or took the process down with it, since `Utils::Thread.spawn` defaulted to `abort_on_exception: true` and neither thread rescued. Ferrum's threads no longer abort, they report to stderr and mark the connection dead [#470] diff --git a/lib/ferrum/contexts.rb b/lib/ferrum/contexts.rb index 22b6b4d1..f3b0a9d4 100644 --- a/lib/ferrum/contexts.rb +++ b/lib/ferrum/contexts.rb @@ -90,17 +90,26 @@ def create(**options) # Disposes a browser context and all of its targets. The browser's implicit # context is not ours to dispose, see {Context#implicit?}. # + # The context is forgotten even if Chrome fails to dispose it in time: its + # targets are disconnected by then, and asking again only gets + # `Disposal of browser context ... is already pending` back. + # # @param [String] context_id # # @return [Boolean, nil] + # + # @raise [TimeoutError, BrowserError] def dispose(context_id) context = @contexts[context_id] return unless context return if context.implicit? - context.close_targets_connection - @client.command("Target.disposeBrowserContext", browserContextId: context.id) - @contexts.delete(context_id) + begin + context.close_targets_connection + @client.command("Target.disposeBrowserContext", browserContextId: context.id) + ensure + @contexts.delete(context_id) + end true end @@ -112,13 +121,23 @@ def close_connections @contexts.each_value(&:close_targets_connection) end - # Disposes every context still known to the browser. + # Disposes every context still known to the browser. A context that fails + # to dispose doesn't stop the others from being disposed; the first error + # is raised once all of them have been tried. # # @return [void] + # + # @raise [TimeoutError, BrowserError] def reset + error = nil context_ids = @client.command("Target.getBrowserContexts")["browserContextIds"] @default_context = nil if context_ids.include?(@default_context&.id) - @contexts.each_key { |id| dispose(id) if context_ids.include?(id) } + (@contexts.keys & context_ids).each do |id| + dispose(id) + rescue BrowserError, TimeoutError => e + error ||= e + end + raise error if error end # Number of known contexts. diff --git a/spec/unit/contexts_spec.rb b/spec/unit/contexts_spec.rb new file mode 100644 index 00000000..5c688deb --- /dev/null +++ b/spec/unit/contexts_spec.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +describe Ferrum::Contexts do + describe "#reset" do + it "forgets a context whose disposal fails and still disposes the others" do + stuck = browser.contexts.create + other = browser.contexts.create + allow(browser.client).to receive(:command).and_call_original + allow(browser.client).to receive(:command) + .with("Target.disposeBrowserContext", browserContextId: stuck.id) + .and_raise(Ferrum::TimeoutError) + + expect { browser.reset }.to raise_error(Ferrum::TimeoutError) + expect(browser.contexts[stuck.id]).to be_nil + expect(browser.contexts[other.id]).to be_nil + expect(browser.client.command("Target.getBrowserContexts")["browserContextIds"]).not_to include(other.id) + + allow(browser.client).to receive(:command) + .with("Target.disposeBrowserContext", browserContextId: stuck.id) + .and_raise(Ferrum::BrowserError, { "message" => "Disposal of browser context #{stuck.id} is already pending" }) + + expect { browser.reset }.not_to raise_error + end + end +end