fix(contexts): forget a context whose disposal fails

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>(<scope>): <subject>`, 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?`.

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]

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.

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