Return :invalid_option for bad search and summarize options instead of crashing

README.md

@@ -74,7 +74,7 @@ goes to another host. To turn them on, set `redirect: true` or `retry: :safe_tra
 - `:lens`: `:default`, `:programming`, `:forums`, `:pdfs`, `:non_commercial`, or `:world_news`.
 - `:sort`: `:recency`, `:website`, or `:ad_trackers`.
 - `:time`: `:day`, `:week`, `:month`, or `:year`.
-- `:from` and `:to`: a `YYYY-MM-DD` date range. You cannot use them with `:time`.
+- `:from` and `:to`: a `YYYY-MM-DD` date range. `:from` must not be after `:to`. You cannot use them with `:time`.
 - `:site`: adds a `site:` filter.
 - `:filetype`: adds a `filetype:` filter.
 - `:verbatim`: `true` turns off query expansion.

lib/kagi/search.ex

@@ -79,8 +79,8 @@ defmodule Kagi.Search do
   @spec query_params(String.t() | [String.t()], keyword()) ::
           {:ok, keyword()} | {:error, Error.t()}
   defp query_params(query, options) do
-    with {:ok, query} <- build_query(query, options),
-         {:ok, options} <- validate_options(options) do
+    with {:ok, options} <- validate_options(options),
+         {:ok, query} <- build_query(query, options) do
       params = [
         {:plain, :r, options[:region]},
         {:mapped, :l, options[:lens], &lens_value/1},
@@ -97,13 +97,34 @@ defmodule Kagi.Search do
 
   @spec validate_options(keyword()) :: {:ok, keyword()} | {:error, Error.t()}
   defp validate_options(options) do
-    with :ok <- validate_time_range(options),
-         :ok <- validate_date(:from, options[:from]),
-         :ok <- validate_date(:to, options[:to]) do
+    with :ok <- validate_string(:site, options[:site]),
+         :ok <- validate_string(:filetype, options[:filetype]),
+         :ok <- validate_string(:region, options[:region]),
+         :ok <- validate_verbatim(options[:verbatim]),
+         :ok <- validate_time_range(options),
+         {:ok, from} <- parse_date(:from, options[:from]),
+         {:ok, to} <- parse_date(:to, options[:to]),
+         :ok <- validate_date_order(from, to) do
       {:ok, options}
     end
   end
 
+  @spec validate_string(:site | :filetype | :region, term()) :: :ok | {:error, Error.t()}
+  defp validate_string(_key, nil), do: :ok
+  defp validate_string(_key, value) when is_binary(value) and value != "", do: :ok
+
+  defp validate_string(key, value) do
+    {:error,
+     Error.new(:invalid_option, ":#{key} must be a non-empty string, got: #{inspect(value)}")}
+  end
+
+  @spec validate_verbatim(term()) :: :ok | {:error, Error.t()}
+  defp validate_verbatim(value) when is_nil(value) or is_boolean(value), do: :ok
+
+  defp validate_verbatim(value) do
+    {:error, Error.new(:invalid_option, ":verbatim must be a boolean, got: #{inspect(value)}")}
+  end
+
   @spec validate_time_range(keyword()) :: :ok | {:error, Error.t()}
   defp validate_time_range(options) do
     if options[:time] && (options[:from] || options[:to]) do
@@ -113,21 +134,31 @@ defmodule Kagi.Search do
     end
   end
 
-  @spec validate_date(:from | :to, term()) :: :ok | {:error, Error.t()}
-  defp validate_date(_key, nil), do: :ok
+  @spec parse_date(:from | :to, term()) :: {:ok, Date.t() | nil} | {:error, Error.t()}
+  defp parse_date(_key, nil), do: {:ok, nil}
 
-  defp validate_date(key, date) when is_binary(date) do
-    if Regex.match?(~r/^\d{4}-\d{2}-\d{2}$/, date) do
-      :ok
+  defp parse_date(key, value) do
+    with true <- is_binary(value),
+         {:ok, date} <- Date.from_iso8601(value) do
+      {:ok, date}
     else
-      {:error, Error.new(:invalid_option, ":#{key} must use YYYY-MM-DD")}
+      _invalid ->
+        {:error,
+         Error.new(:invalid_option, ":#{key} must be a YYYY-MM-DD date, got: #{inspect(value)}")}
     end
   end
 
-  defp validate_date(key, _date) do
-    {:error, Error.new(:invalid_option, ":#{key} must use YYYY-MM-DD")}
+  @spec validate_date_order(Date.t() | nil, Date.t() | nil) :: :ok | {:error, Error.t()}
+  defp validate_date_order(%Date{} = from, %Date{} = to) do
+    if Date.compare(from, to) == :gt do
+      {:error, Error.new(:invalid_option, ":from must not be after :to")}
+    else
+      :ok
+    end
   end
 
+  defp validate_date_order(_from, _to), do: :ok
+
   @spec build_query(String.t() | [String.t()], keyword()) ::
           {:ok, String.t()} | {:error, Error.t()}
   defp build_query(query, options) do

lib/kagi/summary.ex

@@ -94,17 +94,20 @@ defmodule Kagi.Summary do
 
   @spec query_params(String.t(), keyword()) :: {:ok, keyword()} | {:error, Error.t()}
   defp query_params(url, options) do
-    with {:ok, summary_type} <- summary_type(Keyword.get(options, :type, :summary)) do
-      {:ok,
-       [
-         url: url,
-         stream: "1",
-         target_language: Keyword.get(options, :lang, "EN"),
-         summary_type: summary_type
-       ]}
+    with {:ok, summary_type} <- summary_type(Keyword.get(options, :type, :summary)),
+         {:ok, lang} <- target_language(Keyword.get(options, :lang, "EN")) do
+      {:ok, [url: url, stream: "1", target_language: lang, summary_type: summary_type]}
     end
   end
 
+  @spec target_language(term()) :: {:ok, String.t()} | {:error, Error.t()}
+  defp target_language(lang) when is_binary(lang) and lang != "", do: {:ok, lang}
+
+  defp target_language(value) do
+    {:error,
+     Error.new(:invalid_option, ":lang must be a non-empty string, got: #{inspect(value)}")}
+  end
+
   @spec summary_type(term()) :: {:ok, String.t()} | {:error, Error.t()}
   defp summary_type(:summary), do: {:ok, "summary"}
   defp summary_type(:takeaway), do: {:ok, "takeaway"}

test/kagi/search_test.exs

@@ -96,6 +96,71 @@ defmodule Kagi.SearchTest do
     refute_received :network
   end
 
+  @spec client_capturing_url(pid()) :: Kagi.Client.t()
+  defp client_capturing_url(test_pid) do
+    adapter =
+      Kagi.FakeAdapter.put(Kagi.FakeAdapter, fn request ->
+        send(test_pid, {:network, request.url})
+        {request, Req.Response.new(status: 200, body: ~s(<div id="search-app"></div>))}
+      end)
+
+    %Kagi.Client{session_token: "token", req_options: [adapter: adapter]}
+  end
+
+  test "rejects a :site, :filetype, or :region that is not a non-empty string before any request" do
+    client = client_capturing_url(self())
+
+    for key <- [:site, :filetype, :region], value <- [%{a: 1}, 42, ""] do
+      assert {:error, %Error{reason: :invalid_option, message: message}} =
+               Kagi.search(client, "elixir", [{key, value}])
+
+      assert message =~ ":#{key} must be a non-empty string, got: #{inspect(value)}"
+    end
+
+    refute_received {:network, _url}
+  end
+
+  test "rejects a :verbatim that is not a boolean" do
+    client = client_capturing_url(self())
+
+    for value <- ["false", 1] do
+      assert {:error, %Error{reason: :invalid_option, message: message}} =
+               Kagi.search(client, "elixir", verbatim: value)
+
+      assert message =~ ":verbatim must be a boolean"
+    end
+
+    refute_received {:network, _url}
+  end
+
+  test "rejects :from with a trailing newline, an impossible date 2024-13-45, and a range where :from is after :to" do
+    client = client_capturing_url(self())
+
+    for options <- [[from: "2024-01-01\n"], [from: "2024-13-45"], [to: ~D[2024-01-01]]] do
+      assert {:error, %Error{reason: :invalid_option, message: message}} =
+               Kagi.search(client, "elixir", options)
+
+      assert message =~ "must be a YYYY-MM-DD date"
+    end
+
+    assert {:error, %Error{reason: :invalid_option, message: ":from must not be after :to"}} =
+             Kagi.search(client, "elixir", from: "2025-01-01", to: "2024-01-01")
+
+    refute_received {:network, _url}
+  end
+
+  test "accepts a valid :from/:to range and sends from_date and to_date unchanged" do
+    client = client_capturing_url(self())
+
+    assert {:ok, %Search{}} =
+             Kagi.search(client, "elixir", from: "2024-01-01", to: "2024-01-01", verbatim: true)
+
+    assert_received {:network, url}
+
+    assert %{"from_date" => "2024-01-01", "to_date" => "2024-01-01", "verbatim" => "1"} =
+             URI.decode_query(url.query)
+  end
+
   test "rejects queries that are not strings or lists of strings" do
     client = %Kagi.Client{session_token: "token"}
 

test/kagi/summary_test.exs

@@ -123,6 +123,19 @@ defmodule Kagi.SummaryTest do
     end
   end
 
+  test "rejects a :lang that is not a non-empty string" do
+    client = client_capturing_timeout(self(), [])
+
+    for lang <- [%{}, :de, ""] do
+      assert {:error, %Error{reason: :invalid_option, message: message}} =
+               Kagi.summarize(client, "https://example.com", lang: lang)
+
+      assert message =~ ":lang must be a non-empty string, got: #{inspect(lang)}"
+    end
+
+    refute_received {:receive_timeout, _timeout}
+  end
+
   defp fixture(name) do
     "test/fixtures/summary/#{name}"
     |> File.read!()