Split Error types into API and SDK

Co-authored-by: Ryan Spore <ryan@mechanical-orchard.com>

lib/playwright/sdk/channel/error.ex → lib/playwright/api/error.ex

@@ -1,8 +1,7 @@
-defmodule Playwright.SDK.Channel.Error do
+defmodule Playwright.API.Error do
   @moduledoc false
   # `Error` represents an error message received from the Playwright server
   # that is in response to a `Message` previously sent.
-  alias Playwright.SDK.Channel
 
   @enforce_keys [:type, :message]
   defstruct [:type, :message]
@@ -13,17 +12,15 @@ defmodule Playwright.SDK.Channel.Error do
         }
 
   def new(%{error: %{name: name, message: message} = _error}, _catalog) do
-    %Channel.Error{
+    %__MODULE__{
       type: name,
       message: String.split(message, "\n") |> List.first()
     }
   end
-
-  # TODO: determine why we get here...
-  # DONE: see comment at error_handling.ex:9.
-  def new(%{error: %{message: message} = _error}, _catalog) do
-    %Channel.Error{
-      type: "NotImplementedError",
+  def new(%{error: %{message: message} = error}, _catalog) do
+    dbg(error)
+    %__MODULE__{
+      type: "UnknownError",
       message: String.split(message, "\n") |> List.first()
     }
   end

lib/playwright/browser_context.ex

@@ -468,7 +468,7 @@ defmodule Playwright.BrowserContext do
     end)
   end
 
-  @spec grant_permissions(t(), [String.t()], options()) :: t() | {:error, Channel.Error.t()}
+  @spec grant_permissions(t(), [String.t()], options()) :: t() | {:error, Playwright.API.Error.t()}
   def grant_permissions(%BrowserContext{} = context, permissions, options \\ %{}) do
     params = Map.merge(%{permissions: permissions}, options)
     post!(context, :grant_permissions, params)

lib/playwright/cdp_session.ex

@@ -33,7 +33,7 @@ defmodule Playwright.CDPSession do
       {:ok, _} ->
         cdp_session
 
-      {:error, %Channel.Error{} = error} ->
+      {:error, %Playwright.API.Error{} = error} ->
         {:error, error}
     end
   end

lib/playwright/frame.ex

@@ -905,7 +905,7 @@ defmodule Playwright.Frame do
 
   FIXME: the following is NOT TRUE... Returns `nil` if waiting for a hidden or detached element.
   """
-  @spec wait_for_selector(t(), binary(), map()) :: ElementHandle.t() | {:error, Channel.Error.t()}
+  @spec wait_for_selector(t(), binary(), map()) :: ElementHandle.t() | {:error, Playwright.API.Error.t()}
   def wait_for_selector(%Frame{session: session} = frame, selector, options \\ %{}) do
     Channel.post(session, {:guid, frame.guid}, :wait_for_selector, Map.merge(%{selector: selector}, options))
   end

lib/playwright/locator.ex

@@ -262,7 +262,7 @@ defmodule Playwright.Locator do
   If the element is detached from the DOM at any moment during the action, this method throws.
 
   When all steps combined have not finished during the specified timeout, this method throws a
-  `Playwright.SDK.Channel.Error.t()`. Passing `0` timeout disables this.
+  `Playwright.API.Error.t()`. Passing `0` timeout disables this.
 
   ## Returns
 
@@ -282,10 +282,10 @@ defmodule Playwright.Locator do
   | `:timeout`       | option | `number()`                        | Maximum time in milliseconds. Pass `0` to disable timeout. The default value can be changed via `Playwright.BrowserContext.set_default_timeout/2` or `Playwright.Page.set_default_timeout/2`. `(default: 30 seconds)` |
   | `:trial`         | option | `boolean()`                       | When set, this call only performs the actionability checks and skips the action. Useful to wait until the element is ready for the action without performing it. `(default: false)` |
   """
-  @spec click(t(), options_click()) :: :ok
+  @spec click(t(), options_click()) :: t()
   def click(%Locator{} = locator, options \\ %{}) do
     options = Map.merge(options, %{strict: true})
-    Frame.click(locator.frame, locator.selector, options)
+    returning(locator, fn -> Frame.click(locator.frame, locator.selector, options) end)
   end
 
   # @spec content_frame(Locator.t()) :: FrameLocator.t()
@@ -420,15 +420,15 @@ defmodule Playwright.Locator do
   ## Returns
 
   - `Playwright.ElementHandle.t()`
-  - `{:error, Playwright.SDK.Channel.Error.t()}`
+  - `{:error, Playwright.API.Error.t()}`
 
   ## Arguments
 
   | key/name | type   |            | description |
   | ---------- | ------ | ---------- | ----------- |
   | `:timeout` | option | `number()` | Maximum time in milliseconds. Pass `0` to disable timeout. The default value can be changed by using the `Playwright.BrowserContext.set_default_timeout/2` or `Playwright.Page.set_default_timeout/2` functions. `(default: 30 seconds)` |
   """
-  @spec element_handle(t(), options()) :: ElementHandle.t() | {:error, Channel.Error.t()}
+  @spec element_handle(t(), options()) :: ElementHandle.t() | {:error, Playwright.API.Error.t()}
   def element_handle(%Locator{} = locator, options \\ %{}) do
     options = Map.merge(%{strict: true, state: "attached"}, options)
 
@@ -529,7 +529,7 @@ defmodule Playwright.Locator do
   ## Returns
 
     - `Playwright.ElementHandle.t()`
-    - `{:error, Playwright.SDK.Channel.Error.t()}`
+    - `{:error, Playwright.API.Error.t()}`
 
   ## Arguments
 
@@ -539,7 +539,7 @@ defmodule Playwright.Locator do
   | `arg`        | param  | `any()`    | Argument to pass to `expression` `(optional)` |
   | `:timeout`   | option | `number()` | Maximum time in milliseconds. Pass `0` to disable timeout. The default value can be changed by using the `Playwright.BrowserContext.set_default_timeout/2` or `Playwright.Page.set_default_timeout/2` functions. `(default: 30 seconds)` |
   """
-  @spec evaluate_handle(t(), binary(), any(), options()) :: ElementHandle.t() | {:error, Channel.Error.t()}
+  @spec evaluate_handle(t(), binary(), any(), options()) :: ElementHandle.t() | {:error, Playwright.API.Error.t()}
   def evaluate_handle(locator, expression, arg \\ nil, options \\ %{})
 
   # NOTE: need to do all of the map-like things before a plain `map()`,
@@ -1294,7 +1294,7 @@ defmodule Playwright.Locator do
   # const orderSent = page.locator('#order-sent');
   # await orderSent.waitFor();
 
-  @spec wait_for(t(), options()) :: t() | {:error, Channel.Error.t()}
+  @spec wait_for(t(), options()) :: t() | {:error, Playwright.API.Error.t()}
   def wait_for(%Locator{} = locator, options \\ %{}) do
     case Frame.wait_for_selector(locator.frame, locator.selector, options) do
       %ElementHandle{} ->
@@ -1309,8 +1309,12 @@ defmodule Playwright.Locator do
   # ---------------------------------------------------------------------------
 
   defp returning(subject, task) do
-    task.()
-    subject
+    case task.() do
+      {:error, _} = error ->
+        error
+      _ ->
+        subject
+    end
   end
 
   defp with_element(%Locator{frame: frame} = locator, options, task) do

lib/playwright/response.ex

@@ -62,7 +62,7 @@ defmodule Playwright.Response do
   end
 
   @spec ok({t(), t()}) :: boolean()
-  def ok({:error, %Playwright.SDK.Channel.Error{}}) do
+  def ok({:error, %Playwright.SDK.Error{}}) do
     false
   end
 

lib/playwright/sdk/channel.ex

@@ -1,7 +1,7 @@
 defmodule Playwright.SDK.Channel do
   @moduledoc false
   import Playwright.SDK.Helpers.ErrorHandling
-  alias Playwright.SDK.Channel.{Catalog, Connection, Error, Event, Message, Response, Session}
+  alias Playwright.SDK.Channel.{Catalog, Connection, Event, Message, Response, Session}
 
   # API
   # ---------------------------------------------------------------------------
@@ -135,7 +135,7 @@ defmodule Playwright.SDK.Channel do
     item
   end
 
-  defp reply(%Error{} = error, from) do
+  defp reply(%Playwright.API.Error{} = error, from) do
     Task.start_link(fn ->
       GenServer.reply(from, {:error, error})
     end)

lib/playwright/sdk/channel/catalog.ex

@@ -7,7 +7,6 @@ defmodule Playwright.SDK.Channel.Catalog do
   """
   use GenServer
   import Playwright.SDK.Helpers.ErrorHandling
-  alias Playwright.SDK.Channel
 
   defstruct [:awaiting, :storage]
 
@@ -74,7 +73,7 @@ defmodule Playwright.SDK.Channel.Catalog do
   | `guid`     | param  | `binary()` | GUID to look up |
   | `:timeout` | option | `float()`  | Maximum time to wait, in milliseconds. Defaults to `30_000` (30 seconds). |
   """
-  @spec get(pid(), binary(), map()) :: struct() | {:error, Channel.Error.t()}
+  @spec get(pid(), binary(), map()) :: struct() | {:error, Playwright.API.Error.t()}
   def get(catalog, guid, options \\ %{}) do
     with_timeout(options, fn timeout ->
       GenServer.call(catalog, {:get, {:guid, guid}}, timeout)

lib/playwright/sdk/channel/response.ex

@@ -38,7 +38,7 @@ defmodule Playwright.SDK.Channel.Response do
   end
 
   def recv(_session, %{error: error, id: _}) do
-    Channel.Error.new(error, nil)
+    Playwright.API.Error.new(error, nil)
   end
 
   def recv(session, %{id: _} = message) do

lib/playwright/sdk/channel_owner.ex

@@ -114,7 +114,7 @@ defmodule Playwright.SDK.ChannelOwner do
             Channel.find(owner.session, {:guid, owner.guid})
 
           # acceptable (API call) errors
-          {:error, %Channel.Error{} = error} ->
+          {:error, %Playwright.API.Error{} = error} ->
             {:error, error}
         end
       end

lib/playwright/sdk/error.ex

@@ -0,0 +1,22 @@
+defmodule Playwright.SDK.Error do
+  @moduledoc false
+  # `Error` represents an error message received from the Playwright server
+  # that is in response to a `Message` previously sent.
+
+  @enforce_keys [:type, :message]
+  defstruct [:type, :message]
+
+  @type t() :: %__MODULE__{
+          type: String.t(),
+          message: String.t()
+        }
+
+  # TODO: determine why we get here...
+  # DONE: see comment at error_handling.ex:9.
+  def new(type, message) do
+    %__MODULE__{
+      type: type,
+      message: message
+    }
+  end
+end

lib/playwright/sdk/helpers/error_handling.ex

@@ -1,6 +1,6 @@
 defmodule Playwright.SDK.Helpers.ErrorHandling do
   @moduledoc false
-  alias Playwright.SDK.Channel.Error
+  alias Playwright.SDK.Error
 
   def with_timeout(options, action) when is_map(options) and is_function(action) do
     timeout = options |> Map.get(:timeout, 30_000)
@@ -14,7 +14,7 @@ defmodule Playwright.SDK.Helpers.ErrorHandling do
       action.(timeout + 5)
     catch
       :exit, {:timeout, _} = _reason ->
-        {:error, Error.new(%{error: %{message: "Timeout #{inspect(timeout)}ms exceeded."}}, nil)}
+        {:error, Error.new("TimeoutError", "Timeout #{inspect(timeout)}ms exceeded waiting for reply from Playwright Server.")}
     end
   end
 end

test/api/locator_test.exs

@@ -2,7 +2,7 @@ defmodule Playwright.LocatorTest do
   use Playwright.TestCase, async: true
 
   alias Playwright.{ElementHandle, Locator, Page}
-  alias Playwright.SDK.Channel.Error
+  alias Playwright.API.Error
 
   describe "Locator.all/1" do
     test "returns a list of Locators, addressing each respective element", %{page: page} do
@@ -112,11 +112,11 @@ defmodule Playwright.LocatorTest do
       [options: options]
     end
 
-    test "returns :ok on a successful click", %{options: options, page: page} do
+    test "returns 'subject' on a successful click", %{options: options, page: page} do
       frame = Page.main_frame(page)
 
       locator = Locator.new(frame, "a#link")
-      assert :ok = Locator.click(locator, options)
+      assert ^locator = Locator.click(locator, options)
     end
 
     test "returns a timeout error when unable to click", %{options: options, page: page} do

test/api/navigation_test.exs

@@ -1,7 +1,7 @@
 defmodule Playwright.NavigationTest do
   use Playwright.TestCase, async: true
   alias Playwright.{Page, Response}
-  alias Playwright.SDK.Channel.Error
+  alias Playwright.API.Error
 
   describe "Page.goto/2" do
     test "works (and updates the page's URL)", %{assets: assets, page: page} do

test/api/page/expect_test.exs

@@ -1,7 +1,7 @@
 defmodule Playwright.Page.NetworkTest do
   use Playwright.TestCase, async: true
   alias Playwright.{BrowserContext, Page, Response}
-  alias Playwright.SDK.Channel.{Error, Event}
+  alias Playwright.SDK.Channel.Event
 
   describe "Page.expect_event/3 without a 'trigger" do
     test "w/ an event", %{assets: assets, page: page} do
@@ -35,12 +35,12 @@ defmodule Playwright.Page.NetworkTest do
     end
 
     test "w/ an event and a timeout", %{page: page} do
-      {:error, %Error{message: message}} =
+      {:error, %Playwright.SDK.Error{message: message}} =
         Page.expect_event(page, :request_finished, %{
           timeout: 500
         })
 
-      assert message == "Timeout 500ms exceeded."
+      assert "Timeout 500ms exceeded" <> _ = message
     end
 
     test "w/ an event, a (truthy) predicate, and a timeout", %{assets: assets, page: page} do
@@ -60,15 +60,15 @@ defmodule Playwright.Page.NetworkTest do
     test "w/ an event, a (falsy) predicate, and (incidentally) a timeout", %{assets: assets, page: page} do
       Task.start(fn -> Page.goto(page, assets.empty) end)
 
-      {:error, %Error{message: message}} =
+      {:error, %Playwright.SDK.Error{message: message}} =
         Page.expect_event(page, :request_finished, %{
           predicate: fn _, _ ->
             false
           end,
           timeout: 500
         })
 
-      assert message == "Timeout 500ms exceeded."
+      assert "Timeout 500ms exceeded" <> _ = message
     end
   end
 
@@ -107,7 +107,7 @@ defmodule Playwright.Page.NetworkTest do
     end
 
     test "w/ an event and a (falsy) predicate", %{assets: assets, page: page} do
-      {:error, %Error{message: message}} =
+      {:error, %Playwright.SDK.Error{message: message}} =
         Page.expect_event(
           page,
           :request_finished,
@@ -122,7 +122,7 @@ defmodule Playwright.Page.NetworkTest do
           end
         )
 
-      assert message == "Timeout 500ms exceeded."
+      assert "Timeout 500ms exceeded" <> _ = message
     end
 
     test "w/ an event and a timeout", %{assets: assets, page: page} do

test/api/page_test.exs

@@ -2,7 +2,7 @@ defmodule Playwright.PageTest do
   use Playwright.TestCase, async: true
   alias Playwright.{Browser, ElementHandle, Frame, Page, Request, Response, Route}
   alias Playwright.SDK.Channel
-  alias Playwright.SDK.Channel.{Error, Event}
+  alias Playwright.SDK.Channel.Event
 
   describe "Page.drag_and_drop/4" do
     test "returns 'subject'", %{assets: assets, page: page} do
@@ -276,7 +276,7 @@ defmodule Playwright.PageTest do
     test "with a single option given mismatched attributes, returns a timeout", %{assets: assets, page: page} do
       page |> Page.goto(assets.prefix <> "/input/select.html")
 
-      assert {:error, %Error{message: "Timeout 500ms exceeded."}} =
+      assert {:error, %Playwright.API.Error{message: "Timeout 500ms exceeded."}} =
                Page.select_option(page, "select", %{value: "green", label: "Brown"}, %{timeout: 500})
     end
 
@@ -356,7 +356,7 @@ defmodule Playwright.PageTest do
 
       page |> Page.close()
 
-      assert {:error, %Error{message: "Timeout 100ms exceeded."}} =
+      assert {:error, %Playwright.SDK.Error{message: "Timeout 100ms exceeded" <> _}} =
                Channel.find(page.session, {:guid, page.guid}, %{timeout: 100})
     end
   end
@@ -398,7 +398,7 @@ defmodule Playwright.PageTest do
       assert page |> Page.get_attribute("div#outer", "name") == "value"
       assert page |> Page.get_attribute("div#outer", "foo") == nil
 
-      assert({:error, %Error{message: "Timeout 500ms exceeded."}} = Page.get_attribute(page, "glorp", "foo", %{timeout: 500}))
+      assert({:error, %Playwright.API.Error{message: "Timeout 500ms exceeded."}} = Page.get_attribute(page, "glorp", "foo", %{timeout: 500}))
     end
   end
 

test/sdk/channel/catalog_test.exs

@@ -1,6 +1,7 @@
 defmodule Plawyeright.Channel.CatalogTest do
   use ExUnit.Case, async: true
-  alias Playwright.SDK.Channel.{Catalog, Error}
+  alias Playwright.SDK.Error
+  alias Playwright.SDK.Channel.Catalog
 
   setup do
     %{
@@ -23,7 +24,7 @@ defmodule Plawyeright.Channel.CatalogTest do
     end
 
     test "returns an Error when there is no match within the timeout period", %{catalog: catalog} do
-      assert {:error, %Error{message: "Timeout 50ms exceeded."}} = Catalog.get(catalog, "Missing", %{timeout: 50})
+      assert {:error, %Error{message: "Timeout 50ms exceeded" <> _}} = Catalog.get(catalog, "Missing", %{timeout: 50})
     end
   end