diff --git a/CHANGELOG.md b/CHANGELOG.md index 9444999..c41d9b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,21 @@ ## [Unreleased] +## [v0.11.0] - 2026-08-20 + +### Changed + +- **Breaking:** report socket failures as atoms. A call that fails at the socket now returns + `{:error, :timeout}`, `{:error, :closed}` or `{:error, }` where it previously returned + a message string built from a `MatchError`, so callers can tell a `soffice` that stopped + answering from a document it refused. Errors from `soffice` itself are still message strings; + code that assumed every reason was a binary has to handle both. + +### Fixed + +- Raise `URP.SocketError` from `URP.Protocol` when a `:gen_tcp` send or recv fails, instead of + failing a match and reporting the resulting `MatchError` message as the error. + ## [v0.10.3] - 2026-08-03 ### Fixed diff --git a/README.md b/README.md index 8d73e10..8af9331 100644 --- a/README.md +++ b/README.md @@ -64,6 +64,22 @@ config :urp, :default, > pool sends every worker to the configured host and port; distributing workers > across containers requires separate named pools or an external TCP balancer. +### Errors + +A failure is `{:error, reason}`. A message string means soffice objected to the +document or the filter; an atom means the socket gave out — `:timeout` when +soffice stopped answering, `:closed` when it hung up, or a POSIX error. The two +call for different handling: retrying a document soffice refused is pointless, +and a wedged soffice is not the document's fault. + +```elixir +case URP.convert(path, filter: "writer_pdf_Export", output: pdf) do + {:ok, ^pdf} -> :converted + {:error, reason} when is_atom(reason) -> {:unavailable, reason} + {:error, message} -> {:refused, message} +end +``` + ### Testing Stub conversions in tests — no running soffice needed. See `URP.Test`. diff --git a/VERSION b/VERSION index a3f5a8e..d9df1bb 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.10.3 +0.11.0 diff --git a/lib/urp.ex b/lib/urp.ex index 987fd69..e66de1f 100644 --- a/lib/urp.ex +++ b/lib/urp.ex @@ -49,6 +49,12 @@ defmodule URP do See `URP.Test` for details. """ + @typedoc """ + Why a call failed: a `soffice` message, or the atom `:gen_tcp` reported when + the socket itself gave out (`:timeout`, `:closed`, a POSIX error). + """ + @type error :: String.t() | atom() + @type setting :: {String.t(), String.t(), boolean() | integer() | String.t()} @type output :: Path.t() | :binary | (binary() -> any()) @type io_mode :: :file | :stream | {:file | :stream, :file | :stream} @@ -79,7 +85,7 @@ defmodule URP do * `:pool` — named pool to use (default: the auto-started pool) * `:timeout` — checkout timeout in ms (default `120_000`) """ - @spec version(keyword()) :: {:ok, String.t()} | {:error, String.t()} + @spec version(keyword()) :: {:ok, String.t()} | {:error, error()} def version(opts \\ []) do {pool, opts} = resolve_pool(opts) URP.Pool.version(pool, opts) @@ -90,7 +96,7 @@ defmodule URP do def version!(opts \\ []) do case version(opts) do {:ok, v} -> v - {:error, message} -> raise message + {:error, reason} -> raise_reason(reason) end end @@ -110,7 +116,7 @@ defmodule URP do * `:pool` — named pool to use (default: the auto-started pool) * `:timeout` — checkout timeout in ms (default `120_000`) """ - @spec services(keyword()) :: {:ok, [String.t()]} | {:error, String.t()} + @spec services(keyword()) :: {:ok, [String.t()]} | {:error, error()} def services(opts \\ []) do {pool, opts} = resolve_pool(opts) URP.Pool.services(pool, opts) @@ -121,7 +127,7 @@ defmodule URP do def services!(opts \\ []) do case services(opts) do {:ok, v} -> v - {:error, message} -> raise message + {:error, reason} -> raise_reason(reason) end end @@ -142,7 +148,7 @@ defmodule URP do * `:pool` — named pool to use (default: the auto-started pool) * `:timeout` — checkout timeout in ms (default `120_000`) """ - @spec filters(keyword()) :: {:ok, [String.t()]} | {:error, String.t()} + @spec filters(keyword()) :: {:ok, [String.t()]} | {:error, error()} def filters(opts \\ []) do {pool, opts} = resolve_pool(opts) URP.Pool.filters(pool, opts) @@ -153,7 +159,7 @@ defmodule URP do def filters!(opts \\ []) do case filters(opts) do {:ok, v} -> v - {:error, message} -> raise message + {:error, reason} -> raise_reason(reason) end end @@ -174,7 +180,7 @@ defmodule URP do * `:pool` — named pool to use (default: the auto-started pool) * `:timeout` — checkout timeout in ms (default `120_000`) """ - @spec types(keyword()) :: {:ok, [String.t()]} | {:error, String.t()} + @spec types(keyword()) :: {:ok, [String.t()]} | {:error, error()} def types(opts \\ []) do {pool, opts} = resolve_pool(opts) URP.Pool.types(pool, opts) @@ -185,7 +191,7 @@ defmodule URP do def types!(opts \\ []) do case types(opts) do {:ok, v} -> v - {:error, message} -> raise message + {:error, reason} -> raise_reason(reason) end end @@ -204,7 +210,7 @@ defmodule URP do * `:pool` — named pool to use (default: the auto-started pool) * `:timeout` — checkout timeout in ms (default `120_000`) """ - @spec locale(keyword()) :: {:ok, String.t()} | {:error, String.t()} + @spec locale(keyword()) :: {:ok, String.t()} | {:error, error()} def locale(opts \\ []) do {pool, opts} = resolve_pool(opts) URP.Pool.locale(pool, opts) @@ -215,7 +221,7 @@ defmodule URP do def locale!(opts \\ []) do case locale(opts) do {:ok, v} -> v - {:error, message} -> raise message + {:error, reason} -> raise_reason(reason) end end @@ -302,9 +308,24 @@ defmodule URP do :ok iex> URP.convert("/tmp/test.docx", filter: "writer_pdf_Export") {:ok, "/tmp/fake.pdf"} + + ## Errors + + A failure is `{:error, reason}`. `reason` is a message string when soffice + itself objected — a document it would not open, a filter it does not know — + and the atom `:gen_tcp` reported when the socket gave out: `:timeout` if + soffice stopped answering, `:closed` if it hung up, or a POSIX error. A caller + that has to tell "this document is bad" from "soffice is wedged" matches on + the shape: + + case URP.convert(path, filter: "writer_pdf_Export", output: pdf) do + {:ok, ^pdf} -> :converted + {:error, reason} when is_atom(reason) -> {:unavailable, reason} + {:error, message} -> {:refused, message} + end """ @spec convert(binary() | {:binary, binary()} | Enumerable.t(), [opt()]) :: - {:ok, Path.t()} | {:ok, binary()} | :ok | {:error, String.t()} + {:ok, Path.t()} | {:ok, binary()} | :ok | {:error, error()} def convert(input, opts \\ []) def convert(input, opts) when is_binary(input) and is_list(opts) do @@ -332,7 +353,7 @@ defmodule URP do case convert(input, opts) do {:ok, v} -> v :ok -> :ok - {:error, message} -> raise message + {:error, reason} -> raise_reason(reason) end end @@ -423,6 +444,11 @@ defmodule URP do ":io must be :file, :stream, or {:file | :stream, :file | :stream}; got: #{inspect(value)}" end + # A socket failure re-raises as itself, so the reason survives into the message. + @spec raise_reason(error()) :: no_return() + defp raise_reason(reason) when is_binary(reason), do: raise(reason) + defp raise_reason(reason), do: raise(URP.SocketError, reason: reason) + defp validate_timeout!(_name, :infinity), do: :ok defp validate_timeout!(_name, value) when is_integer(value) and value >= 0, do: :ok diff --git a/lib/urp/bridge.ex b/lib/urp/bridge.ex index 6b64768..9cf31ef 100644 --- a/lib/urp/bridge.ex +++ b/lib/urp/bridge.ex @@ -73,7 +73,7 @@ defmodule URP.Bridge do recv_timeout: timeout(), max_frame_size: pos_integer(), reply: term(), - error: String.t() | nil, + error: String.t() | atom() | nil, tid_cache: map(), oid_cache: map(), private: map() @@ -589,6 +589,7 @@ defmodule URP.Bridge do {:error, message} -> %{conn | error: message} end rescue + e in URP.SocketError -> %{conn | error: e.reason} error -> %{conn | error: "input stream failed: #{Exception.message(error)}"} end end @@ -645,6 +646,10 @@ defmodule URP.Bridge do %{conn | reply: result} end rescue + e in URP.SocketError -> + remove_partial_sink(sink) + %{conn | error: e.reason, reply: nil} + error -> remove_partial_sink(sink) %{conn | error: "output stream failed: #{Exception.message(error)}", reply: nil} @@ -787,6 +792,7 @@ defmodule URP.Bridge do P.send_frame(conn.sock, frame) conn rescue + e in URP.SocketError -> %{conn | error: e.reason} e -> %{conn | error: Exception.message(e)} end @@ -813,6 +819,7 @@ defmodule URP.Bridge do end end rescue + e in URP.SocketError -> %{conn | error: e.reason} e -> %{conn | error: Exception.message(e)} end diff --git a/lib/urp/pool.ex b/lib/urp/pool.ex index 2a41044..f34c1d9 100644 --- a/lib/urp/pool.ex +++ b/lib/urp/pool.ex @@ -42,23 +42,23 @@ defmodule URP.Pool do end @doc false - @spec version(NimblePool.pool(), keyword()) :: {:ok, String.t()} | {:error, String.t()} + @spec version(NimblePool.pool(), keyword()) :: {:ok, String.t()} | {:error, URP.error()} def version(pool, opts \\ []), do: query(pool, opts, &Bridge.version/1, :version) @doc false - @spec services(NimblePool.pool(), keyword()) :: {:ok, [String.t()]} | {:error, String.t()} + @spec services(NimblePool.pool(), keyword()) :: {:ok, [String.t()]} | {:error, URP.error()} def services(pool, opts \\ []), do: query(pool, opts, &Bridge.services/1, :services) @doc false - @spec filters(NimblePool.pool(), keyword()) :: {:ok, [String.t()]} | {:error, String.t()} + @spec filters(NimblePool.pool(), keyword()) :: {:ok, [String.t()]} | {:error, URP.error()} def filters(pool, opts \\ []), do: query(pool, opts, &Bridge.filters/1, :filters) @doc false - @spec types(NimblePool.pool(), keyword()) :: {:ok, [String.t()]} | {:error, String.t()} + @spec types(NimblePool.pool(), keyword()) :: {:ok, [String.t()]} | {:error, URP.error()} def types(pool, opts \\ []), do: query(pool, opts, &Bridge.types/1, :types) @doc false - @spec locale(NimblePool.pool(), keyword()) :: {:ok, String.t()} | {:error, String.t()} + @spec locale(NimblePool.pool(), keyword()) :: {:ok, String.t()} | {:error, URP.error()} def locale(pool, opts \\ []), do: query(pool, opts, &Bridge.locale/1, :locale) defp query(pool, opts, bridge_fun, key) do @@ -78,7 +78,7 @@ defmodule URP.Pool do @doc false @spec convert(NimblePool.pool(), binary() | {:binary, binary()} | Enumerable.t(), keyword()) :: - {:ok, binary()} | :ok | {:error, String.t()} + {:ok, binary()} | :ok | {:error, URP.error()} def convert(pool, input, opts \\ []) do {timeout, opts} = Keyword.pop(opts, :timeout, @default_timeout) {sink, opts} = Keyword.pop(opts, :sink) @@ -123,8 +123,8 @@ defmodule URP.Pool do end @doc false - @spec checkout_outcome(term(), String.t() | nil, String.t() | nil, boolean()) :: - {:ok | {:ok, binary()} | {:error, String.t()}, :reuse | :discard} + @spec checkout_outcome(term(), URP.error() | nil, URP.error() | nil, boolean()) :: + {:ok | {:ok, binary()} | {:error, URP.error()}, :reuse | :discard} def checkout_outcome(result, convert_error, error, stream_input?) do # Stream-based input registers an XInputStream at a fixed OID cache slot. # soffice's URP cache doesn't fully reset on reuse, producing truncated diff --git a/lib/urp/protocol.ex b/lib/urp/protocol.ex index 9967c56..a2443b2 100644 --- a/lib/urp/protocol.ex +++ b/lib/urp/protocol.ex @@ -44,7 +44,11 @@ defmodule URP.Protocol do @spec send_frame(:gen_tcp.socket(), iodata()) :: :ok def send_frame(sock, payload) do size = IO.iodata_length(payload) - :ok = :gen_tcp.send(sock, [<>, payload]) + + case :gen_tcp.send(sock, [<>, payload]) do + :ok -> :ok + {:error, reason} -> raise URP.SocketError, reason: reason + end end @doc """ @@ -55,7 +59,7 @@ defmodule URP.Protocol do """ @spec recv_frame(:gen_tcp.socket(), timeout(), pos_integer()) :: binary() def recv_frame(sock, timeout \\ @recv_timeout, max_frame_size \\ @max_frame_size) do - {:ok, <>} = :gen_tcp.recv(sock, 8, timeout) + <> = recv(sock, 8, timeout) if count != 1 do raise "URP: received block with count=#{count}, expected 1 (multi-message blocks not supported)" @@ -75,8 +79,7 @@ defmodule URP.Protocol do defp recv_exact(_sock, 0, _timeout), do: <<>> defp recv_exact(sock, size, timeout) when size <= @recv_chunk_size do - {:ok, payload} = :gen_tcp.recv(sock, size, timeout) - payload + recv(sock, size, timeout) end defp recv_exact(sock, size, timeout) do @@ -89,10 +92,19 @@ defmodule URP.Protocol do defp recv_chunks(sock, remaining, timeout, acc) do chunk_size = min(remaining, @recv_chunk_size) - {:ok, chunk} = :gen_tcp.recv(sock, chunk_size, timeout) + chunk = recv(sock, chunk_size, timeout) recv_chunks(sock, remaining - chunk_size, timeout, [chunk | acc]) end + # A socket failure carries the reason as an atom, so callers can tell a + # soffice that went quiet from one that refused the document. + defp recv(sock, size, timeout) do + case :gen_tcp.recv(sock, size, timeout) do + {:ok, payload} -> payload + {:error, reason} -> raise URP.SocketError, reason: reason + end + end + ## Request header builder @doc """ diff --git a/lib/urp/socket_error.ex b/lib/urp/socket_error.ex new file mode 100644 index 0000000..b509dbd --- /dev/null +++ b/lib/urp/socket_error.ex @@ -0,0 +1,27 @@ +defmodule URP.SocketError do + @moduledoc """ + Raised when the socket to `soffice` fails during a call. + + `reason` is the atom `:gen_tcp` reported — `:timeout`, `:closed`, or a POSIX + error. Conversion functions return that atom as-is, so a caller can tell a + `soffice` that stopped answering from a document it refused, which arrives as + a message string. + """ + + defexception [:reason, :message] + + @type t :: %__MODULE__{reason: :timeout | :closed | :inet.posix(), message: String.t()} + + @impl true + @spec exception(keyword()) :: t() + def exception(opts) do + reason = Keyword.fetch!(opts, :reason) + + %__MODULE__{reason: reason, message: describe(reason)} + end + + @spec describe(atom()) :: String.t() + defp describe(:timeout), do: "soffice did not answer in time" + defp describe(:closed), do: "soffice closed the connection" + defp describe(reason), do: "socket failed: #{:inet.format_error(reason)}" +end diff --git a/test/urp/pool_test.exs b/test/urp/pool_test.exs index cd02de3..005c5eb 100644 --- a/test/urp/pool_test.exs +++ b/test/urp/pool_test.exs @@ -37,6 +37,11 @@ defmodule URP.PoolTest do {{:error, "connection closed"}, :discard} end + test "passes a socket reason through as the atom it came in as" do + assert URP.Pool.checkout_outcome(nil, :timeout, :timeout, false) == + {{:error, :timeout}, :discard} + end + test "reports the error when a failed conversion left stale bytes in the reply" do assert URP.Pool.checkout_outcome(<<0x80, 0, 0, 0, 0>>, "timeout", "timeout", true) == {{:error, "timeout"}, :discard} diff --git a/test/urp/socket_error_test.exs b/test/urp/socket_error_test.exs new file mode 100644 index 0000000..fdf2e49 --- /dev/null +++ b/test/urp/socket_error_test.exs @@ -0,0 +1,92 @@ +defmodule URP.SocketErrorTest do + use ExUnit.Case, async: true + + alias URP.Protocol, as: P + + defp socket_pair do + {:ok, listen} = :gen_tcp.listen(0, [:binary, active: false, reuseaddr: true]) + {:ok, port} = :inet.port(listen) + {:ok, client} = :gen_tcp.connect(~c"localhost", port, [:binary, active: false]) + {:ok, server} = :gen_tcp.accept(listen, 1_000) + :gen_tcp.close(listen) + + on_exit(fn -> + :gen_tcp.close(client) + :gen_tcp.close(server) + end) + + {client, server} + end + + describe inspect(&P.recv_frame/3) do + test "returns the payload of a well-formed block" do + {client, server} = socket_pair() + payload = "a reply" + + :ok = :gen_tcp.send(server, [<>, payload]) + + assert P.recv_frame(client, 1_000) == payload + end + + test "raises with reason :timeout when soffice stops answering" do + {client, _server} = socket_pair() + + assert_raise URP.SocketError, "soffice did not answer in time", fn -> + P.recv_frame(client, 10) + end + end + + test "carries the reason as an atom rather than a formatted message" do + {client, _server} = socket_pair() + + assert %URP.SocketError{reason: :timeout} = + assert_raise(URP.SocketError, fn -> P.recv_frame(client, 10) end) + end + + test "raises with reason :closed when soffice hangs up" do + {client, server} = socket_pair() + :gen_tcp.close(server) + + assert %URP.SocketError{reason: :closed} = + assert_raise(URP.SocketError, fn -> P.recv_frame(client, 1_000) end) + end + + test "raises with reason :closed while reading the body of an announced frame" do + {client, server} = socket_pair() + + # Announce 64 bytes, deliver 4, then hang up. + :ok = :gen_tcp.send(server, [<<64::32, 1::32>>, "abcd"]) + :gen_tcp.close(server) + + assert %URP.SocketError{reason: :closed} = + assert_raise(URP.SocketError, fn -> P.recv_frame(client, 1_000) end) + end + end + + describe inspect(&P.send_frame/2) do + test "raises rather than failing a match when the socket is gone" do + {client, server} = socket_pair() + :gen_tcp.close(server) + :gen_tcp.close(client) + + error = assert_raise(URP.SocketError, fn -> P.send_frame(client, "a request") end) + + assert is_atom(error.reason) + end + end + + describe inspect(&URP.SocketError.exception/1) do + test "explains the reasons a caller is expected to act on" do + assert Exception.message(URP.SocketError.exception(reason: :timeout)) == + "soffice did not answer in time" + + assert Exception.message(URP.SocketError.exception(reason: :closed)) == + "soffice closed the connection" + end + + test "formats a POSIX reason it has no wording for" do + assert Exception.message(URP.SocketError.exception(reason: :econnreset)) == + "socket failed: connection reset by peer" + end + end +end